Bug 1816549 Comment 20 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I implemented `--compare-preferences` support because it is opt-in, and TV failures are tier 2. In the rollout of bug 1783248 two years ago, there were some failures at first, but that died out once the affected tests fixed up their tests. My try push in comment 4 shows that there are still many plain mochitests that would still fail if run on TV.

A clear negative side effect of [force-enabling the `comparePrefs` option in TV](https://searchfox.org/mozilla-central/rev/311230215f69ac675d0fb4d5c0f5108228f17388/testing/mochitest/runtests.py#3367) is that devs don't get the benefit from the other test coverage in TV, because the test fails early. This issue could be countered by moving the `comparePrefs` option to the last step of test verification ([steps declared here](https://searchfox.org/mozilla-central/rev/311230215f69ac675d0fb4d5c0f5108228f17388/testing/mochitest/runtests.py#3430-3453)), basically introducing another run that sets the `comparePrefs` option to True for that run. To those who really want to have `--compare-preferences` behavior early in TV, we could ensure that `--compare-preferences` is supported, so that someone can run the following locally: `./mach test path/to/mochitest.js --compare-preferences --verify`

While delaying the `comparePrefs` check until the end of TV would help with maximizing the value out of TV jobs, it doesn't magically fix the issue of bugs getting filed due to a TV failure caused by an unexpected preference change. Fixing this properly would be more time consuming, but I see several options that do not alter the test behavior:
- Add a blanket exception for all knowingly failing prefs and make an effort to shrink the set of exceptions over time. With the existing mechanisms, this means adding the prefs from comment 16 to `ignorePrefs.json`.
- Changing the hard failure to a mere warning, to be printed at the end of test verification. When run locally with `--verify`, developers can see the warnings, but it won't cause failures locally nor in CI.
- Setting `comparePrefs` to `False` by default when the test flavor is a browser test (or equivalently, only set it when it is a plain mochitest, because the `--compare-preferences` feature is not implemented for other test types IIUC). That restores the TV behavior to before my patch.

I'm intentionally listing options that do not alter test behavior, because my objective here was implementing `--compare-preferences`, without taking on the task of fixing tons of other tests across the whole code base.

:jmaher, what do you think that we should do here?
I implemented `--compare-preferences` support because it is opt-in, and TV failures are tier 2. In the rollout of bug 1783248 two years ago, there were some failures at first, but that died out once the affected tests fixed up their tests. My try push in comment 4 shows that there are still many plain mochitests that would still fail if run on TV.

EDIT: Although we print `TEST-UNEXPECTED-FAIL` (which appears in the log), the TV job is not interrupted, so the only negative effect is the filing of bug reports for TV failures.
~A clear negative side effect of [force-enabling the `comparePrefs` option in TV](https://searchfox.org/mozilla-central/rev/311230215f69ac675d0fb4d5c0f5108228f17388/testing/mochitest/runtests.py#3367) is that devs don't get the benefit from the other test coverage in TV, because the test fails early. This issue could be countered by moving the `comparePrefs` option to the last step of test verification ([steps declared here](https://searchfox.org/mozilla-central/rev/311230215f69ac675d0fb4d5c0f5108228f17388/testing/mochitest/runtests.py#3430-3453)), basically introducing another run that sets the `comparePrefs` option to True for that run. To those who really want to have `--compare-preferences` behavior early in TV, we could ensure that `--compare-preferences` is supported, so that someone can run the following locally: `./mach test path/to/mochitest.js --compare-preferences --verify`~

While delaying the `comparePrefs` check until the end of TV would help with maximizing the value out of TV jobs, it doesn't magically fix the issue of bugs getting filed due to a TV failure caused by an unexpected preference change. Fixing this properly would be more time consuming, but I see several options that do not alter the test behavior:
- Add a blanket exception for all knowingly failing prefs and make an effort to shrink the set of exceptions over time. With the existing mechanisms, this means adding the prefs from comment 16 to `ignorePrefs.json`.
- Changing the hard failure to a mere warning, to be printed at the end of test verification. When run locally with `--verify`, developers can see the warnings, but it won't cause failures locally nor in CI.
- Setting `comparePrefs` to `False` by default when the test flavor is a browser test (or equivalently, only set it when it is a plain mochitest, because the `--compare-preferences` feature is not implemented for other test types IIUC). That restores the TV behavior to before my patch.

I'm intentionally listing options that do not alter test behavior, because my objective here was implementing `--compare-preferences`, without taking on the task of fixing tons of other tests across the whole code base.

:jmaher, what do you think that we should do here?

Back to Bug 1816549 Comment 20