Summary: | [GTK] run-gtk-tests: Use a timeout per test instead of a global timeout | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | gustavo, pnormand | ||||||
Priority: | P2 | Keywords: | Gtk | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Linux | ||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2012-04-24 03:14:44 PDT
Created attachment 138528 [details]
Patch
Comment on attachment 138528 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138528&action=review Nice stuff! Looks good to me. > Tools/Scripts/run-gtk-tests:271 > + if timeout > 0: > + alarm(0) I'm not sure to understand this code. Can you clarify please? > Tools/Scripts/run-gtk-tests:307 > + timedout_tests = [] Hum, what about slow_tests? (In reply to comment #2) > (From update of attachment 138528 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=138528&action=review > > Nice stuff! Looks good to me. > > > Tools/Scripts/run-gtk-tests:271 > > + if timeout > 0: > > + alarm(0) > > I'm not sure to understand this code. Can you clarify please? alarm(0) just resets the alarm so that no new alarms are scheduled. > > Tools/Scripts/run-gtk-tests:307 > > + timedout_tests = [] > > Hum, what about slow_tests? I don't think there's any test so slow, in any case we could use a value higher than 10, but I think 10 seconds per test is enough. (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 138528 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=138528&action=review > > > > Nice stuff! Looks good to me. > > > > > Tools/Scripts/run-gtk-tests:271 > > > + if timeout > 0: > > > + alarm(0) > > > > I'm not sure to understand this code. Can you clarify please? > > alarm(0) just resets the alarm so that no new alarms are scheduled. > Ok. > > > Tools/Scripts/run-gtk-tests:307 > > > + timedout_tests = [] > > > > Hum, what about slow_tests? > > I don't think there's any test so slow, in any case we could use a value higher than 10, but I think 10 seconds per test is enough. I guess I meant the "timedout_tests" name wasn't great but that's only my opinion. I proposed slow_tests because I couldn't come up with a better name. (In reply to comment #4) > > > > Tools/Scripts/run-gtk-tests:307 > > > > + timedout_tests = [] > > > > > > Hum, what about slow_tests? > > > > I don't think there's any test so slow, in any case we could use a value higher than 10, but I think 10 seconds per test is enough. > > I guess I meant the "timedout_tests" name wasn't great but that's only my opinion. I proposed slow_tests because I couldn't come up with a better name. Ah, you meant the name, sorry. Well, there are some tests that are slow, like BackForardList but they don't time out. (In reply to comment #5) > (In reply to comment #4) > > > > > Tools/Scripts/run-gtk-tests:307 > > > > > + timedout_tests = [] > > > > > > > > Hum, what about slow_tests? > > > > > > I don't think there's any test so slow, in any case we could use a value higher than 10, but I think 10 seconds per test is enough. > > > > I guess I meant the "timedout_tests" name wasn't great but that's only my opinion. I proposed slow_tests because I couldn't come up with a better name. > > Ah, you meant the name, sorry. Well, there are some tests that are slow, like BackForardList but they don't time out. btw, it's called @testsTimedOut in run-api-tests, fwiw ok then what about timed_out_tests? Created attachment 138972 [details]
Updated patch
Rebased to apply on current git master and renamed timedout_tests to timed_out_tests
Committed r115313: <http://trac.webkit.org/changeset/115313> |