Closed Bug 1372567 Opened 3 years ago Closed 3 years ago

Fix problems with structured logger in TestRunner.js

Categories

(Testing :: Mochitest, enhancement)

enhancement
Not set

Tracking

(firefox56 fixed)

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

I was in the midst of writing some tests in bug 1372263 when I started noticing all kinds of weirdness with the structured log. Namely:

1. It's possible for several test_end messages to  be logged
2. Assertions are handled with test_end instead of test_status
3. We always log a test_end with status == "ok" no matter what (even if there were failing test_status messages)

Normally this stuff would have raised flags in mozlog, but it's not due to bug 1372565. We'll need to fix this bug before we can fix that other one.
While the other bug needs to be fixed first, I still needed to write up the patch for this one to properly test it. So might as well get the review going too.
Of course I uploaded the patch to the wrong bug..
Attachment #8877152 - Attachment is obsolete: true
Attachment #8877152 - Flags: review?(james)
Comment on attachment 8880403 [details]
Bug 1372567 - Fix problems with mochitest structured logging,

https://reviewboard.mozilla.org/r/151786/#review156810
Attachment #8880403 - Flags: review?(james) → review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/25838d44520a
Fix problems with mochitest structured logging, r=jgraham
I think there's an assertion that happens in-between those tests. My patch pushes the assertion check slightly earlier, probably early enough that there's a 50% chance the assertion happens after the check and therefore gets assigned to the next test. So we can either move the assertion check back to where it was or else mark both those tests with expected counts of 0-1.

The only reason the assertion check was move before testEnd was so that we could end with status==ASSERT, but that's probably not all that important. I'll move it back to where it was, it's possible there are other assertions that straddle two tests like this one, so it's the safer call.
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0889c455cf63
Fix problems with mochitest structured logging, r=jgraham
https://hg.mozilla.org/mozilla-central/rev/0889c455cf63
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.