Permanent orange: TEST-UNEXPECTED-FAIL | test_iteratorUtils.js | TypeError: childArray is null

VERIFIED FIXED in Thunderbird 15.0

Status

MailNews Core
Backend
--
blocker
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: standard8, Assigned: mconley)

Tracking

({intermittent-failure, regression})

Trunk
Thunderbird 15.0
intermittent-failure, regression
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [perma-orange])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Since the compartments landing, we've been seeing this failure:

>>>>>>>

TEST-INFO | (xpcshell/head.js) | test 1 pending

TEST-PASS | /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js | [test_toArray_NodeList : 68] rootnode == rootnode

TEST-PASS | /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js | [test_toArray_NodeList : 72] true == true

TEST-UNEXPECTED-FAIL | /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js:74 | TypeError: childArray is null - See following stack:
JS frame :: test_toArray_NodeList@/buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js:74
JS frame :: run_test@/buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js:123
JS frame :: _execute_test@/buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/head.js:326
JS frame :: @-e:1
<<<<<<<
(Reporter)

Comment 1

5 years ago
Links to relevant code:

http://hg.mozilla.org/comm-central/annotate/9b83555adeb2/mailnews/base/util/iteratorUtils.jsm#l58
http://hg.mozilla.org/comm-central/annotate/9b83555adeb2/mailnews/base/test/unit/test_iteratorUtils.js#l74

Changing the function to return just Array.prototype.slice.call(aObj) "worked". Using items like aObj = XPCNativeWrapper.unwrap(aObj) didn't work.
As mentioned in irc, it looks like this test relies on .constructor.name for a dom object, which isn't something that can be relied on. It just happened to work for some reason before CPG.
So if NodeLists aren't nsIDOMNodeLists any more and constructor.name is out, how would you check whether something's a node list or iterator in a way that works across globals?
You could toString the constructor. You could also just duck-type it and expect it to do the right thing.
SM 2.12 has this bug too:

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1336185053.1336188629.28028.gz
WINNT 5.2 comm-central-trunk debug test xpcshell on 2012/05/04 19:30:53
{
TEST-UNEXPECTED-FAIL | e:\builds\slave\test\build\xpcshell\tests\mailnews\base\test\unit\test_iteratorUtils.js | test failed (with xpcshell return code: 0), see following log:
TEST-UNEXPECTED-FAIL | e:/builds/slave/test/build/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js:74 | TypeError: childArray is null - See following stack:
}
Whiteboard: [tb-orange] → [tb-orange] [perma-orange]
Target Milestone: --- → Thunderbird 15.0
Version: 12 → Trunk
(Assignee)

Updated

5 years ago
Assignee: nobody → mconley
(Assignee)

Comment 6

5 years ago
Created attachment 621590 [details] [diff] [review]
Patch v1

Taking bholley's advice and checking the constructor toString.  Test passes with this change.

What do you think, Sid?
Attachment #621590 - Flags: review?(sagarwal)
(Assignee)

Comment 7

5 years ago
Created attachment 621658 [details] [diff] [review]
Patch v2

Sid gave me a review over IRC.  Addressing his comments in this patch, and adding tests for content iterators as requested.
Attachment #621590 - Attachment is obsolete: true
Attachment #621590 - Flags: review?(sagarwal)
Attachment #621658 - Flags: review?(sagarwal)
Comment on attachment 621658 [details] [diff] [review]
Patch v2

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

I haven't run the patch, but r+ assuming tests pass.

::: mail/test/mozmill/utils/test-iteratorUtils.js
@@ +11,5 @@
> +const RELATIVE_ROOT = '../shared-modules';
> +const MODULE_REQUIRES = ['folder-display-helpers',
> +                         'content-tab-helpers',]
> +
> +var iteratorUtils = {};

At this point I think we should exclusively use let for new files.

@@ +27,5 @@
> +
> +  gOriginalWhatsNew = Services.prefs.getCharPref(kWhatsNewPref);
> +  Services.prefs.setCharPref(kWhatsNewPref, gCollectionsUrl);
> +  gTab = open_content_tab_with_click(mc.menus.helpMenu.whatsNew,
> +                                        gCollectionsUrl);

- Please fix the indentation.
- Can we do the open_content_tab outside setupModule?
Attachment #621658 - Flags: review?(sagarwal) → review+
(Assignee)

Comment 9

5 years ago
Created attachment 621685 [details] [diff] [review]
Patch v3 (carrying over r+ from sid0)

Thanks Sid!  Comments addressed in this patch.
Attachment #621658 - Attachment is obsolete: true
Attachment #621685 - Flags: review+
(Assignee)

Comment 10

5 years ago
comm-central: https://hg.mozilla.org/comm-central/rev/945f1b82c5ad
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1336469848.1336473505.24387.gz
WINNT 5.2 comm-central-trunk debug test xpcshell on 2012/05/08 02:37:28

V.Fixed
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Keywords: intermittent-failure
Whiteboard: [tb-orange] [perma-orange] → [perma-orange]
You need to log in before you can comment on or make changes to this bug.