When a test crashes in the chromium port, the full stacktrace is produced by the binary but isn't actually visible it the output. It looks like the output is ending up in DriverOutput.text, but when a test crashes we only output DriverOutput.error. http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py#L69. I think this is due to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py#L448 which pipes stderr to the same place as stdout. If locally I patch http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py#L69 to write "driver_output.error + driver_output.text", then I do see the full stack (in addition to whatever test output was produced on stdout). Based on the comments we should be using ServerProcess, which does have logic to open up stdout and stderr FDs and select() between them. Because of this bug we often don't get usable stack traces from even the debug layout test bots, which sucks.
At a glance maybe I could change the Popen call to leave stderr in its own PIPE, modify ChromiumDriver._write_command_and_read_line() to return a tuple (line, error, did_crash) and do a select() on stdout/stderr inside that function instead of reading directly from stdout always. Sound reasonable?
Our stdout vs. stderr handling needs work in both the WebKit and Chromium Driver implementations. I do have plans to fix WebKit's/ServerProcess, but I don't have any plans to fix ChromiumDriver since Chromium's DRT is so different from the rest (and I don't want to break it). It seems we should be writing out to a stderr file liek the rest of the webkit ports do, no?
Why would we write the stderr output into a file instead of reading it directly from the test harness?
https://bugs.webkit.org/show_bug.cgi?id=58708 seems related.
The problem with your solution in comment #1 is that select doesn't work on windows. You end up needing to do something like what I outlined in bug bug 58708, or looping between the two fd's after setting them both to be nonblocking (assuming that even works, I haven't tested it on all platforms, but it probably does). It would be nice if we could unify around ServerProcess, but as Eric says, the logic is at least somewhat different so it's a non-zero amount of work. We could consider also making Chromium DRT match the other ports' output; Tony & co got this mostly working but we never finished working out the bugs. In addition to letting us delete code, it would also probably provide a decent speed boost since we wouldn't have to write the PNGs to disk.
Can you point me at where that work went to unify the DRT output? I think that would be a big win, since currently when we get a crash on the bots we have to reproduce locally just to get a stack. If we could just get a stack from the bots it would help tremendously.
Created attachment 105091 [details] Patch
Comment on attachment 105091 [details] Patch change the run_test() method in ChromiumPort to return output + error if it's a crash instead. That way the hack is isolated to Chromium ports.
(In reply to comment #6) > Can you point me at where that work went to unify the DRT output? Chromium's DRT can do either output. If you pass in --test-shell, we get the Chromium style output, otherwise, we're supposed to output DRT style. Most of this logic seems encapsulated in TestEventPrinter. The non-test-shell output may be out of date. http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/chromium/TestEventPrinter.cpp When we were upstreaming chromium's DRT, we tried to switch to the common DRT format at the same time, but there were bugs that we failed to track down (random test timeouts) so we cut our losses and just used the old test_shell format. I'm not sure exactly what you'd need to do to try to use the common DRT format. You might be able to just change ChromiumMacPort to inherit from WebKitPort instead of ChromiumPort, but there are probably some additional changes needed.
Created attachment 105092 [details] Patch
Note that if we were to unify the output, we'd also need to make sure that ServerProcess works on windows (win32, not just cygwin). I have no idea what the status of the NRWT-on-win port is. Eric?
Comment on attachment 105092 [details] Patch Clearing flags on attachment: 105092 Committed r93751: <http://trac.webkit.org/changeset/93751>
All reviewed patches have been landed. Closing bug.