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

RESOLVED FIXED in mozilla13

Status

RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Dolske, Assigned: Dolske)

Tracking

unspecified
mozilla13
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
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. :)
(Assignee)

Comment 1

7 years ago
Created attachment 601153 [details] [diff] [review]
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
Assignee: nobody → dolske
Attachment #601153 - Flags: review?(ted.mielczarek)
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.
Attachment #601153 - Flags: review?(ted.mielczarek) → review?(jmaher)
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.
Attachment #601153 - Flags: review?(jmaher) → review+

Comment 5

7 years ago
https://hg.mozilla.org/mozilla-central/rev/7eb6749bbc17
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.