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

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Mark Banner (:standard8)]

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

Comment 1

[Mark Banner (:standard8)]

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.

Comment 2

[Bobby Holley (:bholley)]

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.

Comment 3

[Rain [:rain1] (inactive)]

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?

Comment 4

[Bobby Holley (:bholley)]

You could toString the constructor. You could also just duck-type it and expect it to do the right thing.

Comment 5

[Serge Gautherie (:sgautherie)]

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:
}

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

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

What do you think, Sid?

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v2

Sid gave me a review over IRC.  Addressing his comments in this patch, and adding tests for content iterators as requested.

Comment 8

[Rain [:rain1] (inactive)]

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?

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v3 (carrying over r+ from sid0)

Thanks Sid!  Comments addressed in this patch.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

comm-central: https://hg.mozilla.org/comm-central/rev/945f1b82c5ad

Comment 11

[Serge Gautherie (:sgautherie)]

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