WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(5.52 KB, patch)
2010-06-02 23:59 PDT
,
Fumitoshi Ukai
no flags
Details
Formatted Diff
Diff
Patch
(1.27 KB, patch)
2010-06-08 02:41 PDT
,
Fumitoshi Ukai
eric
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 56219
[details]
Patch
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
Created
attachment 57738
[details]
Patch
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
Committed
r60652
: <
http://trac.webkit.org/changeset/60652
>
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
Created
attachment 58125
[details]
Patch
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
Committed
r60875
: <
http://trac.webkit.org/changeset/60875
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug