Bug 56729 – Switch webkit2 bot to NRWT

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Maciej Stachowiak]

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.

[Eric Seidel (no email)]

OK.  I'm working on NRWT again.  It's unclear how much work it will be to add this support.

[Eric Seidel (no email)]

Adding support for using WebKitTestRunnner instead of DumpRenderTree seems trivial.

[Eric Seidel (no email)]

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)]

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)]

(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)]

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)]

(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)]

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]

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

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

<rdar://problem/10224918>

[Eric Seidel (no email)]

We're ready to pull the trigger.  Support is in.

[Eric Seidel (no email)]

Created attachment 111902 [details]
Patch

[Eric Seidel (no email)]

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 on attachment 111902 [details]
Patch

Clearing flags on attachment: 111902

Committed r98074: <http://trac.webkit.org/changeset/98074>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

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)]

(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)]

(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)]

I think so. Maybe the test plugin isn't getting built?

[Eric Seidel (no email)]

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)]

Try this list:
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(WebKit2%20Tests)/r98243%20(15595)/results.html

[Simon Fraser (smfr)]

I looked on the bot, and TestNetscapePlugin.plugin is missing from the build results dir for some reason

[Simon Fraser (smfr)]

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)]

I suspect that's exactly the bug, yes.  I'll move that hack into build-webkittestrunner instead of ORWT/NRWT.

[Simon Fraser (smfr)]

Yep, that's it. I filed bug 70759.

[Eric Seidel (no email)]

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