Bug 81606 - [WK2][WTR] Set waitUntilDone watchdog timer value equal to WK1.
Summary: [WK2][WTR] Set waitUntilDone watchdog timer value equal to WK1.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kangil Han
URL: http://build.webkit.org/results/Lion%...
Keywords: InRadar, LayoutTestFailure, Regression
Depends on:
Blocks:
 
Reported: 2012-03-19 18:12 PDT by Jessie Berlin
Modified: 2012-09-12 19:12 PDT (History)
11 users (show)

See Also:


Attachments
patch (2.57 KB, patch)
2012-09-12 06:31 PDT, Kangil Han
thorton: review+
Details | Formatted Diff | Diff
patch (2.56 KB, patch)
2012-09-12 18:25 PDT, Kangil Han
kangil.han: review+
Details | Formatted Diff | Diff
patch (2.56 KB, patch)
2012-09-12 18:26 PDT, Kangil Han
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
patch (2.56 KB, patch)
2012-09-12 18:28 PDT, Kangil Han
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Radar WebKit Bug Importer 2012-03-19 18:12:18 PDT
<rdar://problem/11078649>
Comment 2 Jessie Berlin 2012-03-19 19:35:09 PDT
Added them to the Skipped list in http://trac.webkit.org/changeset/111286
Comment 3 Kangil Han 2012-09-12 06:31:50 PDT
Created attachment 163614 [details]
patch
Comment 4 Tim Horton 2012-09-12 18:08:32 PDT
Comment on attachment 163614 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163614&action=review

> LayoutTests/ChangeLog:9
> +        6 seconds is harsh for some jquery test cases.
> +        Therefore, adjusted the value same as WK1 does.

This does not describe the change you are making to LayoutTests. This should read something like "Unskip jQuery tests which will now consistently pass due to the increased waitUntilDone timeout", or some such.

> Tools/ChangeLog:9
> +        6 seconds is harsh for some jquery test cases.
> +        Therefore, adjusted the value same as WK1 does.

I think it's unfortunate that we have single tests that take six seconds, but since they're imported I guess there's not much we can do.

jQuery is capitalized like so.

The second line should read "Therefore, adjust the value to match DumpRenderTree".

> Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:58
> +const double TestRunner::waitToDumpWatchdogTimerInterval = 30;

Fine with me.
Comment 5 Kangil Han 2012-09-12 18:24:14 PDT
(In reply to comment #4)
> (From update of attachment 163614 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163614&action=review
> 
> > LayoutTests/ChangeLog:9
> > +        6 seconds is harsh for some jquery test cases.
> > +        Therefore, adjusted the value same as WK1 does.
> 
> This does not describe the change you are making to LayoutTests. This should read something like "Unskip jQuery tests which will now consistently pass due to the increased waitUntilDone timeout", or some such.
> 
> > Tools/ChangeLog:9
> > +        6 seconds is harsh for some jquery test cases.
> > +        Therefore, adjusted the value same as WK1 does.
> 
> I think it's unfortunate that we have single tests that take six seconds, but since they're imported I guess there's not much we can do.
> 
> jQuery is capitalized like so.
> 
> The second line should read "Therefore, adjust the value to match DumpRenderTree".
> 
> > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:58
> > +const double TestRunner::waitToDumpWatchdogTimerInterval = 30;
> 
> Fine with me.

Thx for the review!!
Comment 6 Kangil Han 2012-09-12 18:25:35 PDT
Created attachment 163754 [details]
patch

Done!
Comment 7 Kangil Han 2012-09-12 18:26:52 PDT
Created attachment 163755 [details]
patch

My name besides 'review : +' looks weird so try again!
Comment 8 WebKit Review Bot 2012-09-12 18:27:21 PDT
Comment on attachment 163755 [details]
patch

Rejecting attachment 163755 [details] from commit-queue.

kangil.han@samsung.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 9 Kangil Han 2012-09-12 18:28:05 PDT
(In reply to comment #8)
> (From update of attachment 163755 [details])
> Rejecting attachment 163755 [details] from commit-queue.
> 
> kangil.han@samsung.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.
> 
> - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.
> 
> - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

Oh.. my mistake again.. Will get back with '?'. :-(
Comment 10 Kangil Han 2012-09-12 18:28:33 PDT
Created attachment 163757 [details]
patch

Try again!
Comment 11 Tim Horton 2012-09-12 18:32:34 PDT
Comment on attachment 163757 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163757&action=review

> Tools/ChangeLog:6
> +        Reviewed by Tim Horton.

The bot would have Done The Right Thing(TM) (and put whoever r+'d this patch's name in here). Just notes for next time!
Comment 12 Kangil Han 2012-09-12 18:36:10 PDT
(In reply to comment #11)
> (From update of attachment 163757 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163757&action=review
> 
> > Tools/ChangeLog:6
> > +        Reviewed by Tim Horton.
> 
> The bot would have Done The Right Thing(TM) (and put whoever r+'d this patch's name in here). Just notes for next time!

Oh, I see. Thx!
I rushed off. ;-)
Comment 13 WebKit Review Bot 2012-09-12 19:12:43 PDT
Comment on attachment 163757 [details]
patch

Clearing flags on attachment: 163757

Committed r128395: <http://trac.webkit.org/changeset/128395>
Comment 14 WebKit Review Bot 2012-09-12 19:12:48 PDT
All reviewed patches have been landed.  Closing bug.