1667809 - Support failures in parameterized tests

Status: NEW

(Testing :: Mochitest, enhancement)

Description

[Mirko Brodesser (:mbrodesser) -- not reading bugmails currently]

WPT allow adding meta information about specific failures, e.g. https://searchfox.org/mozilla-central/rev/670e13b51d272125c76a1bf84b9f3707583cde12/testing/web-platform/meta/editing/run/inserthtml.html.ini.

Mochitests only allow file-specific fail-if and so on and todo_is so on. However, when a test-file is a test-suite including parameterized tests, fail-if is too restrictive, because the whole suite would be skipped. todo_is not applicable, because for some parameters it should be is.

Was it ever considered to support meta information like for WPT?

Comment 1

[Brian Grinstead [:bgrins]]

(In reply to Mirko Brodesser (:mbrodesser) from comment #0)

WPT allow adding meta information about specific failures, e.g. https://searchfox.org/mozilla-central/rev/670e13b51d272125c76a1bf84b9f3707583cde12/testing/web-platform/meta/editing/run/inserthtml.html.ini.

Mochitests only allow file-specific fail-if and so on and todo_is so on. However, when a test-file is a test-suite including parameterized tests, fail-if is too restrictive, because the whole suite would be skipped. todo_is not applicable, because for some parameters it should be is.

I think I understand the request here, but just to be sure: can you give an example where todo_is isn't suitable in a mochitest?

Comment 2

[Mirko Brodesser (:mbrodesser) -- not reading bugmails currently]

I think I understand the request here, but just to be sure: can you give an example where todo_is isn't suitable in a mochitest?

When the test has the following form:

function runTest(aTestInput) {
  ok(checkSomeProperty(aTestInput), "This property should always be fulfilled.");
}

function runTests() {
  kTestInputs = [
    "1", // Data for the first test.
    "2", // Data for the second test.
   ];

  for (const testInput of kTestInputs) {
    runTest(testInput);
  }
}

If the test should pass for all kTestInputs, but passes only for "1", todo shouldn't be used instead of ok.

Comment 3

[Brian Grinstead [:bgrins]]

Maybe it's because I'm unfamiliar with the syntax for specifying failures in the meta file but it's hard for me to understand exactly what this line is doing: https://searchfox.org/mozilla-central/rev/670e13b51d272125c76a1bf84b9f3707583cde12/testing/web-platform/meta/editing/run/inserthtml.html.ini#4-5. As a reader, it's easier for me to understand what's expected to fail if the expected failures live inside the test itself. For a contrived example, something like:

function runTest(aTestInput) {
  if (aTestInput == "1") { // .. or ["1", "2"].includes(aTestInput)
    todo(checkSomeProperty(aTestInput), "This property should always be fulfilled.");
  } else {
    ok(checkSomeProperty(aTestInput), "This property should always be fulfilled.");
  }
}

Of course storing this in metadata could make it easier to report exactly what is expected to fail. But it's not clear to me if this comes up often enough in mochitests to justify the required work on the test harness / tooling / CI. Because:

• WPT should have a lot more expected failures of individual assertions since we are importing the whole test but may only support a subset of the feature and can't change the test itself to indicate.
• In general we'd like to encourage people to write WPT instead of certain types of mochitests - and maybe the tests that are more likely to want this feature would be better suited as WPT? I'm not sure though - if you had some examples where this has come up as a pain point it could help to scope / prioritize this.

Comment 4

[Mirko Brodesser (:mbrodesser) -- not reading bugmails currently]

(In reply to (Mostly unavailable through October 12) Brian Grinstead [:bgrins] from comment #3)

Maybe it's because I'm unfamiliar with the syntax for specifying failures in the meta file but it's hard for me to understand exactly what this line is doing: https://searchfox.org/mozilla-central/rev/670e13b51d272125c76a1bf84b9f3707583cde12/testing/web-platform/meta/editing/run/inserthtml.html.ini#4-5. As a reader, it's easier for me to understand what's expected to fail if the expected failures live inside the test itself. For a contrived example, something like:

function runTest(aTestInput) {
  if (aTestInput == "1") { // .. or ["1", "2"].includes(aTestInput)
    todo(checkSomeProperty(aTestInput), "This property should always be fulfilled.");
  } else {
    ok(checkSomeProperty(aTestInput), "This property should always be fulfilled.");
  }
}

That's possible, although the test-input can in practice be more complex, bloating the code for the necessary extra checks.

Of course storing this in metadata could make it easier to report exactly what is expected to fail. But it's not clear to me if this comes up often enough in mochitests to justify the required work on the test harness / tooling / CI. Because:

• WPT should have a lot more expected failures of individual assertions since we are importing the whole test but may only support a subset of the feature and can't change the test itself to indicate.
• In general we'd like to encourage people to write WPT instead of certain types of mochitests - and maybe the tests that are more likely to want this feature would be better suited as WPT? I'm not sure though - if you had some examples where this has come up as a pain point it could help to scope / prioritize this.

Agree with both arguments. Note that there are some Gecko-specific features, which hence are not testable via WPT. IIRC, some time ago the idea was to move the Mochitest-framework closer to the WPT-framework, which could be another way to solve this problem while reducing the test-framework code.
I'm also uncertain about the priority of this issue. Feedback from other developers could be helpful.

Comment 5

[James Graham [:jgraham]]

Note that there are some Gecko-specific features, which hence are not testable via WPT.

FWIW I've just put up a review request in bug 1668458 to enable specialPowers in wpt tests that we aren't going to upstream. I think that makes it possible to write almost anything that was a mochitest-plain test as a wpt instead.

Comment 6

[Mirko Brodesser (:mbrodesser) -- not reading bugmails currently]

(In reply to James Graham [:jgraham] from comment #5)

Note that there are some Gecko-specific features, which hence are not testable via WPT.

FWIW I've just put up a review request in bug 1668458 to enable specialPowers in wpt tests that we aren't going to upstream. I think that makes it possible to write almost anything that was a mochitest-plain test as a wpt instead.

Thanks, that's helpful to know, wasn't aware of that work.

I guess that Mochitest-specific test-helpers (e.g. SimpleTest, EventUtils) should move to Gecko-specific WPT-test-helpers too.

I wonder if there's anything left, which can only be tested in mochitest-plain.