Bug 37794 - new-run-webkit-tests --verbose shows ever-increasing #EOF lines
: new-run-webkit-tests --verbose shows ever-increasing #EOF lines
Status: RESOLVED FIXED
: WebKit
Tools / Tests
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2010-04-19 03:05 PST by
Modified: 2010-04-22 18:56 PST (History)


Attachments
Patch (2.95 KB, patch)
2010-04-22 02:12 PST, Eric Seidel
no flags Review Patch | Details | Formatted Diff | Diff
Fix assertion (1.52 KB, patch)
2010-04-22 14:29 PST, Eric Seidel
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-04-19 03:05:09 PST
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.
------- Comment #1 From 2010-04-19 08:23:59 PST -------
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.
------- Comment #2 From 2010-04-19 12:18:35 PST -------
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
------- Comment #3 From 2010-04-22 02:10:00 PST -------
The log message was just confusing.  What it was actually printing was the stderr.  DRT prints an #EOF on stderr too.
------- Comment #4 From 2010-04-22 02:12:34 PST -------
Created an attachment (id=54043) [details]
Patch
------- Comment #5 From 2010-04-22 02:14:18 PST -------
(From update of attachment 54043 [details])
This seems like a bit of a hack, but this seems to be blocking you...
------- Comment #6 From 2010-04-22 02:21:31 PST -------
Committed r58076: <http://trac.webkit.org/changeset/58076>
------- Comment #7 From 2010-04-22 03:19:24 PST -------
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.
------- Comment #8 From 2010-04-22 05:08:55 PST -------
Committed r58088: <http://trac.webkit.org/changeset/58088>
------- Comment #9 From 2010-04-22 13:58:56 PST -------
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.
------- Comment #10 From 2010-04-22 14:05:29 PST -------
Yeah.  I've seen the crash too.

Our handling of stderr is clearly wrong.  I'll add another layer of hack in a second...
------- Comment #11 From 2010-04-22 14:08:17 PST -------
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.
------- Comment #12 From 2010-04-22 14:17:17 PST -------
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.
------- Comment #13 From 2010-04-22 14:29:14 PST -------
Reopening to fix dglazkov's assertion.
------- Comment #14 From 2010-04-22 14:29:34 PST -------
Created an attachment (id=54096) [details]
Fix assertion
------- Comment #15 From 2010-04-22 14:30:55 PST -------
(From update of attachment 54096 [details])
ok, makes sense.
------- Comment #16 From 2010-04-22 18:56:01 PST -------
(From update of attachment 54096 [details])
Clearing flags on attachment: 54096

Committed r58139: <http://trac.webkit.org/changeset/58139>
------- Comment #17 From 2010-04-22 18:56:08 PST -------
All reviewed patches have been landed.  Closing bug.