Closed Bug 1395116 Opened 2 years ago Closed 2 years ago

TEST-UNEXPECTED-FAIL | mailnews/base/test/unit/test_iteratorUtils.js

Categories

(Thunderbird :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 57.0

People

(Reporter: jorgk-bmo, Assigned: aceman)

Details

Attachments

(1 file)

TEST-UNEXPECTED-FAIL | mailnews/base/test/unit/test_iteratorUtils.js
ReferenceError: Iterator is not defined at /builds/slave/test/build/tests/xpcshell/tests/mailnews/base/test/unit/test_iteratorUtils.js:34

M-C last good: db7f19e26e571ae1dd309f5d2f387b06ba
M-C first bad: ab2d700fda2b4934d24227216972dce9fa

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=db7f19e26e571ae1dd309f5d2f387b06ba&tochange=ab2d700fda2b4934d24227216972dce9fa

Quite obviously: Bug 1098412 - Remove the legacy Iterator constructor

Arai and/or Masatoshi: Can you please advise what we need to do here. Just remove that test?

I also found:

mailnews/base/content/newsError.xhtml
 41 for (let [,piece] in Iterator(query.split("&"))) {

mailnews/base/test/unit/test_iteratorUtils.js

function test_fixIterator() {
 34 let JSIterator = Iterator([1, 2, 3, 4, 5]);
136 let iterator = Iterator(arr);
Flags: needinfo?(arai.unmht)
Flags: needinfo?(VYV03354)
It would be enough to change `Iterator(xxx)` to `xxx`.
Flags: needinfo?(VYV03354)
(In reply to Jorg K (GMT+2) from comment #0)
> mailnews/base/content/newsError.xhtml
>  41 for (let [,piece] in Iterator(query.split("&"))) {

Change to 'for (let piece of query.split("&"))'
 
> mailnews/base/test/unit/test_iteratorUtils.js
> 
> function test_fixIterator() {
>  34 let JSIterator = Iterator([1, 2, 3, 4, 5]);
> 136 let iterator = Iterator(arr);

I'll check if this test is needed. It is quite possible we checked many different constructs that callers could use. But if the syntax is going away, no caller will use it so the test is not needed.
Great, so small bustage only and it's in good hands. Thanks for the replies so far.
Flags: needinfo?(arai.unmht)
I should prepare the patch soon.
Assignee: nobody → acelists
Attached patch patchSplinter Review
This should do it.

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=90f7f3f79d21230aebcd712252ae904ac2c2396e

I changed the test to use generator instead of iterator. It may be useful in the new future.
Attachment #8902909 - Flags: review?(jorgk)
Comment on attachment 8902909 [details] [diff] [review]
patch

Thanks. Since I'm not a JS guru on these things, let's get feedback from Aria.
Attachment #8902909 - Flags: review?(jorgk)
Attachment #8902909 - Flags: review+
Attachment #8902909 - Flags: feedback?(arai.unmht)
Comment on attachment 8902909 [details] [diff] [review]
patch

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

looks good :)
Attachment #8902909 - Flags: feedback?(arai.unmht) → feedback+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/46717004e108
port bug 1098412: Remove uses of Iterator() in JS. r=jorgk,arai
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 57.0
You need to log in before you can comment on or make changes to this bug.