Closed Bug 1372567 Opened 3 years ago Closed 3 years ago
Fix problems with structured logger in Test
59 bytes, text/x-review-board-request
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..
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 firstname.lastname@example.org: 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 email@example.com: https://hg.mozilla.org/integration/autoland/rev/0889c455cf63 Fix problems with mochitest structured logging, r=jgraham
You need to log in before you can comment on or make changes to this bug.