417943 - Use runtests.py instead of runtests.pl

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[Jeff Walden [:Waldo]]

We're up to seven or eight checkins that have been duplicated across both files, and that duplicated effort can't stop until runtests.pl is no longer used.  We should do this one platform at a time to keep things simple and to detect platform variations; I'll start with OS X first because it's what I've given the most testing.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: OS X

Comment 2

[Mats Palmgren (inactive)]

s/perl/python/ ?

Comment 3

[Jeff Walden [:Waldo]]

Attachment: Truly, I have a dizzying intellect!

Comment 4

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 303771 [details] [diff] [review]
Truly, I have a dizzying intellect!

short and sweet.

Comment 5

[Rob Campbell [:rc] (:robcee)]

coop: can you add this to staging to get it running there?

Comment 6

[Jeff Walden [:Waldo]]

Need motion on this to make use of the functionality I'm providing in bug 419339, as I've decided I've reached the end of my patience with the Perl version...

Comment 7

[Rob Campbell [:rc] (:robcee)]

well, I'm glad you've decided but we have other priorities. We'll get to it when we get to it. Should be soon!

Comment 8

[Jeff Walden [:Waldo]]

Sorry if I'm sounding impatient; part of it is that I really don't expect problems (especially for OS X), so I'm fighting a strong temptation to just make the checkins to the main tree and not bother with staging them first (which wouldn't require any effort on your side at all if indeed there were no problems as I expect).

Comment 9

[Rob Campbell [:rc] (:robcee)]

I know it's a pain dealing with both of these. We're just down a couple of people (coop and myself are only operating on about half-power this week due to moving houses [yes, we magically coincided on moving dates - we should have consulted one another]) and a slight internal reorganization.

Best guess on a timeline: we're triaging bugs tomorrow and I'm going to recommend bug 419509 gets a P2 so we get it installed next week.

Comment 10

[Rob Campbell [:rc] (:robcee)]

On WINNT 5.1 qm-stage-winxp01 dep unit test, we hit this error:

Traceback (most recent call last):
  File "runtests.py", line 418, in ?
    main()
  File "runtests.py", line 273, in main
    start = automation.runApp(testURL, browserEnv, options.app,
PROFILE_DIRECTORY)
  File "C:\slave\trunk\mozilla\objdir\_tests\testing\mochitest\automation.py",
line 202, in runApp
    profileDirectory = commands.getoutput("cygpath -w \"" + profileDir + "/\"")
NameError: global name 'commands' is not defined

This is a cygwin machine. I'm going to check its path settings to make sure they make sense. Also, I'm hoping to get a win2k3 clone into the staging environment tomorrow sometime so we can see how it works there. In the meantime, I'll do a little debugging to see what the issue is.

Comment 11

[Jeff Walden [:Waldo]]

Should the 'import commands' in runtests.py.in really be there and not in automation.py.in?  I'm betting that's the problem.

Comment 12

[Rob Campbell [:rc] (:robcee)]

definitely.

Comment 13

[Rob Campbell [:rc] (:robcee)]

I'm curious though, because I don't think we're actually using cygwin's python here. It looks like automation.py is getting its IS_CYGWIN variable set through the build's make processing. Buildbot should be running runtests.py through the system's python installed in C:\Python24. This would seem to be kind of broken.

Granted, cygwin's going to be deprecated Real Soon Now, but still... We'll see what happens when we get our MozillaBuild machine up.

Comment 14

[Jeff Walden [:Waldo]]

So, that's a problem; the commands module claims to be Unix-only, so I'd expect only the Cygwin python would have it.

Comment 15

[Rob Campbell [:rc] (:robcee)]

yeah, that's right. It might be possible to invoke using cygwin's python but we've had problems doing that through buildbot in the past. I can try tweaking the environment on those machines to see what breaks.

Comment 16

[Jeff Walden [:Waldo]]

Something like:

popen2.popen2("cygpath -w \"" + profileDir + "/\"")[0].read()

appears to be equivalent to using commands as happens currently.

Comment 17

[Rob Campbell [:rc] (:robcee)]

That should work, yes, though you might want to add a little exception handling in there in case something fails. Also, we don't need cygpathed path names if we're not using the cygwin version of python.

alternatively, you could (and probably should) use a better OS detection
mechanism in automation.py, rather than relying on autoconf+make weirdness.
Could use os.name or similar? That way you can avoid worrying about which python you're using at build time.

Comment 18

[Jeff Walden [:Waldo]]

I've tried to avoid these things in Python because, to be honest, Python's documentation is rarely good enough for me to be confident that something always works or behaves a particular way across varying platforms.  On the other hand, I can be far more confident in our build system to do exactly what I want (or at least be debuggable).

Comment 19

[(not currently active) Ted Mielczarek]

So yeah, why haven't we nuked runtests.pl from mozilla-central yet?

Comment 20

[(not currently active) Ted Mielczarek]

Attachment: remove runtests.pl from mozilla-central [checked in]

Kill runtests.pl in hg.

Comment 21

[(not currently active) Ted Mielczarek]

Comment on attachment 327019 [details] [diff] [review]
remove runtests.pl from mozilla-central [checked in]

Pushed to mozilla-central in 678ac45d2936.

Comment 22

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Gentle ping - anything left to do in this bug?

Comment 23

[(not currently active) Ted Mielczarek]

This is done. I removed runtests.pl from Hg a in comment 21. (Over 6 months ago. :)