Closed
Bug 1120093
Opened 11 years ago
Closed 11 years ago
mailnews/imap/test/unit/test_imapSearch.js, line 260: SyntaxError: test for equality (==) mistyped
Categories
(MailNews Core :: Search, defect)
Tracking
(thunderbird37 fixed)
RESOLVED
FIXED
Thunderbird 38.0
| Tracking | Status | |
|---|---|---|
| thunderbird37 | --- | fixed |
People
(Reporter: aceman, Assigned: aceman)
References
Details
(Keywords: intermittent-failure)
Attachments
(2 files, 1 obsolete file)
|
2.07 KB,
patch
|
rkent
:
review+
standard8
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
|
1.00 KB,
patch
|
Details | Diff | Splinter Review |
Seemingly new failure in test_imapSearch.js.
A JS warning suddenly became a SyntaxError? Maybe a change in the JS engine.
JavaScript strict warning: tbird-bin/_tests/xpcshell/mailnews/imap/test/unit/test_imapSearch.js, line 260: SyntaxError: test for equality (==) mistyped as
assignment (=)?
PROCESS | 13696 | set mailnews.customDBHeaders to x-spam-status oneliner twoliner threeliner nospace withspace
(xpcshell/head.js) | test MAIN run_test pending (1)
(xpcshell/head.js) | test pending (2)
(xpcshell/head.js) | test _async_driver pending (3)
(xpcshell/head.js) | test MAIN run_test finished (3)
running event loop
Error console says [stackFrame SyntaxError: test for equality (==) mistyped as assignment (=)?]
../../../resources/logHelper.js:_errorConsoleTunnel.observe:95
mozilla/testing/xpcshell/head.js:_do_main:208
mozilla/testing/xpcshell/head.js:_execute_test:500
-e:null:1
null:null:0
exiting test
"CONSOLE_MESSAGE: (warn) [JavaScript Warning: "SyntaxError: test for equality (==) mistyped as assignment (=)?" {file: "tbird-bin/_tests/xpcshell/mailnews/imap/test/unit/tes
t_imapSearch.js" line: 260 column: 35 source: " while (test = searchTests.shift())
"}]"
Attachment #8547067 -
Flags: review?(kent)
Attachment #8547067 -
Attachment is obsolete: true
Attachment #8547067 -
Flags: review?(kent)
Attachment #8547068 -
Flags: review?(kent)
Comment 4•11 years ago
|
||
Attachment #8547103 -
Flags: review?(acelists)
Comment 5•11 years ago
|
||
Can't we just do the simpler fix, which is normal for these warnings? See try run:
https://treeherder.mozilla.org/ui/#/jobs?repo=try-comm-central&revision=62af7dd8e24d
I would not understand why there are double brackets and would remove them if I did cleanup in that file ;) I just handle basic JS methods :)
Comment 7•11 years ago
|
||
So the question here is whether if(( test = something())) is recognized as an idiom to avoid the warning. It is seen in calendar code:
http://mxr.mozilla.org/comm-central/source/calendar/providers/gdata/content/gdata-calendar-creation.js#175
loop:
http://mxr.mozilla.org/comm-central/source/mozilla/browser/components/loop/content/shared/libs/lodash-2.4.1.js#1173
devtools:
http://mxr.mozilla.org/comm-central/source/mozilla/browser/devtools/debugger/debugger-controller.js#1949
webapps:
http://mxr.mozilla.org/comm-central/source/mozilla/dom/apps/Webapps.jsm#4060
So I guess I consider this to be the canonical way to allow the perfectly reasonable if (test = something())
Let's see what Magnus thinks.
Updated•11 years ago
|
Attachment #8547103 -
Flags: review?(acelists) → review?(mkmelin+mozilla)
Comment 8•11 years ago
|
||
Comment on attachment 8547103 [details] [diff] [review]
juat fix the warning
Review of attachment 8547103 [details] [diff] [review]:
-----------------------------------------------------------------
I'd argue that aceman's patch is preferable, though I think that this would be even better (so you don't need the shift either):
for (let test of searchTests) {
....
}
With the while loop the "test" is declared outside the smallest scope needed, and IMHO less readable (tough of course technically ok).
Attachment #8547103 -
Flags: review?(mkmelin+mozilla)
| Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•11 years ago
|
Keywords: intermittent-failure
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 29•11 years ago
|
||
Yes, please let's clean up the code especially as aceman has already done the work :-)
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•11 years ago
|
Attachment #8547068 -
Flags: review?(kent) → review+
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
| Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 86•11 years ago
|
||
Comment on attachment 8547068 [details] [diff] [review]
patch v1.1
Checked in https://hg.mozilla.org/comm-central/rev/1ba46100231d
Updated•11 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 38.0
Comment 87•11 years ago
|
||
Comment on attachment 8547068 [details] [diff] [review]
patch v1.1
[Triage Comment]
Please try and remember to see if patches need uplift, especially around merge times - marking this is a+ for aurora as its needed there.
Attachment #8547068 -
Flags: approval-comm-aurora+
Updated•11 years ago
|
status-thunderbird37:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•