Open Bug 1399139 Opened 7 years ago Updated 2 years ago

[manifestparser] Built-in support for 'reason' and 'bugs' keys

Categories

(Testing :: Mozbase, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: ahal, Unassigned)

Details

In manifestparser tests can be skipped with 'skip-if' and 'disabled'. Often people do something like:

[test_foo.html]
skip-if = os == 'win'  # Bug 123456 - fails intermittently on windows

The problem with this is that the comment provides no visibility into why the test was skipped to tools. The 'disabled' key is a little better, since at least the message is exposed to tools, but there is no standard for that message. It might be just a bug number, or even blank.

I'm proposing we add some new well-defined built-in keys. One explaining why a test was skipped or disabled. The other listing relevant bugs. So it might look something like:

[test_foo.html]
skip-if = os == 'win'
reason = intermittent
bugs = 1234567, 1765432

The reason key should only accept a discrete list of values and error out if something else is set. I think ideally these keys would be mandatory when skipping a test, but that might be too big of a burden to enable all at once. Maybe they can be optional to start and we can make them mandatory later.

Once this metadata is in the manifest, we can get test harnesses to add it to the structured log (which would make it query-able from ActiveData). The build system could also expose it through the TestResolver via a mach command.
Jgraham mentioned it would be really hard to add this to all the existing annotations (required to make this mandatory). We could have "untriaged" as one of the options for 'reason' and I guess 'bugs' doesn't need to be mandatory.
I think this is something worth pursuing, but solving this will be tricky, getting something in place to enforce this would be interesting.
We could also mass set reason to "unknown" or some other placeholder value to clear the initial hurdle of making the field mandatory.
Priority: -- → P3
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.