Bug 84695 – [GTK] run-gtk-tests: Use a timeout per test instead of a global timeout

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

Created attachment 138528 [details]
Patch

[Philippe Normand]

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?

[Carlos Garcia Campos]

(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.

[Philippe Normand]

(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.

[Carlos Garcia Campos]

(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.

[Carlos Garcia Campos]

(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

[Philippe Normand]

ok then what about timed_out_tests?

[Carlos Garcia Campos]

Created attachment 138972 [details]
Updated patch

Rebased to apply on current git master and renamed timedout_tests to timed_out_tests

[Carlos Garcia Campos]

Committed r115313: <http://trac.webkit.org/changeset/115313>