Closed
Bug 142744
Opened 22 years ago
Closed 20 years ago
Testing suite should work on Win32
Categories
(Bugzilla :: Testing Suite, enhancement)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: jouni, Assigned: glob)
References
Details
Attachments
(1 file, 2 obsolete files)
1.02 KB,
patch
|
Details | Diff | Splinter Review |
Windows development is seriously harmed by the fact that the testing suite doesn't run on Windows. I can take a look at this as I'm the one with the need. Ccing zach@zachlipton.com for comments: do you know if this is downright impossible? At least the runtests.sh needs a Windows companion, but wonder if Test:: modules work equally on Win32 platforms?
Comment 1•22 years ago
|
||
The Test::* should work find under win32, but there are two issues that I know of: 1. runtests.sh, we should rewrite it in perl so it works anywhere 2. Filepaths, I think we currently assume that the directory seperator is a '/' and not a '\', this needs to be fixed to use this little code nugget from my bucket-o-tricks: use File::Spec; sub fixpath($) { my ($path) = @_; my @pathlist = split('/',$path); return File::Spec->catfile(@pathlist); } This takes a path using a standard '/' directory seperator and returns it for the current filesystem.
Reporter | ||
Comment 2•22 years ago
|
||
Ok, thanks for your input. I'll delve into this soonish. Marking 2.18 per the "Out-of-the-box compatibility with Win32" statement in the Bugzilla roadmap.
Target Milestone: --- → Bugzilla 2.18
Reporter | ||
Comment 3•22 years ago
|
||
I've cast a quick glance at the code. The essential problems seem to be the some instances of backticks and input redirection - that stuff is fairly unixy. It's probably going to take a while before this truly works, but I'm looking into it. ActiveState is doing quite an impressive job hiding the problems with \ and / conflicts, by the way...
Reporter | ||
Comment 4•22 years ago
|
||
This should make it tick...
Reporter | ||
Comment 5•22 years ago
|
||
Ok, now the version version is ready for review. In order to test this on Win32 you also need the new runtests.pl from bug 143155; unix tests can be run with the old runtests.sh. I've tested the patch on Win32 ActiveState Perl 5.6.1 and on Perl 5.6.0 on netbsd. My other Unix testing platforms are currently in pieces, so can't test more on those. The changes should be safe though. Ccing justdave since all the more appropriate reviewers are unavailable for a while and you do know something about Win32 (according to some comments on bug 124174).
Comment 6•22 years ago
|
||
Comment on attachment 84078 [details] [diff] [review] Patch v1 This is really really ugly, but if it works for you, r=bbaetz x2 I suspect using IPC::Run for the test suite would be overkill.
Attachment #84078 -
Flags: review+
Comment 7•22 years ago
|
||
Questions: Do we need to put win32Exec() in every test file? Why not put it in a module rather than using the same block of code 4+ times? Also, can we have a bugzillaexec() sub that Does The Right Thing for windows and for linux instead of things like my $loginfo = $iswin ? win32Exec($command) : `$command 2>&1`;
Comment 8•22 years ago
|
||
Actually, why don't we just slurp the file into a var, wrap it in a sub so that code isn't run, and then eval it? Thats what mod_perl does, and we'll have to do that to test suitablilty. The downside is that thta will generate warnings at the moment, because our code isn't designed to work that way
Reporter | ||
Comment 9•22 years ago
|
||
Comment on attachment 84078 [details] [diff] [review] Patch v1 This is bitrotten, so obsoleting. Zach, Bbaetz: Some of what you're discussing implies changes of a wider scope for the testing suite. That's way beyond my resources now. If you consider this approach useful, I might try refreshing the patch. What do you think about this?
Attachment #84078 -
Attachment is obsolete: true
Reporter | ||
Comment 10•21 years ago
|
||
Umh. Do we consider this to be a 2.18 blocker? I'd go for saying no, since that would mostly be catering for me, myself and couple of other people who develop on Windows. "Out of the box compatibility" maybe shouldn't be extended to development tools. This patch should be almost totally rewritten, as the testing suite has changed quite a bit since I wrote this patch 18 months ago...
Comment 11•21 years ago
|
||
It'd be nice, but not necessary. I agree. On the other hand, once upon a time, Jake had a tinderbox running on a Win32 box... and it worked. Did something break since then?
Updated•20 years ago
|
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
Assignee | ||
Comment 12•20 years ago
|
||
all tests work ok for me on win32, with the exception of 008filter, due to the path separator. here's a patch.
Attachment #150329 -
Flags: review?(jouni)
Reporter | ||
Comment 13•20 years ago
|
||
Comment on attachment 150329 [details] [diff] [review] 008filter.t patch Sorry it's taken so long, I've been insanely busy for the last weeks. >+ if ($^O eq 'MSWin32') { >+ # filterexceptions.pl uses / so convert to \ on win32 Make this comment something like # filterexceptions.pl uses / separated paths, while # find_actual_files returns \ separated ones on Windows. # Here, we convert the filter exception hash to use \. >+ foreach my $file (keys %safe) { >+ my $orig_file = $file; >+ $file =~ s|/|\\|g; >+ $safe{$file} = $safe{$orig_file}; Throw in a $safe{$orig_file} = undef unless $file eq $orig_file; for good measure; it's not necessary to store duplicate entries with both path separators. Although that's hardly relevant in any practical sense, it's also a matter of clarity; we do want to _alter_, not duplicate the hash keys. Other than those, r=jouni. I'm surprised rest of the testing suite worked so painlessly. That's good. :)
Attachment #150329 -
Flags: review?(jouni) → review+
Reporter | ||
Updated•20 years ago
|
Assignee: jouni → bugzilla
Status: ASSIGNED → NEW
Flags: approval?
Assignee | ||
Comment 14•20 years ago
|
||
addresses jouni's comments
Attachment #150329 -
Attachment is obsolete: true
Updated•20 years ago
|
Flags: approval? → approval+
Target Milestone: Bugzilla 2.20 → Bugzilla 2.18
Assignee | ||
Comment 15•20 years ago
|
||
can someone please check this patch in for me.
Reporter | ||
Comment 16•20 years ago
|
||
Certainly. Checking in 008filter.t; /cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v <-- 008filter.t new revision: 1.12; previous revision: 1.11 done
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•