Bug 56729 (nrwt-wk2)

Summary: Switch webkit2 bot to NRWT
Product: WebKit Reporter: Maciej Stachowiak <mjs>
Component: Tools / TestsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, dpranke, eric, laszlo.gombos, lforschler, simon.fraser, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 63439, 63501, 63673, 63683, 63780, 64056, 67530, 70371, 70435    
Bug Blocks: 34984, 66965    
Attachments:
Description Flags
Patch none

Maciej Stachowiak
Reported 2011-03-20 21:15:00 PDT
new-run-webkit-tests lacks old-run-webkit-tests' support for WebKit2. This consists of the --2 option which uses the WebKitTestRunner tool, and support for detecting WebProcess crashes.
Attachments
Patch (2.02 KB, patch)
2011-10-20 21:58 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2011-06-26 23:28:01 PDT
OK. I'm working on NRWT again. It's unclear how much work it will be to add this support.
Eric Seidel (no email)
Comment 2 2011-06-27 00:01:09 PDT
Adding support for using WebKitTestRunnner instead of DumpRenderTree seems trivial.
Eric Seidel (no email)
Comment 3 2011-06-27 00:04:24 PDT
Looks like this should be as simple as teaching the port how to return driver_name() of "WebKitTestRunner" when passed the --2 option, as well as teaching WebKitDriver._read_block how to understand #CRASHED and #CRASHED - WebProcess
Eric Seidel (no email)
Comment 4 2011-06-27 01:56:46 PDT
Did half of what is needed in bug 63439. The harder part of actually making us talk to WebKitTestRunner (and supporting the new #CRASHED stuff) is left.
Adam Roben (:aroben)
Comment 5 2011-06-27 05:40:13 PDT
(In reply to comment #3) > Looks like this should be as simple as teaching the port how to return driver_name() of "WebKitTestRunner" when passed the --2 option, as well as teaching WebKitDriver._read_block how to understand #CRASHED and #CRASHED - WebProcess FYI, Apple's Windows port of DRT also prints #CRASHED in some cases, so it would be useful for #CRASHED to be understood even when using DRT.
Eric Seidel (no email)
Comment 6 2011-06-27 20:41:28 PDT
new-run-webkit-tests -2 "just works" after bug 63501. I've not yet implemented the #CRASHED handling, but I'm not actually sure it's necessary. We're still correctly detecting crashes, etc. Does WebKitTestRunner continue running after a WebProcess crash, or does it itself exit?
Eric Seidel (no email)
Comment 7 2011-06-27 20:42:08 PDT
(My previous comment was not to imply that I'm in any way against the #CRASHED handling, just stating that new-run-webkit-tests -2 seems to work even though we haven't added it yet.)
Eric Seidel (no email)
Comment 8 2011-06-27 20:59:06 PDT
After the patch on bug 63501 lands, we still have some 20 unexpected failures with -2. We can use this bug to track such.
Maciej Stachowiak
Comment 9 2011-06-28 00:36:52 PDT
(In reply to comment #6) > new-run-webkit-tests -2 "just works" after bug 63501. I've not yet implemented the #CRASHED handling, but I'm not actually sure it's necessary. We're still correctly detecting crashes, etc. > > Does WebKitTestRunner continue running after a WebProcess crash, or does it itself exit? Yes, WebKitTestRunner continues after a WebProcess crash (unless the driver script tells it otherwise). That's kind of the point of the WebProcess, to not crash its client when it crashes. Also, if WKTR made itself crash when WebProcess crashes, then results would show a backtrace of the wrong process.
Maciej Stachowiak
Comment 10 2011-06-28 00:38:22 PDT
(In reply to comment #7) > (My previous comment was not to imply that I'm in any way against the #CRASHED handling, just stating that new-run-webkit-tests -2 seems to work even though we haven't added it yet.) It should work without it, you just need to detect WebProcess crashes and hangs if you don't want the test session to hang indefinitely when it happens. Making the WebProcess crash or hang deliberately (using kill -SEGV to crash or kill -STOP to hang) should be a reasonable way to test it. It could be a separate bug.
Radar WebKit Bug Importer
Comment 11 2011-10-03 11:26:01 PDT
<rdar://problem/10224918>
Eric Seidel (no email)
Comment 12 2011-10-19 21:57:18 PDT
We're ready to pull the trigger. Support is in.
Eric Seidel (no email)
Comment 13 2011-10-20 21:58:07 PDT
Created attachment 111902 [details] Patch
Eric Seidel (no email)
Comment 14 2011-10-20 22:00:28 PDT
I've tested locally. My machine sees 81 failures with WebKit2, in both ORWT and NRWT. Which matches the 81 failures seen on the bot.
WebKit Review Bot
Comment 15 2011-10-20 23:48:16 PDT
Comment on attachment 111902 [details] Patch Clearing flags on attachment: 111902 Committed r98074: <http://trac.webkit.org/changeset/98074>
WebKit Review Bot
Comment 16 2011-10-20 23:48:23 PDT
All reviewed patches have been landed. Closing bug.
Simon Fraser (smfr)
Comment 17 2011-10-21 11:12:54 PDT
This seems to have caused lots of plugin tests to fail: http://build.webkit.org/results/Lion%20Intel%20Release%20(Tests)/r98106%20(2090)/results.html
Eric Seidel (no email)
Comment 18 2011-10-21 11:15:21 PDT
(In reply to comment #17) > This seems to have caused lots of plugin tests to fail: > http://build.webkit.org/results/Lion%20Intel%20Release%20(Tests)/r98106%20(2090)/results.html That builder doesn't use WK2 does it? I"m not sure that's related to this specific change, but may be related to other changes which I made to support WK2?
Eric Seidel (no email)
Comment 19 2011-10-22 18:04:59 PDT
(In reply to comment #17) > This seems to have caused lots of plugin tests to fail: > http://build.webkit.org/results/Lion%20Intel%20Release%20(Tests)/r98106%20(2090)/results.html Do you still believe this to be the case?
Simon Fraser (smfr)
Comment 20 2011-10-24 11:30:57 PDT
I think so. Maybe the test plugin isn't getting built?
Eric Seidel (no email)
Comment 21 2011-10-24 11:55:00 PDT
Behavior on the lion bot (non-wk2) should not have changed (at least not intentionally). It's unclear which of the tests you cite from: http://build.webkit.org/results/Lion%20Intel%20Release%20(Tests)/r98106%20(2090)/results.html are plugin tests. If you can give me an example test, which you see failing on the bots now with NRWT, or can demonstrate locally as failing with NRWT and passing with ORWT, i'm *very* happy to investigate and fix. Thank you again for your help and patience!
Simon Fraser (smfr)
Comment 23 2011-10-24 13:40:33 PDT
I looked on the bot, and TestNetscapePlugin.plugin is missing from the build results dir for some reason
Simon Fraser (smfr)
Comment 24 2011-10-24 13:43:30 PDT
ORWT has: # FIXME: We build both DumpRenderTree and WebKitTestRunner for # WebKitTestRunner runs becuase DumpRenderTree still includes # the DumpRenderTreeSupport module and the TestNetscapePlugin. # These two projects should be factored out into their own # projects. buildDumpTool("DumpRenderTree"); buildDumpTool("WebKitTestRunner") if $useWebKitTestRunner; Do we need the same in NRWT?
Eric Seidel (no email)
Comment 25 2011-10-24 13:56:22 PDT
I suspect that's exactly the bug, yes. I'll move that hack into build-webkittestrunner instead of ORWT/NRWT.
Simon Fraser (smfr)
Comment 26 2011-10-24 13:57:48 PDT
Yep, that's it. I filed bug 70759.
Eric Seidel (no email)
Comment 27 2011-10-24 13:58:23 PDT
(In reply to comment #25) > I suspect that's exactly the bug, yes. I'll move that hack into build-webkittestrunner instead of ORWT/NRWT. Filed https://bugs.webkit.org/show_bug.cgi?id=70760. Will post a patch momentarily.
Note You need to log in before you can comment on or make changes to this bug.