Bug 97081

Summary: [EFL][WK2] Check timeout on waitUntilLoadFinished() and waitUntilTitleChangedTo()
Product: WebKit Reporter: Byungwoo Lee <bw80.lee>
Component: WebKit EFLAssignee: Byungwoo Lee <bw80.lee>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, gyuyoung.kim, lucas.de.marchi, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 97094    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Byungwoo Lee
Reported 2012-09-19 00:58:02 PDT
add timeout to the functions and check return value of the functions will be better to prevent test timeout due to the infinite loop.
Attachments
Patch (17.58 KB, patch)
2012-09-19 07:47 PDT, Byungwoo Lee
no flags
Patch (17.56 KB, patch)
2012-09-19 21:20 PDT, Byungwoo Lee
no flags
Patch (18.13 KB, patch)
2012-09-20 01:50 PDT, Byungwoo Lee
no flags
Byungwoo Lee
Comment 1 2012-09-19 07:47:30 PDT
Chris Dumez
Comment 2 2012-09-19 08:21:03 PDT
Comment on attachment 164737 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164737&action=review Makes sense, thanks. > Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.h:45 > + static const double timeOutAfter10Seconds = 10.0; Should probably be named something like "defaultTimeoutSeconds".
Gyuyoung Kim
Comment 3 2012-09-19 18:01:22 PDT
Comment on attachment 164737 [details] Patch Looks fine as well. Please land after fixing the nit Chrisophe pointed out.
Byungwoo Lee
Comment 4 2012-09-19 18:13:29 PDT
(In reply to comment #3) > (From update of attachment 164737 [details]) > Looks fine as well. Please land after fixing the nit Chrisophe pointed out. Thanks~ I'll change it :)
Byungwoo Lee
Comment 5 2012-09-19 21:20:40 PDT
Gyuyoung Kim
Comment 6 2012-09-19 21:38:47 PDT
Please verify if there is no unit test failure on both release and debug before landing. As you know, there were sometime problems on unit test.
Byungwoo Lee
Comment 7 2012-09-19 21:40:56 PDT
(In reply to comment #6) > Please verify if there is no unit test failure on both release and debug before landing. As you know, there were sometime problems on unit test. Sure. I'll test and reply. Thanks.
Byungwoo Lee
Comment 8 2012-09-20 01:30:48 PDT
(In reply to comment #7) > (In reply to comment #6) > > Please verify if there is no unit test failure on both release and debug before landing. As you know, there were sometime problems on unit test. > > Sure. I'll test and reply. Thanks. I tested this patch with the latest source again and it works find for both release and debug. But I found that waitUntilURIChanged is added, so I'll apply this to the function also and upload it. :)
Chris Dumez
Comment 9 2012-09-20 01:34:04 PDT
https://bugs.webkit.org/show_bug.cgi?id=97094 added a new waitForXXX() method. You should probably fix the default timeout value for that method as well.
Byungwoo Lee
Comment 10 2012-09-20 01:35:44 PDT
(In reply to comment #9) > https://bugs.webkit.org/show_bug.cgi?id=97094 added a new waitForXXX() method. You should probably fix the default timeout value for that method as well. Yes, I'm preparing
Byungwoo Lee
Comment 11 2012-09-20 01:50:23 PDT
Chris Dumez
Comment 12 2012-09-20 01:54:08 PDT
Comment on attachment 164861 [details] Patch LGTM. Thanks.
Gyuyoung Kim
Comment 13 2012-09-20 01:55:04 PDT
Comment on attachment 164861 [details] Patch Please land this after passing unit test both release and debug.
Byungwoo Lee
Comment 14 2012-09-20 03:01:01 PDT
Comment on attachment 164861 [details] Patch I tested the last patch again for release/debug, and both were ok. Thanks :)
WebKit Review Bot
Comment 15 2012-09-20 03:27:01 PDT
Comment on attachment 164861 [details] Patch Clearing flags on attachment: 164861 Committed r129111: <http://trac.webkit.org/changeset/129111>
WebKit Review Bot
Comment 16 2012-09-20 03:27:05 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.