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

Status: RESOLVED INCOMPLETE

(Testing :: Mochitest, defect)

Description

[Ed Morley [:emorley]]

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");

Comment 1

[Ed Morley [:emorley]]

Example logs:
https://tbpl.mozilla.org/php/getParsedLog.php?id=45643910&tree=Mozilla-Central
https://tbpl.mozilla.org/php/getParsedLog.php?id=45657650&tree=Mozilla-Central

Comment 2

[Ahmed Kachkach [:akachkach]]

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 "?

Comment 3

[Ed Morley [:emorley]]

(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.

Comment 4

[Ahmed Kachkach [:akachkach]]

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)

Comment 5

[Martijn Wargers (dead)]

I guess something with SpecialPowers could be used in this case, also.

Comment 6

[Ed Morley [:emorley]]

Mass-closing old bugs I filed that have not had recent activity/no longer affect me.