Closed
Bug 1263425
Opened 8 years ago
Closed 8 years ago
Crashes in Marionette still finish green (?!)
Categories
(Remote Protocol :: Marionette, defect)
Tracking
(firefox47 fixed, firefox48 fixed, firefox-esr45 fixed)
RESOLVED
FIXED
mozilla48
People
(Reporter: RyanVM, Assigned: impossibus)
Details
Attachments
(1 file)
Take a look at this log, which demonstrates a clear startup crash. Yet the run shows up as green on Treeherder. That seems Really Really Bad. http://archive.mozilla.org/pub/firefox/try-builds/ryanvm@gmail.com-d0996368bb81e116b817887b2582772b44aa317f/try-win32/try_win7-all_test-marionette-e10s-bm111-tests1-windows-build151.txt.gz
Flags: needinfo?(dburns)
Assignee | ||
Comment 1•8 years ago
|
||
Here's what happened. The mozharness script reported 'success' based on data provided by the Marionette runner: the number of failures/errors recorded (0), the numbers of passes (1) and the return code (0 because no exceptions were thrown). In other words, it seems to me that although Marionette runner checks for crashes at several different occasions, it never actually records them as errors or anything (?!). I guess it's counting on an exception being raised by marionette_client after a crash? I will probably have a patch ready tomorrow -- just need to add some unit tests for this aspect of Marionette runner first.
Assignee: nobody → mjzffr
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•8 years ago
|
||
Start-up crash: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e53aa95c8d5 Early crash (as in Ryan's original try run): https://treeherder.mozilla.org/#/jobs?repo=try&revision=3dd4ef738c67 No crash: https://treeherder.mozilla.org/#/jobs?repo=try&revision=940f0fd553db
Assignee | ||
Comment 3•8 years ago
|
||
Cool, it works: http://archive.mozilla.org/pub/firefox/try-builds/mjzffr@gmail.com-3dd4ef738c6759ef6f76fccf4f934f0c1dd5f0c9/try-win32/try_win7-all_test-marionette-bm109-tests1-windows-build236.txt.gz The above log shows the same scenario as Ryan's original report: a crash happens mid-test, but the test is reported as a PASS. However, the crash detection increments the error count, so runner.failed is non-zero, yay!
Flags: needinfo?(dburns)
Assignee | ||
Comment 4•8 years ago
|
||
This addresses the case where there is a crash immediately after a passing test, in which case the test suite stops and no failure is recorded. Review commit: https://reviewboard.mozilla.org/r/47219/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47219/
Attachment #8742475 -
Flags: review?(dburns)
Comment 5•8 years ago
|
||
Comment on attachment 8742475 [details] MozReview Request: Bug 1263425 - Include crashes in Marionette runner's error/failure count; r?AutomatedTester https://reviewboard.mozilla.org/r/47219/#review43883
Attachment #8742475 -
Flags: review?(dburns) → review+
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8b071443fc72
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 9•8 years ago
|
||
David and Maja, we should consider this fix for backporting to beta, release, and esr45. Ryan agreed on that via IRC. David will you do that?
Flags: needinfo?(dburns)
Comment 10•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/dc62a0f70e53
status-firefox47:
--- → fixed
Updated•8 years ago
|
Flags: needinfo?(dburns)
Whiteboard: checkin-needed-esr45
Comment 11•8 years ago
|
||
remote: https://hg.mozilla.org/releases/mozilla-esr45/rev/8820567218bb
status-firefox-esr45:
--- → fixed
Whiteboard: checkin-needed-esr45
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•