1973910 - Some tests are comparing strings or different types inside Assert.ok, which is unsupported by Assert's comparison functions

Status: NEW

(Firefox :: General, task, P3)

Description

[:Gijs (he/him)]

As in summary. This is getting flagged up by the lint rule if using ok() and I'm extending that to Assert.ok in bug 1971751. Autofixing to use Assert.greater/less or similar then makes the tests fail because Assert also verifies that the types support comparisons.

We could conceivably update Assert to support strings but that may be unexpected for people coming from node, and feels a little footgunny for consumers who may not realize they're passing strings or arguments with different types (and implicitly relying on JS to convert). We should update the (comparatively small but not 0) tests to be more explicit about what they're doing.

Comment 1

[:Gijs (he/him)]

Some of these tests are actually wrong, cf. https://searchfox.org/mozilla-central/rev/ec8a326713f60dec138a3e3383b03ac739870fc7/toolkit/content/tests/browser/browser_findbar_marks.js#168 where 0 <= -5 <= 10 returns true in JS even though the intent of the test is both to verify that the middle value is >= 0 and that it's below maxMarkPos.

Comment 2

[:Gijs (he/him)]

https://hg-edge.mozilla.org/mozilla-central/rev/40f3facd5a75 and https://searchfox.org/mozilla-central/search?q=bug%201973910 can be used to find the offending callsites. They should be straightforward to fix.