Closed Bug 1051775 Opened 10 years ago Closed 6 years ago

Jobs don't fail when outputting the TEST-UNEXPECTED-FAIL for SimpleTest.js throwing on parent.TestRunner

Categories

(Testing :: Mochitest, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: emorley, Unassigned)

References

(Blocks 1 open bug)

Details

As seen in bug 1051157, failures found by the recent harness check added in bug 1049999 aren't turning the run orange - we should fix this :-)

Changing the string dumped may even be sufficient (so mozharness picks it up), though a less hacky fix (that definitely works with structured logging) would be better.

07:37:49     INFO -  22967 INFO TEST-UNEXPECTED-FAIL, Exception caught: Permission denied to access property 'TestRunner', at: http://test1.example.org/tests/SimpleTest/SimpleTest.js (27), location: http://test1.example.org/tests/js/xpconnect/tests/mochitest/test_bug789713.html
07:37:49     INFO -  22968 INFO JavaScript error: http://test1.example.org/tests/SimpleTest/SimpleTest.js, line 48: Permission denied to access property 'TestRunner'


29  dump("TEST-UNEXPECTED-FAIL, Exception caught: " + e.message +
30                ", at: " + e.fileName + " (" + e.lineNumber +
31                "), location: " + window.location.href + "\n");
Blocks: 1049999
Blocks: 1051157
I think some tests also dump TEST-UNEXPECTED-FAIL directly (which wouldn't contribute to the summary) so we definitely have to fix this.

If I understand correctly, mozharness turns the job orange if a TEST-UNEXPECTED... is at the beginning of the line? Could we modify that to also pick up TEST-UNEXPECTED-FAIL that are prefixed by "\d+ INFO "?
(In reply to Ahmed Kachkach [:akachkach] from comment #2)
> I think some tests also dump TEST-UNEXPECTED-FAIL directly (which wouldn't
> contribute to the summary) so we definitely have to fix this.
> 
> If I understand correctly, mozharness turns the job orange if a
> TEST-UNEXPECTED... is at the beginning of the line? Could we modify that to
> also pick up TEST-UNEXPECTED-FAIL that are prefixed by "\d+ INFO "?

I think that's still pretty fragile - ideally the harness would detect these itself, otherwise the failure counts will be wrong etc.
The only place where we can detect the TEST-UNEXPECTED-FAIL logs is on the Python side, and the summary is currently calculated on the JS side.

Errors in TestRunner would use "TestRunner.updateUI([{ result: false }]);" to virtually increment the number of failures after a harness error (and turn the job orange), but this is not possible in this case since the error happens because we can't access TestRunner).

So we don't have much of a choice for now :/ we will have to make things more robust once the dust settles and structured logging is integrated with mozharness (by calculating the summary / detecting errors based on these logs)
I guess something with SpecialPowers could be used in this case, also.
Mass-closing old bugs I filed that have not had recent activity/no longer affect me.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.