1754015 - Assert.jsm should throw when comparing NaN with another number

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Florian Quèze [:florian]]

Assert.greater(NaN, 0) currently returns true.

I noticed browser_test_procinfo.js was pretty broken (bug 1754012) but still passed, because of this bug. It did something like:

let total = 0;
for (let object of objects) {
  total += object.bogus_property_name;
}
Assert.greater(total, 0);

0 + undefined is NaN.

Any comparison with NaN returns false. But greater is implemented using a lhs <= rhs comparison, which returns false, thus Assert.greater checks always pass when one of the operands is NaN.

compareNumbers does have checks for the type of operands, implemented as typeof lhs == "number", but typeof NaN is "number".

Comment 1

[Florian Quèze [:florian]]

Attachment: Bug 1754015 - Assert.jsm should throw when comparing a NaN number, r=Gijs.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f11731d9aee
Assert.jsm should throw when comparing a NaN number, r=Gijs.

Comment 3

[Cristina Cozmuta (:CrissCozmuta)]

Backed out for causing failures on test_nsIX509CertValidity.js

Backout link : https://hg.mozilla.org/integration/autoland/rev/734a68144915c34759b8fd5fdd63b1e525ad6aa5

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=os%2Cx%2C10.15%2Cwebrender%2Cdebug%2Cxpcshell%2Ctests%2Cwith%2Cnetworking%2Con%2Csocket%2Cprocess%2Ctest-macosx1015-64-qr%2Fdebug-xpcshell-spi-nw-e10s%2Cx2&revision=6928edee3bbcdd3d4a6773cdc379ee9c62ae0f66

Link to failure log: https://treeherder.mozilla.org/logviewer?job_id=367050282&repo=autoland&lineNumber=1638

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5b887e2deaf
Assert.jsm should throw when comparing a NaN number, r=Gijs.

Comment 5

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d5b887e2deaf

Comment 6

[Florian Quèze [:florian]]

(In reply to Cristina Cozmuta (:CrissCozmuta) from comment #3)

Backed out for causing failures on test_nsIX509CertValidity.js

I relanded after this was fixed in bug 1754217.