Test helper ok() will now fail if used with more than 2 arguments

[Julian Descottes]

We are about to land https://bugzilla.mozilla.org/show_bug.cgi?id=1467712
which changes the behavior of `ok()` test helpers provided by Assert.jsm,
SimpleTest.js and browser-test.js (as well as some test helpers based on
them).

`ok()` used to accept up to four arguments: condition, name,
exception/diagnostic and stack.
After bug 1467712 lands, `ok()`will only accept the 2 first arguments and
will fail the test if more arguments are provided.

The reason behind this change is that `ok()` was regularly used by mistake
instead of `is()`:
ok(value1, value2, "Check value1 == value2");
// passes as long as value1 is truthy and won't detect regressions if
value1 != value2

We recently fixed all the incorrect uses of ok() in the code base in
https://bugzilla.mozilla.org/show_bug.cgi?id=1499096. `is()` is called with
3 arguments, so restricting the number of arguments for ok() will prevent
developers from making similar mistakes in the future.

If you need the old `ok()` with 4 arguments, it has been moved to the
`record()` method and you can use it instead.

[Ehsan Akhgari]

Thank you for doing this! I cannot overstate how many times this has
tripped me and others over in the past.

Cheers,
Ehsan

Post by Julian Descottes
We are about to land https://bugzilla.mozilla.org/show_bug.cgi?id=1467712
which changes the behavior of `ok()` test helpers provided by Assert.jsm,
SimpleTest.js and browser-test.js (as well as some test helpers based on
them).
`ok()` used to accept up to four arguments: condition, name,
exception/diagnostic and stack.
After bug 1467712 lands, `ok()`will only accept the 2 first arguments and
will fail the test if more arguments are provided.
The reason behind this change is that `ok()` was regularly used by mistake
ok(value1, value2, "Check value1 == value2");
// passes as long as value1 is truthy and won't detect regressions if
value1 != value2
We recently fixed all the incorrect uses of ok() in the code base in
https://bugzilla.mozilla.org/show_bug.cgi?id=1499096. `is()` is called with
3 arguments, so restricting the number of arguments for ok() will prevent
developers from making similar mistakes in the future.
If you need the old `ok()` with 4 arguments, it has been moved to the
`record()` method and you can use it instead.
_______________________________________________
dev-platform mailing list
https://lists.mozilla.org/listinfo/dev-platform