Bug 37794 – new-run-webkit-tests --verbose shows ever-increasing #EOF lines

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

[Eric Seidel (no email)]

new-run-webkit-tests --verbose shows ever-increasing #EOF lines

100419 03:02:32 dump_render_tree_thread.py:95   DEBUG Previous test output extra lines after dump:
#EOF
#EOF
#EOF

After every test, the number of #EOFs dumped there is larger.  Eventually you just can't read the log at all.

Something is wrong in the WebKit driver or in the logging or both.

[Ojan Vafai]

Context: NRWT outputs the results between the #EOF for a test and the #URL... for the next test. This avoids putting the results from one test into the next (reduces flakiness). We spit this to stdout in the hopes that we can put it on the waterfall and get it fixed.

In either case, it sounds like maybe DRT is printing multiple #EOF lines per test and that is confusing NRWT since it expects exactly one #EOF per #URL line. That's just a guess though.

[Eric Seidel (no email)]

I believe that DRT prints out two #EOFs.  One for the test content and one for the pixel content.
http://trac.webkit.org/browser/trunk/WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm#L1061

[Eric Seidel (no email)]

The log message was just confusing.  What it was actually printing was the stderr.  DRT prints an #EOF on stderr too.

[Eric Seidel (no email)]

Created attachment 54043 [details]
Patch

[Adam Barth]

Comment on attachment 54043 [details]
Patch

This seems like a bit of a hack, but this seems to be blocking you...

[Eric Seidel (no email)]

Committed r58076: <http://trac.webkit.org/changeset/58076>

[Eric Seidel (no email)]

You were right.  It was too much of a hack.  I had to add another hack to handle the case where we get nothing back from stderr.  I'm still not really sure what's going on here, but hopefully Dirk does.  We're definitely in a better state than we were before, but not yet where we want to end up.

[Eric Seidel (no email)]

Committed r58088: <http://trac.webkit.org/changeset/58088>

[Dimitri Glazkov (Google)]

This broke new-run-webkit-tests --platform=chromium-mac --use-drt, which now crashes on this assertion. Not sure whether it's a bug being tickled, just noting here what's happening.

[Eric Seidel (no email)]

Yeah.  I've seen the crash too.

Our handling of stderr is clearly wrong.  I'll add another layer of hack in a second...

[Ojan Vafai]

A bit late but...

Seems like the "correct" fix would be to add a read_error_line method to  match read_line. Then in the appropriate loops where you are reading output, you could also read the errors. In retrospect, it's weird that read_line also processes stderr.

Alternately, we could modify readline to return a (output, error_output) tuple where output or error_output may be None/empty-string.

[Eric Seidel (no email)]

It appears like we're not always waiting for the full error though.  That's the ASSERT dglazkov is seeing, is that sometimes the error doens't contain the #EOF.

[Eric Seidel (no email)]

Reopening to fix dglazkov's assertion.

[Eric Seidel (no email)]

Created attachment 54096 [details]
Fix assertion

[Dimitri Glazkov (Google)]

Comment on attachment 54096 [details]
Fix assertion

ok, makes sense.

[WebKit Commit Bot]

Comment on attachment 54096 [details]
Fix assertion

Clearing flags on attachment: 54096

Committed r58139: <http://trac.webkit.org/changeset/58139>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.