Summary: | new-run-webkit-tests should upload crash logs | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dirk Pranke <dpranke> | ||||
Component: | Tools / Tests | Assignee: | Adam Barth <abarth> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, aroben, eric, lforschler, mrobinson | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | OS X 10.5 | ||||||
Bug Depends on: | 63468 | ||||||
Bug Blocks: | 34984 | ||||||
Attachments: |
|
If we wrote the code out nicely in webkitpy, we could easily expose commands (as part of webkit-patch or a separate tool) which ORWT could call, so that we could share a single python implementation instead of having two copies. :) Created attachment 98793 [details]
Patch
Comment on attachment 98793 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98793&action=review > Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py:181 > + fs.write_text_file(filename, log if log else error) I just looked at the crashlogs.py module for the first time; does this actually work if you are running multiple DRTs at once? How do you know which pid generated the log? It looks like there is a timestamp and maybe some other number in the crash log filename; is that naming convention configurable, or is there some other way to get a pid into the filename? The pid is in the file. We could parse it out if necessary. I'm not sure we need to associate pid->crashlog yet though. (In reply to comment #4) > The pid is in the file. We could parse it out if necessary. I'm not sure we need to associate pid->crashlog yet though. Well, since there are multiple workers running in parallel, the risk is that you'll end up associating stacks with the wrong tests. I've certainly seen cases where we've had lots (or all) tests crashing at once, but maybe the break is so obvious that you only really need to see one stack to fix the problem? If the pid is in the crash file, it doesn't seem like it would be that much harder to pull it out ... Comment on attachment 98793 [details]
Patch
LGTM. I suspect we'll iterate from here.
There's a slight problem in that ReportCrash takes a very long time to run and the master gets bored and kills it. :( Committed r89873: <http://trac.webkit.org/changeset/89873> Comment on attachment 98793 [details]
Patch
I'm going to leave this bug closed, but there's going to be some follow up work because ReportCrash is super, duper slow and tricks us into think the test timed out.
Have other bugs been filed to cover the remaining work? (Waiting appropriately for ReportCrash, dealing with multiple concurrent crashes, making this work on Windows…) It seems wrong to close this bug if not. |
old-run-webkit-tests now uploads crash logs; we need to modify new-run-webkit-tests to do the same. Quoting from an email to aroben@: On Mar 2, 2011, at 3:44 PM, Dirk Pranke wrote: > What did you need to change to make this happen? What do I need to add > to new-run-webkit-tests? There (thankfully) isn't very much code in old-run-webkit-tests to support this. The functions captureSavedCrashLogs and setUpWindowsCrashLogSaving and their callers/callees, plus the END block, are all that is required.