Closed
Bug 411007
(runtests.py)
Opened 17 years ago
Closed 17 years ago
Rewrite runtests.pl.in in Python
Categories
(Testing :: Mochitest, defect)
Testing
Mochitest
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Waldo, Assigned: Waldo)
References
Details
Attachments
(1 file, 5 obsolete files)
(deleted),
patch
|
rcampbell
:
review+
|
Details | Diff | Splinter Review |
...and correct one of the greatest mistakes I've ever made in not rewriting in a sane language when I had the chance earlier.
Assignee | ||
Comment 1•17 years ago
|
||
This seems to work on OS X, but for whatever reason it's showing the same errors the debug tinderboxen on MozillaTest are currently showing -- no idea why I see it with a Python script but not a Perl script. The only other known problem I'm aware of is that the server isn't properly killed on Windows, but there's not much I can do about that for the moment -- need bsmedberg's killableprocess to handle that. I'm using the serverShutdown handler for now to tell the server to go away, which is sufficient in most cases.
This doesn't change the existing Perl version, and the two can coexist peacefully for the moment until we can do a s/runtests\.pl/runtests\.py/g. It's a mostly straightforward translation, and it's probably not especially idiomatic Python. Unless we end up wanting killableprocess, this is 2.4-capable, so it's fine for tinderboxen.
I'm pretty sure there are problems with this patch, but I'm not sure I'd see many of them as I haven't coded Python that often. Also, the improvements I'd want to make go beyond just conversion, and it seems better to make this about just translating from Perl to Python so as not to confuse things too much.
Attachment #295599 -
Attachment is obsolete: true
Attachment #295608 -
Flags: review?(rcampbell)
Assignee | ||
Comment 2•17 years ago
|
||
Attachment #295608 -
Attachment is obsolete: true
Attachment #295609 -
Flags: review?(rcampbell)
Attachment #295608 -
Flags: review?(rcampbell)
Assignee | ||
Comment 3•17 years ago
|
||
The passwordmgr problem was because the profile path I was passing on the command line was a relative path; changing it to an absolute path fixed the problem.
Attachment #296092 -
Flags: review?
Assignee | ||
Updated•17 years ago
|
Attachment #296092 -
Flags: review? → review?(rcampbell)
Assignee | ||
Updated•17 years ago
|
Attachment #295609 -
Attachment is obsolete: true
Attachment #295609 -
Flags: review?(rcampbell)
Assignee | ||
Comment 4•17 years ago
|
||
Comment on attachment 296092 [details] [diff] [review]
Fully working now!
>Index: testing/mochitest/Makefile.in
>-_SERV_FILES = runtests.pl \
>+_SERV_FILES = \
>+ runtests.pl \
>+ runtests.py \
> gen_template.pl \
> redirect.html \
> $(topsrcdir)/netwerk/test/httpserver/httpd.js \
>+ $(topsrcdir)/tools/footprint/leak-gauge.pl \
> $(NULL)
I couldn't be bothered to remove this last addition before creating the patch; pretend it's not there.
Assignee | ||
Comment 5•17 years ago
|
||
Attachment #296092 -
Attachment is obsolete: true
Attachment #296104 -
Flags: review?(rcampbell)
Attachment #296092 -
Flags: review?(rcampbell)
Comment 6•17 years ago
|
||
heh. Give me a couple of days to play with this, Jeff. Thanks for the patch(es)!
Assignee | ||
Updated•17 years ago
|
Alias: runtests.py
Comment 7•17 years ago
|
||
I played with this last night on my windows machine and it seemed to do what we need it to. I had problems running with --file-level and --log-file options (mochitest came up, but just hung there even with --autorun). And I didn't test --appname (which I will) or the --chrome, --browser-chrome options. --autorun, --console-level and --close-when-done all worked as expected though.
one nit at the end of the file:
+##################
+# Do everything. #
+##################
+
+main()
should replace with:
if __name__ == '__main__':
main()
It's a standard pythonism and makes it easier to deal with in an interactive environment for debugging.
More to follow...
Assignee | ||
Comment 8•17 years ago
|
||
--log-file and --log-level work for me on OS X; I'll have to fire up the Windows VM to check there, I guess. Looking at the code, tho, I should probably be urlencoding the query parameters somehow (and need to double-check that would get undone on the other side); look for that in a later patch.
What's the newest version of Python that can be required to run this? I thought I'd been running 2.4 in testing, but since |A if C else B| is a 2.5ism, I clearly couldn't have been doing so. All versions of MozillaBuild, OS X 10.4, and recent Linux distros all ship 2.5, which has been out since September 2006.
Comment 9•17 years ago
|
||
In 1.9 we should work with python 2.3
In moz2 we will require python 2.4
I don't know much about "A if C else B" but I think you can get a similar result with "A or B"... sayrer is this correct?
Assignee | ||
Comment 10•17 years ago
|
||
Ugh. The ternary syntax I can live without, but 2.3 claims not to have subprocess:
http://www.python.org/doc/2.3/modindex.html
Comment 11•17 years ago
|
||
OS X 10.4 ships with an out-dated 2.3 version of python. All the build machines install 2.4 at a minimum as part of the "refplatform".
2.3 should be considered too old and I wouldn't worry about that. We want a target of 2.4 or greater so we can wedge killableprocess.py in here. We'll have to wait for ternary syntax.
Comment 12•17 years ago
|
||
Where does runtests.pl (to-be-py) live? If this is part of the standard unit-test framework, we should continue to support python 2.3... if this is something particular to Talos, then we can use whatever version of python we want.
Comment 13•17 years ago
|
||
It's for mochitest.
Comment 14•17 years ago
|
||
no, as Ted says, this is going to live next to runtests.pl in mozilla/testing/mochitest.
I was thinking about adding killableprocess.py in alongside it, or possibly
putting it into a tools/python/killableprocess package we could install. Any
opinions?
Comment 15•17 years ago
|
||
Ordinary developers are expected to run runtests.pl to run the mochitests locally, according to http://developer.mozilla.org/en/docs/Mochitest#Running_Mochitest
Therefore I think we should stay 2.3-compatible... we could of course import subprocess/killableprocess conditionally for newer pythons that support it.
Assignee | ||
Comment 16•17 years ago
|
||
The point is that I find the existing Perl file hard to read and hard to edit. Every time I have to edit it, I have to relearn the Perl necessary to do so. I don't want to have to do that, I'm pretty sure sayrer doesn't want to have to do that, and I want to be able to use a language I can read and know reasonably well to implement Mochitesting functionality.
I don't know what to say if this can't replace the Perl version; I'm willing to endure pain, even considerable pain, to make it work in 2.3, but I don't know how possible that is staying inside the standard Python distribution. If it is, I will gladly take that over being forced to touch Perl any time I need to change things here, even if it requires considerable effort now. If it isn't...I'd rather not consider that possibility until forced to do so. I can't describe how bad I feel about being forced to use subpar, unusable technology because we can't use something new that's not insane.
Please don't take this as exaggeration; it absolutely is not.
Comment 17•17 years ago
|
||
The python 2.3 requirement is mac/unix only... we can require python 2.5 (mozillabuild) on Windows. So for *nix-only, it looks pretty trivial to import popen2 and use the Popen3 object to get the pid.
Comment 18•17 years ago
|
||
http://docs.python.org/lib/module-popen2.html for reference
Comment 19•17 years ago
|
||
Wow, Jeff. And I thought I disliked runtests.pl. :)
Those are good suggestions, Ben. This should be pretty easy to make backwards compatible. Then, in a year or two we can just rip out the 2.3 requirements (and start porting forward to python 3.0).
Also, there's no reason we can't keep runtests.pl in the tree and just consider it deprecated from this point forward. People can use what they feel comfortable with or what works. I think the majority will move to using the python version as it is much more understandable.
Thanks for your work on this, Jeff!
Assignee | ||
Comment 20•17 years ago
|
||
I've only tested this on Mac, but the Windows parts were just copy-paste so I don't expect any bustage there.
As for the Perl version, I wrote this because I can't stand hacking on this in Perl. As soon as this is in and tinderboxen are converted to use this, I'd like to see us either remove the Perl version or temporarily replace it with a file that prints a warning message and then calls the Python one, which would be removed in a few months once everyone knows what's up. Maintaining two of these isn't a good use of anyone's time.
Attachment #296104 -
Attachment is obsolete: true
Attachment #299475 -
Flags: review?(rcampbell)
Attachment #296104 -
Flags: review?(rcampbell)
Comment 21•17 years ago
|
||
alright, I'm going to spend this afternoon after the meetings playing with this. I agree we should move to deprecate runtests.pl, though I'm reluctant to rip it out entirely right away.
Comment 22•17 years ago
|
||
Comment on attachment 299475 [details] [diff] [review]
Abstract process management to cater to obsolete versions of Python
looks good.
Attachment #299475 -
Flags: review?(rcampbell) → review+
Assignee | ||
Comment 23•17 years ago
|
||
Committed, and added a note to the topic in #developers about this -- so hopefully we'll get some feedback on this. Ideally it's effectively a noop for developers, but we'll see...
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Comment 24•16 years ago
|
||
Mass move of the rest of the Mochitest bugs from Core:Testing to Testing:Mochitest. Filter on MochitestMassMove to ignore.
Component: Testing → Mochitest
Product: Core → Testing
QA Contact: testing → mochitest
You need to log in
before you can comment on or make changes to this bug.
Description
•