Open Bug 1244739 Opened 4 years ago Updated 6 months ago

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

Categories

(Testing :: Reftest, defect)

defect
Not set

Tracking

(Not tracked)

People

(Reporter: ahal, Unassigned)

References

Details

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.
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.
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
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?
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).
You need to log in before you can comment on or make changes to this bug.