1675319 - [manifestparser] Support multi-line skip-ifs

Status: RESOLVED FIXED

(Testing :: Mozbase, enhancement, P2)

Description

[Andrew Halberstadt [:ahal]]

Tests often fail for different reasons on different configurations. Usually we just append a comment and list bugs separate by a comma. But it's not always clear which bug is for which reason. It's sometimes not even clear which parts of the expression belong to which issue.

I propose we try to encourage multi-line skip-if statements. So instead of:

[test_foo]
skip-if = (os == "mac" && debug) || (os == "linux" && !debug) || os == "windows"  # bug 123456, bug 123457, disabled during initial triage

We could write:

[test_foo]
skip-if =
    os == "mac" && debug  # bug 123456
    os == "linux" && !debug  # bug 123457
    os == "windows"  # disabled during initial triage

That way it would be really obvious which condition is disabled for what reason. I think this change would be relatively easy to make in manifestparser.

Going a step further, we could even make the reason part of the skip-if line that gets parsed into the metadata. Then we could have a list of well defined reasons that people could search for using mach test-info. So the above example could become:

[test_foo]
skip-if =
    os == "mac" && debug; intermittent, bug 123456
    os == "linux" && !debug; failure, bug 123457
    os == "windows"; initial-triage, bug 123458

This way the bug and reason metadata would be included in the test object, and would therefore be searchable via tools like mach test-info.

Comment 1

[Andrew Halberstadt [:ahal]]

I should note that it's already technically possible to do this by appending an || to the previous line. E.g:
https://searchfox.org/mozilla-central/source/dom/security/test/mixedcontentblocker/mochitest.ini#24

Although it's not obvious that this works, looks a bit odd and would preclude the "step further" of the previous comment. So I think it's worth supporting this format a little more officially with the implicit ||.

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1675319 - [manifestparser] Properly support multiline skip-if statements, r?jmaher

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ecb229f8ba6f
[manifestparser] Properly support multiline skip-if statements, r=jmaher,extension-reviewers,zombie

Comment 4

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/ecb229f8ba6f