Closed Bug 1218453 Opened 4 years ago Closed 4 years ago

Shouldn't check for TALOSDATA if test failed for other reasons

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

defect
Not set

Tracking

(firefox45 fixed, b2g-v2.5 fixed)

RESOLVED FIXED
Tracking Status
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: wlach, Assigned: wlach)

References

Details

Attachments

(1 file, 1 obsolete file)

A likely rather serious and reproducible problem, bug 1205005, looks more like a harmless harness/mozharness issue because "no talosdata" is the topmost error message. Of course there's no talosdata if the harness crashes or times out. :)

Let's fix this.
Assignee: nobody → wlachance
Attachment #8678943 - Flags: review?(jlund)
Attachment #8678943 - Flags: feedback?(j.parkouss)
Two try runs:

(1) Testing the patch above: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6b8b0b40c22
(2) Testing the patch in (1) + intentional removal of TALOSDATA from output (to see if error's still triggered): https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ed102d3517e
(3) Testing the patch in (1) + intentional harness bustage before TALOSDATA printed (to make sure we don't see TALOSDATA if harness failed): https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f81f2c8555c
See Also: → 1205005
Comment on attachment 8678943 [details] [diff] [review]
Only check for talosdata if harness exited ok

Looks good to me.
Attachment #8678943 - Flags: feedback?(j.parkouss) → feedback+
(In reply to William Lachance (:wlach) from comment #2)
> Two try runs:
> 
> (1) Testing the patch above:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6b8b0b40c22
> (2) Testing the patch in (1) + intentional removal of TALOSDATA from output
> (to see if error's still triggered):
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ed102d3517e
> (3) Testing the patch in (1) + intentional harness bustage before TALOSDATA
> printed (to make sure we don't see TALOSDATA if harness failed):
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f81f2c8555c

Verified that all of these worked as expected.
Attachment #8678943 - Attachment is patch: true
Comment on attachment 8678943 [details] [diff] [review]
Only check for talosdata if harness exited ok

Review of attachment 8678943 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm :)
Attachment #8678943 - Flags: review?(jlund) → review+
Hi, this doesn't apply to mozilla-inbound:

renamed 1218453 -> file_1218453.txt
applying file_1218453.txt
patching file testing/mozharness/mozharness/mozilla/testing/talos.py
Hunk #1 FAILED at 402
1 out of 1 hunks FAILED -- saving rejects to file testing/mozharness/mozharness/mozilla/testing/talos.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and refresh file_1218453.txt

could you take a look, thanks!
Flags: needinfo?(wlachance)
Keywords: checkin-needed
Attached patch tpatch.patchSplinter Review
I believe the previous patch had windows line endings or something (can't even imagine why), try this one.
Attachment #8678943 - Attachment is obsolete: true
Flags: needinfo?(wlachance)
Attachment #8680132 - Flags: review+
Checked this in myself ^^^
Status: NEW → ASSIGNED
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b7f2f9f43f4d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
You need to log in before you can comment on or make changes to this bug.