Make stability checker check for duplicate test names

[sideshowbarker]

For the context, see #4402 (comment).

[jugglinmike]

Test files with duplicate test names may cause confusion for humans as well. Am I overlooking some use case? Or might we resolve this by making the harness more strict (and correcting any errors this uncovers)?

[jugglinmike]

This condition is detected and reported by testharnessreport.js, so it's apparent to humans executing the test in a browser directly. It doesn't seem be be recognized by the web harness or the wptrunner, though. My plan is to investigate whether the current "harness-runner" protocol allows for communicating pass/fail status by some means other than specific sub-test failures.

[sideshowbarker]

Test files with duplicate test names may cause confusion for humans as well. Am I overlooking some use case?

No use case that I’m aware of.

[jgraham]

My suggestion is to extend the result protocol not to change pass/fail, but to package up warnings, which we can log as warnings (or even as lint errors) from the python side and pick up using the handler in the stability checker script. So we would fail the stability check job if we got unstable results or any warnings from the test harness (although not any warnings in general).

[domenic]

So, the stability checker already checks for this, just not in an intuitive way. At least, it checks for it when the tests are failing. It will tell you that the test is unstable because it failed 40/10 times (when there are 4 tests with the same name). You can see this in action in the Travis jobs for #4559.

EDIT: I now see that the original issue was filed in response to me discovering this same thing the first time around. Nevermind!

[jugglinmike]

(Just saw @domenic's updated comment, but I'll post my response anyway in case it helps others understand the motivation for the solution I have in mind.)

Yup, the is_inconsistent function covers two separate conditions: when there are multiple distinct status values and when the number of results is different than the number of iterations.

The problem is that the number of results is insufficient information to identify duplicated test names. That's because the number can also be effected by unstable test logic, making it possible (though unlikely) that duplicate tests are not identified when those same tests occasionally fail to execute at
all.

By enforcing this restriction at the protocol level (where the test environment can consistently identify duplication), we can both avoid confusion from duplicate test names and limit the sources of variability in the number of results.

[jgraham]

@jugglinmike This is fixed, right?

[bobholt]

Closing as resolved by w3c/testharness.js#240.