731093 - head.js fails to log when an non-object exception is thrown from test

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Justin Dolske [:Dolske]]

Error log from a recent push:

https://tbpl.mozilla.org/php/getParsedLog.php?id=9671643&tree=Mozilla-Inbound&full=1#error4

TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/image/test/unit/test_imgtools.js | test failed (with xpcshell return code: 3), see following log:
>>>>>>>
TEST-INFO | (xpcshell/head.js) | test 1 pending
[...]
TEST-PASS | /home/cltbld/talos-slave/test/build/xpcshell/tests/image/test/unit/test_imgtools.js | [run_test : 174] 1078 == 1078
TEST-PASS | /home/cltbld/talos-slave/test/build/xpcshell/tests/image/test/unit/test_imgtools.js | [compareArrays : 86] 1078 == 1078
/home/cltbld/talos-slave/test/build/xpcshell/head.js:338: TypeError: invalid 'in' operand e
<<<<<<<

Code in head.js was changed a couple months ago:

http://hg.mozilla.org/mozilla-central/rev/c99ee2ec1016

test_imgtools.js appears to be in the middle of running compareArrays(), notices the two arrays differ in content, and throws an exception...

88     for (var i = 0; i < aArray1.length; i++)
89         if (aArray1[i] != aArray2[i])
90             throw "arrays differ at index " + i;
91 }

head.js is this try to do |if ('filename' in "arrays differ at index #")|, and throws an error.

The head.js code should just look for |e.filename| (as it does immediately after with |e.stack|. Or if you want to get all fancy-schmancy, do some typechecking around the whole thing to see if |e| smells like an Object.

And/or just be paranoid and wrap the whole blob in a try/catch. :)

Comment 1

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

Trivial fix.

Test output with fix (I added an explicit throw to the test to force it to fail):

TEST-UNEXPECTED-FAIL | /Users/dolske/build/mozilla-central/obj/_tests/xpcshell/image/test/unit/test_imgtools.js | test failed (with xpcshell return code: 0), see following log:
>>>>>>>
TEST-INFO | (xpcshell/head.js) | test 1 pending
[...]
TEST-PASS | /Users/dolske/build/mozilla-central/obj/_tests/xpcshell/image/test/unit/test_imgtools.js | [run_test : 175] 1078 == 1078
TEST-PASS | /Users/dolske/build/mozilla-central/obj/_tests/xpcshell/image/test/unit/test_imgtools.js | [compareArrays : 86] 1078 == 1078
TEST-UNEXPECTED-FAIL | xpcshell/head.js | FAILED in test #2 -- test encoding a scaled JPEG: poop

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 601153 [details] [diff] [review]
Patch v.1

Ted's out with the new baby. Joel, can you review? This patch was very useful in narrowing down failures we were seeing with the libpng update.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 601153 [details] [diff] [review]
Patch v.1

Review of attachment 601153 [details] [diff] [review]:
-----------------------------------------------------------------

this seems to be more inclusive as we are not just checking for the existence of the variable in the object e, we are ensuring it is not null.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7eb6749bbc17

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7eb6749bbc17