If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

TinderboxPrints for crashes and leak failures broken by bug 855697

RESOLVED FIXED

Status

Release Engineering
General Automation
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: philor, Assigned: emorley)

Tracking

({regression})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
In http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/testing/unittest.py#106 we decide whether we hit a crash, a leak, or a failure to detect leaks, based on the first captured match in http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/testing/errors.py#47

Bug 855697 changed full_regex to match both TEST-UNEXPECTED-FAIL and PROCESS-CRASH, but in the process changed full_harness_match.group(1) from being "application crashed" for a crash to being "PROCESS-CRASH", so now every full_regex match falls through and winds up TinderboxPrinting "LEAK", whether it's a crash or a failure to be able to detect leaks or an actual leak.
(Assignee)

Comment 1

5 years ago
Whoops, sorry :-)
Assignee: nobody → emorley
(Assignee)

Comment 2

5 years ago
Created attachment 732953 [details] [diff] [review]
buildbotcustom v1

Utter fail on my part on the first time round, sorry!
Attachment #732953 - Flags: review?(aki)
(Assignee)

Comment 3

5 years ago
Created attachment 732955 [details] [diff] [review]
mozharness v1
Attachment #732955 - Flags: review?(aki)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED

Comment 4

5 years ago
Hunh, I'm not actually familiar with this construct.

By the summary, I would have guessed the fix would have been to remove "PROCESS-CRASH" from the regex entirely.  How will this solve it?
(Assignee)

Comment 5

5 years ago
In various places, we check that the first group captured/matched by the regexp (ie 'foo_match.group(1)') is "application crashed", "bytes leaked..." etc.

By adding PROCESS-CRASH as part of a group in bug 855697, it bumped the matched group numbers up by one. We don't want to remove PROCESS-CRASH, since it's needed, therefore to fix I can either use '.group(2)' (but that risks me missing some consumers) or else make the newly added group non-capturing by using '(?:foo)', since we don't need the value later anyway.

Updated

5 years ago
Attachment #732953 - Flags: review?(aki) → review+

Updated

5 years ago
Attachment #732955 - Flags: review?(aki) → review+
(Assignee)

Updated

5 years ago
Attachment #732953 - Flags: checked-in+
(Assignee)

Comment 6

5 years ago
Comment on attachment 732955 [details] [diff] [review]
mozharness v1

https://hg.mozilla.org/build/buildbotcustom/rev/4cbab1ecb050
https://hg.mozilla.org/build/mozharness/rev/32d6a6c73123
Attachment #732955 - Flags: checked-in+
in production
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
mozharness change didn't get merged to production...is this all working ok?
(Reporter)

Comment 9

5 years ago
Nope, it's not working, I just thought no part of it had been merged.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 10

5 years ago
Sorry forgot we have a prod branch on mozharness, merged now.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.