Closed
Bug 688762
Opened 10 years ago
Closed 9 years ago
add try/except blocks around all harnesses so we always return an error code
Categories
(Testing :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla9
People
(Reporter: jmaher, Assigned: jmaher)
References
Details
(Keywords: intermittent-failure, Whiteboard: [mobile_unittests][android_tier_1])
Attachments
(1 file)
2.71 KB,
patch
|
bear
:
review+
|
Details | Diff | Splinter Review |
currently our test harnesses assume that error are handled properly, but we have ran into many cases where we throw and exception and never catch it. This causes us to let tegras run for hours :(
Assignee | ||
Comment 1•10 years ago
|
||
this covers a lot of the scenarios we have ran into in automation. Most likely we will be more Orange than red/purple, but we will REDUCE the amount of idle time we spend on hung tegras. I still need to test this, but I wanted to get it up to get the ball rolling.
Assignee | ||
Comment 2•10 years ago
|
||
I tested these locally and it is working as expected.
Updated•9 years ago
|
Attachment #562046 -
Flags: review?(bear) → review+
Assignee | ||
Comment 3•9 years ago
|
||
inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/3995f7c64af8
Whiteboard: [orange][mobile_unittests][android_tier_1] → [orange][mobile_unittests][android_tier_1][inbound]
Comment 4•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3995f7c64af8
Whiteboard: [orange][mobile_unittests][android_tier_1][inbound] → [orange][mobile_unittests][android_tier_1]
Target Milestone: --- → mozilla9
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 5•9 years ago
|
||
Comment on attachment 562046 [details] [diff] [review] wrap try/except around harness code (1.0) Review of attachment 562046 [details] [diff] [review]: ----------------------------------------------------------------- (I'm just glancing at this, so my comment may be irrelevent, but just in case...) ::: build/mobile/devicemanagerSUT.py @@ +709,5 @@ > + try: > + retVal = self.pullFile(remoteFile) > + except: > + return None > + I would have expected some kind of notice, but I'm not familiar with this file: (bare) 'return None' might be just correct here !? ::: layout/tools/reftest/remotereftest.py @@ +446,5 @@ > # manifest = "http://" + options.remoteWebServer + "/reftests/layout/reftests/reftest-sanity/reftest.list" > + try: > + reftest.runTests(manifest, options) > + except: > + print "TEST-UNEXPECTED-FAIL | | exception while running reftests" Middle data (currently empty) used to be something like 'remotereftest.py'. But maybe you don't do that (anymore) here ? ::: testing/mochitest/runtestsremote.py @@ +347,5 @@ > > + try: > + retVal = mochitest.runTests(options) > + except: > + print "TEST-UNEXPECTED-ERROR | | Exception caught while running tests." Did you actually mean to use 'ERROR' here, whereas it's 'FAIL' everywhere else ?
Comment 6•9 years ago
|
||
(Ftr, should a try+catch be added to non-remote harnesses too, fwiw ?)
Version: unspecified → Trunk
Assignee | ||
Comment 7•9 years ago
|
||
for devicemanager.pullFile we need to return None to let other error handling capture it successfully. We display an error inside of pullFile when we throw an exception. For "TEST-UNEXPECTED-FAIL | | exception while running reftests", I was just copying other errors. Maybe we do need to ensure all the errors in the file have the test file specified. Oh no on the TEST-UNEXPECTED-ERROR; I will need to clean that up. As for desktop harnesses, we probably should add try/catch statements.
Comment 8•9 years ago
|
||
Comment on attachment 562046 [details] [diff] [review] wrap try/except around harness code (1.0) >--- a/build/mobile/devicemanagerSUT.py >+++ b/build/mobile/devicemanagerSUT.py >+ try: >+ retVal = self.pullFile(remoteFile) >+ except: >+ return None >+ Not to forget the trailing whitespace here... >--- a/testing/mochitest/runtestsremote.py >+++ b/testing/mochitest/runtestsremote.py >+ except: >+ print "TEST-UNEXPECTED-ERROR | | Exception caught while running tests." >+ mochitest.stopWebServer(options) >+ mochitest.stopWebSocketServer(options) >+ sys.exit(1) >+ ... and here ... >+ sys.exit(retVal) >+ ... and here.
Updated•8 years ago
|
Keywords: intermittent-failure
Updated•8 years ago
|
Whiteboard: [orange][mobile_unittests][android_tier_1] → [mobile_unittests][android_tier_1]
Updated•8 years ago
|
Component: Infrastructure → General
You need to log in
before you can comment on or make changes to this bug.
Description
•