Bug 39058 – old-run-webkit-tests should use ensure-valid-python to check if it can run the websockets tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Eric Seidel (no email)]

old-run-webkit-tests should use ensure-valid-python to check if it can run the websockets tests

Right now the tests are manually skipped for Tiger, which is bad.  Would be better to run them automatically when we can find python 2.5+ and not when we can't.

[Eric Seidel (no email)]

ensure-valid-python will need to have a --non-interactive option or some flag to make it not try to install if missing.  Actually it should probably default to not installing if missing since it can't do so w/o a user sitting at the keyboard.

[Alexey Proskuryakov]

I think that this should be WONTFIX. Skipping tests is bad - if we require developers to make the extra effort of installing custom python, this should be mandatory.

[Eric Seidel (no email)]

It's just like how we dynamically skip tests on Windows when your machine isn't supported for hardware compositing.

This came up because the Tiger bot currently has Python 2.3, and the resolution to that was to skip all the websocket tests (which is bad).  I'd rather have the skip only happen if someone is missing the proper python.  We can display a huge warning if you like.

I'm happy to close this as WONTFIX, as it means less work for me.  But I think automatically skipping with a big warning is a better behavior than our current manual skip or than making it impossible to run test at all w/o python 2.5.

[Alexey Proskuryakov]

> It's just like how we dynamically skip tests on Windows when your machine
> isn't supported for hardware compositing.

Yes, but Tiger support WebSocket just fine. It's purely a tools issue, unlike the compositing one.

[Fumitoshi Ukai]

Created attachment 56219 [details]
Patch

[Shinichiro Hamaji]

Comment on attachment 56219 [details]
Patch

Hi Alexey, this patch would not be the best solution, but seems to make the situation better. If I understand correctly, now we are just skipping all websocket tests by mac-tiger/Skipped even when python 2.5 is manually installed. So, I'd like to put r+ for this patch. Or, we might want to add a flag like --skip-python25-tests-if-we-dont-have-python25 into old-run-webkit-tests and use this flag in our buildbot for now. What do you think?

By the way Ukai-san, cannot we remove websocket/tests from mac-tiger/Skipped with this patch?

[Fumitoshi Ukai]

(In reply to comment #6)

> By the way Ukai-san, cannot we remove websocket/tests from mac-tiger/Skipped with this patch?

yes, we can.

[Fumitoshi Ukai]

Created attachment 57738 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 57738 [details]
Patch

>         print "Your Python version is insuficient to run WebKit's Python code.  Please update.\n";

Typo: should be "insufficient".

+    `./WebKitTools/Scripts/ensure-valid-python --check-only`;

We normally use system(), not backticks. But there's precedent for the latter, too.

[Fumitoshi Ukai]

Committed r60652: <http://trac.webkit.org/changeset/60652>

[Eric Seidel (no email)]

Can't exec "WebKitTools/Scripts/ensure-valid-python": No such file or directory at ../../Projects/WebKit/WebKitTools/Scripts/old-run-webkit-tests line 1441.
WARNING: Your platform does not have Python 2.5+, which is required to run websocket server, so disabling websocket/tests.
Running build-dumprendertree

I get that when trying to call run-webkit-tests from outside of a webkit checkout.  Seems the change didn't use the wkdirs module as it should have.

[Eric Seidel (no email)]

Yes, this is the wrong way to launch scripts:

 1441     `./WebKitTools/Scripts/ensure-valid-python --check-only`;

The perl code is lame about how it handles paths.  The python code always uses absolute paths.  The other callsites make similar relative-path calls and get away with it because they're after the chdirWebKit() call in old-run-webkit-tests.

I think you should instead make your path absolute using sourceDir() from webkitdirs.  You could also use relativeScriptsDir().

[Eric Seidel (no email)]

Can't exec "WebKitTools/Scripts/ensure-valid-python": No such file or directory at ../WebKitTools/Scripts/old-run-webkit-tests line 1441.
WARNING: Your platform does not have Python 2.5+, which is required to run websocket server, so disabling websocket/tests.

I'm getting this all the time.  We need to fix or roll this out (please). :)

[Fumitoshi Ukai]

Created attachment 58125 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 58125 [details]
Patch

I think its good practice for all paths to be absolute, regardless of what the current directory is.  I wouldn't bother adding the comment, but this looks fine.

[Eric Seidel (no email)]

Comment on attachment 57738 [details]
Patch

Marking this patch obsolete since it was landed.

[Fumitoshi Ukai]

Committed r60875: <http://trac.webkit.org/changeset/60875>