Closed
Bug 1343129
Opened 7 years ago
Closed 7 years ago
Taskcluster external media tests are not returning error codes in case of failure
Categories
(Testing Graveyard :: external-media-tests, defect)
Testing Graveyard
external-media-tests
Tracking
(firefox54 fixed)
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: bryce, Assigned: bryce)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
As noted in Bug 1342449, the task cluster tests are not correctly setting their return code. This results in failures not correctly being picked up by the test systems.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8841848 [details] Bug 1343129 - Update the base media test class to set self.return_code based on the job result parser. https://reviewboard.mozilla.org/r/115938/#review117414 r+wc ::: testing/mozharness/mozharness/mozilla/testing/firefox_media_tests.py:315 (Diff revision 1) > + self.return_code = return_code > return self.job_result_parser.status Right now `self.return_code` is based just on the return code of the process launched by `run_command`. To be based on the parser you want: `self.return_code = self.job_result_parser.status`
Attachment #8841848 -
Flags: review?(mjzffr) → review+
Assignee | ||
Comment 3•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8841848 [details] Bug 1343129 - Update the base media test class to set self.return_code based on the job result parser. https://reviewboard.mozilla.org/r/115938/#review117414 > Right now `self.return_code` is based just on the return code of the process launched by `run_command`. To be based on the parser you want: `self.return_code = self.job_result_parser.status` I believe return code expects to be an int, is there a convention for conversion between the status string and integer return codes? I see buildbot.py contains an EXIT_STATUS_DICT, would it be appropriate to use something like this?
Comment 4•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8841848 [details] Bug 1343129 - Update the base media test class to set self.return_code based on the job result parser. https://reviewboard.mozilla.org/r/115938/#review117414 > I believe return code expects to be an int, is there a convention for conversion between the status string and integer return codes? I see buildbot.py contains an EXIT_STATUS_DICT, would it be appropriate to use something like this? True! My bad. Yes, that would be good and in fact that's was self.buildbot_status does. I think this really boils down to just consolidating the run_media_tests method from FirefoxMediaTestsBuildbot,. This can all go in the base: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/firefox_media_tests_buildbot.py#47-55 Sorry for missing that earlier.
Assignee | ||
Comment 5•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8841848 [details] Bug 1343129 - Update the base media test class to set self.return_code based on the job result parser. https://reviewboard.mozilla.org/r/115938/#review117414 > True! My bad. Yes, that would be good and in fact that's was self.buildbot_status does. I think this really boils down to just consolidating the run_media_tests method from FirefoxMediaTestsBuildbot,. This can all go in the base: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/firefox_media_tests_buildbot.py#47-55 > > Sorry for missing that earlier. Cool, I'll get that sorted and this updated. Appreciate the help!
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8841848 [details] Bug 1343129 - Update the base media test class to set self.return_code based on the job result parser. https://reviewboard.mozilla.org/r/115938/#review117414 > Cool, I'll get that sorted and this updated. Appreciate the help! Updated. Let me know if this looks sensible.
lgtm
Pushed by bvandyk@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b557bdc314e7 Update the base media test class to set self.return_code based on the job result parser. r=maja_zf
Assignee | ||
Comment 10•7 years ago
|
||
Resolving: I'm now seeing failures reported correctly in TH, looks like landing this has had the desired effect.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment 11•7 years ago
|
||
FYI bugs will be marked as fixed once the patch gets merged into mozilla-central by the sheriffs. Reopening until this happened.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 12•7 years ago
|
||
It does appear to have been merged.
Comment 13•7 years ago
|
||
Indeed: https://hg.mozilla.org/mozilla-central/rev/b557bdc314e7 Interesting that there was no comment from pulsebot about that landing on mozilla-central. Maybe there was a temporary outage again. So we can close it.
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Updated•6 years ago
|
Product: Testing → Testing Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•