Bug 1845221 Comment 30 Edit History

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

FWIW I added some logging to try to get to the bottom of who/what is incrementing the failure count:
https://treeherder.mozilla.org/jobs?repo=try&revision=d69ef28d9bcc9a42108f91f506fc53e75d70b9b2

In at least some cases, the issue seems to be from some test logging a result after SimpleTest.finish() is called:
https://treeherder.mozilla.org/logviewer?job_id=425024689&repo=try&lineNumber=3774-3813

This is probably test_link.html since this happens during that test.

Not every failure shows that pattern, but some of them do.  I know we used to (and probably still intend to?) print a log message when this happens, but apparently that's not happening right now (aside from the logging that I'm adding here).

nlapre, it's probably worth looking into how/why that test is still registering more test-results after SimpleTest.finish() is called (maybe it's listening for events and calling `is()`, `ok()` , etc. in those event-handlers; and some events come in and trigger those handlers after the test calls SimpleTest.finish()?)

(I don't know to-what-extent your posted rewrite patch would already address/not-address that issue.)
FWIW I added some logging to try to get to the bottom of who/what is incrementing the failure count:
https://treeherder.mozilla.org/jobs?repo=try&revision=d69ef28d9bcc9a42108f91f506fc53e75d70b9b2

In at least some cases, the issue seems to be from some test logging a result after SimpleTest.finish() is called:
https://treeherder.mozilla.org/logviewer?job_id=425024689&repo=try&lineNumber=3774-3813

(i.e. we're hitting this line which increments the test failure count:
https://searchfox.org/mozilla-central/rev/b4dadcc4c45a2e381a4866d28ebf3a901decccc1/testing/mochitest/tests/SimpleTest/TestRunner.js#854
```js
TestRunner.updateUI([{ result: false }]);
```
)

This is probably test_link.html since this happens during that test.

Not every failure shows that pattern, but some of them do.  I know we used to (and probably still intend to?) print a log message when this happens, but apparently that's not happening right now (aside from the logging that I'm adding here).

nlapre, it's probably worth looking into how/why that test is still registering more test-results after SimpleTest.finish() is called (maybe it's listening for events and calling `is()`, `ok()` , etc. in those event-handlers; and some events come in and trigger those handlers after the test calls SimpleTest.finish()?)

(I don't know to-what-extent your posted rewrite patch would already address/not-address that issue.)
FWIW I added some logging to try to get to the bottom of who/what is incrementing the failure count:
https://treeherder.mozilla.org/jobs?repo=try&revision=d69ef28d9bcc9a42108f91f506fc53e75d70b9b2

In at least some cases, the issue seems to be from some test logging a result after SimpleTest.finish() is called:
https://treeherder.mozilla.org/logviewer?job_id=425024689&repo=try&lineNumber=3774-3813

(i.e. we're hitting this line which increments the test failure count:
https://searchfox.org/mozilla-central/rev/b4dadcc4c45a2e381a4866d28ebf3a901decccc1/testing/mochitest/tests/SimpleTest/TestRunner.js#854
```js
TestRunner.updateUI([{ result: false }]);
```
)

This is probably test_link.html since this happens during that test.

Not every failure triggers my logging for this situation, but some of them (e.g. the log linked in this comment) do seem to be doing this.  I know we used to (and probably still intend to?) print a log message when this happens, but apparently that's not happening right now (aside from the logging that I'm adding here).

nlapre, it's probably worth looking into how/why that test is still registering more test-results after SimpleTest.finish() is called (maybe it's listening for events and calling `is()`, `ok()` , etc. in those event-handlers; and some events come in and trigger those handlers after the test calls SimpleTest.finish()?)

(I don't know to-what-extent your posted rewrite patch would already address/not-address that issue.)

Back to Bug 1845221 Comment 30