1244739 - Figure out how to fit reftest assertions into the structured log model

Status: NEW

(Testing :: Reftest, defect)

Description

[Andrew Halberstadt [:ahal]]

We don't have a great way of modeling harness level failures (i.e leaks, assertions) in mozlog. In order to get bug 1034290 finally landed, I punted on this problem, and just made the assertion checks bypass mozlog and dump TEST-UNEXPECTED-FAIL to stdout directly.

More info from bug 1034290 comment 24:
Yeah, looking into this some more I don't really have a good answer. I think a brand new test_start/test_end would be both too confusing and too verbose, and putting this inside the existing test is also a little weird. I see two better options:

1. Turn the first two into logger.error, and the last one into logger.info. This doesn't maintain backwards compat.
2. Log these outside of structured logging with gDumpFn and file a follow-up to revisit this.

I'm leaning towards option 2 because I don't have a ton of time for this, but want to get it landed ASAP while it still works.

Comment 1

[James Graham [:jgraham]]

What's actually wrong with test-start / test-end?

I mean the only other option is to have an action, or set of actions, that works like crash i.e. you output them when something bad happens, but not otherwise.

Comment 2

[Andrew Halberstadt [:ahal]]

These checks happen after each test, so it would make the log quite verbose. It's still an option though, we could potentially make the formatter smart enough to not print them unless they fail. Basically, I don't have time to figure this out properly, but want to get the bulk of the patch landed before it bitrots again. So that's why I'm punting here. It's not that it's terribly difficult or anything.

Probably should have linked the assertions in question:
https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/layout/tools/reftest/reftest.js#1874

Comment 3

[(not currently active) Ted Mielczarek]

Is there a reason we can't just do the checks before we output the test-end, so that they can influence the test result?

Comment 4

[Andrew Halberstadt [:ahal]]

Yeah, I like that solution. But the way reftest.js is setup, this is non-trivial to implement as the call to testEnd happens well before this point (not saying it's hard to do, just don't have time atm).