66806 2011-08-23 13:41:21 -0700 [chromium] Stacktrace not in test output when a test crashes 2011-08-24 18:02:20 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jamesr jamesr abarth dglazkov dpranke eric tony webkit.review.bot oldest_to_newest 455667 0 jamesr 2011-08-23 13:41:21 -0700 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. 455673 1 jamesr 2011-08-23 13:45:33 -0700 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? 455712 2 eric 2011-08-23 14:26:05 -0700 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? 455715 3 jamesr 2011-08-23 14:29:35 -0700 Why would we write the stderr output into a file instead of reading it directly from the test harness? 455732 4 jamesr 2011-08-23 15:06:00 -0700 https://bugs.webkit.org/show_bug.cgi?id=58708 seems related. 456489 5 dpranke 2011-08-24 15:52:41 -0700 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. 456493 6 jamesr 2011-08-24 15:56:08 -0700 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. 456498 7 105091 jamesr 2011-08-24 15:59:06 -0700 Created attachment 105091 Patch 456503 8 105091 dpranke 2011-08-24 16:00:47 -0700 Comment on attachment 105091 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. 456516 9 tony 2011-08-24 16:19:43 -0700 (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. 456518 10 105092 jamesr 2011-08-24 16:20:44 -0700 Created attachment 105092 Patch 456519 11 dpranke 2011-08-24 16:22:14 -0700 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? 456574 12 105092 webkit.review.bot 2011-08-24 18:02:15 -0700 Comment on attachment 105092 Patch Clearing flags on attachment: 105092 Committed r93751: <http://trac.webkit.org/changeset/93751> 456575 13 webkit.review.bot 2011-08-24 18:02:20 -0700 All reviewed patches have been landed. Closing bug. 105091 2011-08-24 15:59:06 -0700 2011-08-24 16:20:41 -0700 Patch bug-66806-20110824155706.patch text/plain 1800 jamesr U3VidmVyc2lvbiBSZXZpc2lvbjogOTM3NDIKZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBi L1Rvb2xzL0NoYW5nZUxvZwppbmRleCBmZGRlMTRkNDlmZTk5MWMyMzQwOWRkOWFjNWJiOWJiYTAz NDhlOTE3Li40NTUwZGM4YjQyMjBkOTIxYTJjOGViMjU4YWJmYjI5YWFkOTRiYzkzIDEwMDY0NAot LS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTEtMDgtMjQgIEphbWVzIFJvYmluc29uICA8amFtZXNyQGNocm9taXVtLm9yZz4KKwor ICAgICAgICBbY2hyb21pdW1dIFN0YWNrdHJhY2Ugbm90IGluIHRlc3Qgb3V0cHV0IHdoZW4gYSB0 ZXN0IGNyYXNoZXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTY2ODA2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgQXBwZW5kcyB0aGUgLmVycm9yIGFuZCAudGV4dCBvdXRwdXQgd2hlbiBhIHRlc3QgY3Jhc2hl cyBzaW5jZSBvbiBjaHJvbWl1bSB0aGUgLnRleHQgY29udGFpbnMgdGhlIGFjdHVhbCBzdGFjawor ICAgICAgICB0cmFjZS4KKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3Rz L2xheW91dF9wYWNrYWdlL3Rlc3RfcmVzdWx0X3dyaXRlci5weToKKwogMjAxMS0wOC0yNCAgQWRh bSBCYXJ0aCAgPGFiYXJ0aEB3ZWJraXQub3JnPgogCiAgICAgICAgIFJlbW92ZSBlbXB0eSBkaXJl Y3RvcnkuCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9s YXlvdXRfcGFja2FnZS90ZXN0X3Jlc3VsdF93cml0ZXIucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtp dHB5L2xheW91dF90ZXN0cy9sYXlvdXRfcGFja2FnZS90ZXN0X3Jlc3VsdF93cml0ZXIucHkKaW5k ZXggNWFlZTg5YjhhN2NhMWZiZmI2MDBlYzkyNmYwMDRiYzYzODE4MTgwMC4uY2RjZWUwMzkzNDIz YTk5MTdiNzJkOWVkYTBlNThhNzk5MDNhY2UyYiAxMDA2NDQKLS0tIGEvVG9vbHMvU2NyaXB0cy93 ZWJraXRweS9sYXlvdXRfdGVzdHMvbGF5b3V0X3BhY2thZ2UvdGVzdF9yZXN1bHRfd3JpdGVyLnB5 CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL2xheW91dF9wYWNrYWdl L3Rlc3RfcmVzdWx0X3dyaXRlci5weQpAQCAtNjYsNyArNjYsNyBAQCBkZWYgd3JpdGVfdGVzdF9y ZXN1bHQocG9ydCwgdGVzdF9uYW1lLCBkcml2ZXJfb3V0cHV0LAogICAgICAgICAgICAgaWYgZmFp bHVyZS5pc19yZWZ0ZXN0OgogICAgICAgICAgICAgICAgIHdyaXRlci53cml0ZV9jcmFzaF9yZXBv cnQoZXhwZWN0ZWRfZHJpdmVyX291dHB1dC5lcnJvcikKICAgICAgICAgICAgIGVsc2U6Ci0gICAg ICAgICAgICAgICAgd3JpdGVyLndyaXRlX2NyYXNoX3JlcG9ydChkcml2ZXJfb3V0cHV0LmVycm9y KQorICAgICAgICAgICAgICAgIHdyaXRlci53cml0ZV9jcmFzaF9yZXBvcnQoZHJpdmVyX291dHB1 dC5lcnJvciArIHN0cihkcml2ZXJfb3V0cHV0LnRleHQpKQogICAgICAgICBlbGlmIGlzaW5zdGFu Y2UoZmFpbHVyZSwgdGVzdF9mYWlsdXJlcy5GYWlsdXJlUmVmdGVzdE1pc21hdGNoKToKICAgICAg ICAgICAgIHdyaXRlci53cml0ZV9pbWFnZV9maWxlcyhkcml2ZXJfb3V0cHV0LmltYWdlLCBleHBl Y3RlZF9kcml2ZXJfb3V0cHV0LmltYWdlKQogICAgICAgICAgICAgIyBGSVhNRTogVGhpcyB3b3Jr IHNob3VsZCBiZSBkb25lIGVhcmxpZXIgaW4gdGhlIHBpcGVsaW5lIChlLmcuLCB3aGVuIHdlIGNv bXBhcmUgaW1hZ2VzIGZvciBub24tcmVmIHRlc3RzKS4K 105092 2011-08-24 16:20:44 -0700 2011-08-24 18:02:15 -0700 Patch bug-66806-20110824161845.patch text/plain 1819 jamesr U3VidmVyc2lvbiBSZXZpc2lvbjogOTM3NDIKZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBi L1Rvb2xzL0NoYW5nZUxvZwppbmRleCBmZGRlMTRkNDlmZTk5MWMyMzQwOWRkOWFjNWJiOWJiYTAz NDhlOTE3Li42NjUzZWUyMTFkMTRmZDk5Zjc2ZTViNmM4YTZlMzg3NzZmNmZhZGE1IDEwMDY0NAot LS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTEtMDgtMjQgIEphbWVzIFJvYmluc29uICA8amFtZXNyQGNocm9taXVtLm9yZz4KKwor ICAgICAgICBbY2hyb21pdW1dIFN0YWNrdHJhY2Ugbm90IGluIHRlc3Qgb3V0cHV0IHdoZW4gYSB0 ZXN0IGNyYXNoZXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTY2ODA2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgQXBwZW5kcyB0aGUgLmVycm9yIGFuZCAudGV4dCBvdXRwdXQgd2hlbiBhIHRlc3QgY3Jhc2hl cyBzaW5jZSBvbiBjaHJvbWl1bSB0aGUgLnRleHQgY29udGFpbnMgdGhlIGFjdHVhbCBzdGFjawor ICAgICAgICB0cmFjZS4KKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3Rz L3BvcnQvY2hyb21pdW0ucHk6CisKIDIwMTEtMDgtMjQgIEFkYW0gQmFydGggIDxhYmFydGhAd2Vi a2l0Lm9yZz4KIAogICAgICAgICBSZW1vdmUgZW1wdHkgZGlyZWN0b3J5LgpkaWZmIC0tZ2l0IGEv VG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJvbWl1bS5weSBiL1Rv b2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvY2hyb21pdW0ucHkKaW5kZXgg MDUzNmJmOTU3ZTlhNWYwYjQ2ZjMxNDlhZTYzM2YxZGIzNWY2NTI4Ny4uMDcwMzU0ZDQzOGFiMDJi ZDhmNjgzNDZmYTgxMmJkYmJmZWU0N2ZjMCAxMDA3NTUKLS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJr aXRweS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJvbWl1bS5weQorKysgYi9Ub29scy9TY3JpcHRzL3dl YmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2Nocm9taXVtLnB5CkBAIC01NzYsOCArNTc2LDE1IEBA IGNsYXNzIENocm9taXVtRHJpdmVyKERyaXZlcik6CiAgICAgICAgICAgICBpZiBub3QgdGV4dDoK ICAgICAgICAgICAgICAgICB0ZXh0ID0gTm9uZQogCisgICAgICAgIGVycm9yID0gJycuam9pbihl cnJvcikKKyAgICAgICAgIyBDdXJyZW50bHkgdGhlIHN0YWNrdHJhY2UgaXMgaW4gdGhlIHRleHQg b3V0cHV0LCBub3QgZXJyb3IsIHNvIGFwcGVuZCB0aGUgdHdvIHRvZ2V0aGVyIHNvCisgICAgICAg ICMgdGhhdCB3ZSBjYW4gc2VlIHN0YWNrIGluIHRoZSBvdXRwdXQuIFNlZSBodHRwOi8vd2Via2l0 Lm9yZy9iLzY2ODA2CisgICAgICAgICMgRklYTUU6IFdlIHJlYWxseSBzaG91bGQgcHJvcGVybHkg aGFuZGxlIHRoZSBzdGRlcnIgb3V0cHV0IHNlcGFyYXRlbHkuCisgICAgICAgIGlmIGNyYXNoOgor ICAgICAgICAgICAgZXJyb3IgPSBlcnJvciArIHN0cih0ZXh0KQorCiAgICAgICAgIHJldHVybiBE cml2ZXJPdXRwdXQodGV4dCwgb3V0cHV0X2ltYWdlLCBhY3R1YWxfY2hlY2tzdW0sIGF1ZGlvPWF1 ZGlvX2J5dGVzLAotICAgICAgICAgICAgY3Jhc2g9Y3Jhc2gsIHRlc3RfdGltZT1ydW5fdGltZSwg dGltZW91dD10aW1lb3V0LCBlcnJvcj0nJy5qb2luKGVycm9yKSkKKyAgICAgICAgICAgIGNyYXNo PWNyYXNoLCB0ZXN0X3RpbWU9cnVuX3RpbWUsIHRpbWVvdXQ9dGltZW91dCwgZXJyb3I9ZXJyb3Ip CiAKICAgICBkZWYgc3RvcChzZWxmKToKICAgICAgICAgaWYgbm90IHNlbGYuX3Byb2M6Cg==