RESOLVED FIXED 39058
old-run-webkit-tests should use ensure-valid-python to check if it can run the websockets tests
https://bugs.webkit.org/show_bug.cgi?id=39058
Summary old-run-webkit-tests should use ensure-valid-python to check if it can run th...
Eric Seidel (no email)
Reported 2010-05-13 06:13:39 PDT
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.
Attachments
Patch (4.41 KB, patch)
2010-05-17 00:56 PDT, Fumitoshi Ukai
no flags
Patch (5.52 KB, patch)
2010-06-02 23:59 PDT, Fumitoshi Ukai
no flags
Patch (1.27 KB, patch)
2010-06-08 02:41 PDT, Fumitoshi Ukai
eric: review+
Eric Seidel (no email)
Comment 1 2010-05-13 06:14:35 PDT
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
Comment 2 2010-05-13 09:08:42 PDT
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)
Comment 3 2010-05-13 11:13:09 PDT
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
Comment 4 2010-05-13 11:20:22 PDT
> 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
Comment 5 2010-05-17 00:56:05 PDT
Shinichiro Hamaji
Comment 6 2010-06-02 23:33:25 PDT
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
Comment 7 2010-06-02 23:58:54 PDT
(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
Comment 8 2010-06-02 23:59:33 PDT
Alexey Proskuryakov
Comment 9 2010-06-03 10:19:31 PDT
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
Comment 10 2010-06-03 21:22:24 PDT
Eric Seidel (no email)
Comment 11 2010-06-07 09:45:15 PDT
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)
Comment 12 2010-06-07 09:49:17 PDT
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)
Comment 13 2010-06-08 02:14:55 PDT
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
Comment 14 2010-06-08 02:41:59 PDT
Eric Seidel (no email)
Comment 15 2010-06-08 10:53:34 PDT
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 16 2010-06-08 10:54:01 PDT
Comment on attachment 57738 [details] Patch Marking this patch obsolete since it was landed.
Fumitoshi Ukai
Comment 17 2010-06-08 18:03:03 PDT
Note You need to log in before you can comment on or make changes to this bug.