Summary: | nrwt: clean up exception propagation / handling for interrupts and early exits | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dirk Pranke <dpranke> | ||||
Component: | New Bugs | Assignee: | Dirk Pranke <dpranke> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, dglazkov, eric, ojan, rniwa, tony, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 103824 | ||||||
Attachments: |
|
Description
Dirk Pranke
2012-12-01 19:08:45 PST
Created attachment 177126 [details]
Patch
Comment on attachment 177126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177126&action=review > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:437 > + logging.getLogger().setLevel(logging.DEBUG if options.debug_rwt_logging else logging.INFO) Nit: this doesn't need to be in the try, right? In fact, maybe it makes the most sense right after we do the options parsing? In theory, the other code (e.g. the Host constructor) could log stuff. Comment on attachment 177126 [details] Patch Attachment 177126 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15106106 Comment on attachment 177126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177126&action=review >> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:437 >> + logging.getLogger().setLevel(logging.DEBUG if options.debug_rwt_logging else logging.INFO) > > Nit: this doesn't need to be in the try, right? In fact, maybe it makes the most sense right after we do the options parsing? In theory, the other code (e.g. the Host constructor) could log stuff. Right. Committed r136437: <http://trac.webkit.org/changeset/136437> Comment on attachment 177126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177126&action=review > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:450 > + except: sys.exit(main()) call raises a SystemExit exception which is then again handled here, so a 254 exit code is returned no matter how many tests failed. This affects both BuildBot builders and EWS builders, all of them seemingly failing. I've committed r136474, a simple temporary fix that checks that the exception handled here is not a SystemExit exception. We can address the problem proper when USA wakes up. http://trac.webkit.org/changeset/136474 Comment on attachment 177126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177126&action=review >> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:450 >> + except: > > sys.exit(main()) call raises a SystemExit exception which is then again handled here, so a 254 exit code is returned no matter how many tests failed. This affects both BuildBot builders and EWS builders, all of them seemingly failing. > > I've committed r136474, a simple temporary fix that checks that the exception handled here is not a SystemExit exception. We can address the problem proper when USA wakes up. > http://trac.webkit.org/changeset/136474 whoops. I knew I was going to mess up at least one branch of this somewhere, since of course this is the 5 lines of code I don't have tests for ... This probably should've been: try: return_code = main() except KeyboardInterrupt: return_code = INTERRUPTED_EXIT_STATUS except: return_code = EXCEPTIONAL_EXIT_STATUS sys.exit(return_code) as in the prior version. Thanks for patching this in the meantime! |