Bug 103830 – nrwt: clean up exception propagation / handling for interrupts and early exits

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dirk Pranke]

nrwt: clean up exception propagation / handling for interrupts and early exits

[Dirk Pranke]

Created attachment 177126 [details]
Patch

[Ojan Vafai]

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.

[WebKit Review Bot]

Comment on attachment 177126 [details]
Patch

Attachment 177126 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15106106

[Dirk Pranke]

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.

[Dirk Pranke]

Committed r136437: <http://trac.webkit.org/changeset/136437>

[Zan Dobersek]

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

[Dirk Pranke]

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!