Bug 38796 – DryrunTest fails on every platform other than mac

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

DryrunTest fails on every platform other than mac

[Eric Seidel (no email)]

Windows can be fixed.  But linux can't.  We should re-write the test, or change how dryrun works to not assume Chromium in the linux case.

For now I'm going to disable the test on non-mac to make the bots green.

[Eric Seidel (no email)]

Created attachment 55461 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 55461 [details]
Patch

LGTM, r=me.

[Eric Seidel (no email)]

Committed r58999: <http://trac.webkit.org/changeset/58999>

[Eric Seidel (no email)]

Comment on attachment 55461 [details]
Patch

webkit-patch land --no-close
should have obsoleted this patch.  Something must have regressed in webkit-patch.

[Dirk Pranke]

Created attachment 55623 [details]
Patch

[Dirk Pranke]

I re-wrote the test and fixed what was breaking on chromium windows. If we want to test more ports we can add them now.

[Eric Seidel (no email)]

Comment on attachment 55623 [details]
Patch

LGTM.  Why does the mac port need to be run dryrun on mac only?

[Dirk Pranke]

Grr. Actually, this patch is bad, because it'll still try to run the chromium tests in a non-chromium checkout and fail.

[Dirk Pranke]

Created attachment 55637 [details]
Patch

[Dirk Pranke]

(In reply to comment #8)
> (From update of attachment 55623 [details])
> LGTM.  Why does the mac port need to be run dryrun on mac only?

If you run it on windows, the mac port tries to pull in server_process which pulls in fcntl, which doesn't exist.

[Dirk Pranke]

It seems like there should be a better way to get coverage of the platforms other than the one you're running on, but I'm not sure what it is.

Also, for platforms with multiple implementations (e.g., mac, chromium-mac) we need a way to figure out which is which.

[Eric Seidel (no email)]

Comment on attachment 55637 [details]
Patch

We should add a comment in the code as to why mac is run on mac-only.  Your explanation seemed to suggest it should be run on everything except for "win32".

Also:

+        if sys.platform not in 'mac':

Why change the quotes?  For better or worse our python guidelines in the wiki suggest " over ': http://trac.webkit.org/wiki/PythonGuidelines

[Dirk Pranke]

Created attachment 55640 [details]
Patch

[Dirk Pranke]

(In reply to comment #13)
> (From update of attachment 55637 [details])
> We should add a comment in the code as to why mac is run on mac-only.  Your explanation seemed to suggest it should be run on everything except for "win32".
> 
> Also:
> 
> +        if sys.platform not in 'mac':
> 
> Why change the quotes?  For better or worse our python guidelines in the wiki suggest " over ': http://trac.webkit.org/wiki/PythonGuidelines

That was not intentional. Fixed.

[Eric Seidel (no email)]

Comment on attachment 55640 [details]
Patch

LGTM.

Seems we still could make the check be != "win32" no?

[Eric Seidel (no email)]

Comment on attachment 55640 [details]
Patch

Oh, and I should have fixed:
sys.platform not in "mac"

that shoudl read "sys.platform != "mac""

not in happens to work but is not as intended.

[Dirk Pranke]

(In reply to comment #17)
> (From update of attachment 55640 [details])
> Oh, and I should have fixed:
> sys.platform not in "mac"
> 
> that shoudl read "sys.platform != "mac""
> 
> not in happens to work but is not as intended.

I was wondering about that. 

As to the != 'win32', I dunno; I haven't tested this on non-chromium linux. I think it'll work.

[Dirk Pranke]

Committed r59182: <http://trac.webkit.org/changeset/59182>

[WebKit Review Bot]

http://trac.webkit.org/changeset/59182 might have broken GTK Linux 32-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/59182
http://trac.webkit.org/changeset/59183