Bug 81606

Summary: [WK2][WTR] Set waitUntilDone watchdog timer value equal to WK1.
Product: WebKit Reporter: Jessie Berlin <jberlin>
Component: JavaScriptCoreAssignee: Kangil Han <kangil.han>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, andersca, ap, jberlin, kangil.han, sam, simon.fraser, slewis, thorton, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: InRadar, LayoutTestFailure, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://build.webkit.org/results/Lion%20Intel%20Debug%20(WebKit2%20Tests)/r111273%20(5061)/jquery/core-pretty-diff.html
Attachments:
Description Flags
patch
thorton: review+
patch
kangil.han: review+
patch
webkit.review.bot: commit-queue-
patch none

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.