Bug 84695 - [GTK] run-gtk-tests: Use a timeout per test instead of a global timeout
: [GTK] run-gtk-tests: Use a timeout per test instead of a global timeout
Status: RESOLVED FIXED
: WebKit
Tools / Tests
: 528+ (Nightly build)
: PC Linux
: P2 Normal
Assigned To:
:
: Gtk
:
:
  Show dependency treegraph
 
Reported: 2012-04-24 03:14 PST by
Modified: 2012-04-26 07:44 PST (History)


Attachments
Patch (6.19 KB, patch)
2012-04-24 03:22 PST, Carlos Garcia Campos
no flags Review Patch | Details | Formatted Diff | Diff
Updated patch (6.78 KB, patch)
2012-04-26 04:13 PST, Carlos Garcia Campos
pnormand: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-04-24 03:14:44 PST
Using something like 10 secs per test, like run-api-tests does, would detect timed out test earlier. It would also allow to continue running other tests instead of aborting the whole script just because one test timed out.
------- Comment #1 From 2012-04-24 03:22:55 PST -------
Created an attachment (id=138528) [details]
Patch
------- Comment #2 From 2012-04-24 06:35:43 PST -------
(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?

> Tools/Scripts/run-gtk-tests:307
> +        timedout_tests = []

Hum, what about slow_tests?
------- Comment #3 From 2012-04-24 07:43:28 PST -------
(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.

> > 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.
------- Comment #4 From 2012-04-24 08:16:47 PST -------
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 138528 [details] [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.
------- Comment #5 From 2012-04-24 08:21:52 PST -------
(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.
------- Comment #6 From 2012-04-26 03:25:28 PST -------
(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
------- Comment #7 From 2012-04-26 03:34:21 PST -------
ok then what about timed_out_tests?
------- Comment #8 From 2012-04-26 04:13:11 PST -------
Created an attachment (id=138972) [details]
Updated patch

Rebased to apply on current git master and renamed timedout_tests to timed_out_tests
------- Comment #9 From 2012-04-26 07:44:33 PST -------
Committed r115313: <http://trac.webkit.org/changeset/115313>