1740041 - Add a linter for conditionals in the first argument to `ok()` and `Assert.ok()`

Status: NEW

(Developer Infrastructure :: Lint and Formatting, task)

Description

[:Gijs (away/busy until Mar 10)]

I just reviewed a patch that did something like this:

ok(a == "5" && b == "6" && c == "7", "Some message")

This is bad because if the assertion fails it will say "TEST-UNEXPECTED-FAIL - got false, expected true - Some message", and that is pretty much useless for figuring out what went wrong, so when you get intermittent failures then (a) the messaging for the summary is rubbish and results in poor matching of failures to intermittents and (b) if it's high-frequency enough that engineering needs to investigate, step 0 will be figuring out what the failing thing even is, to be followed by figuring out why it's failing etc. All of that is annoying and could be improved by better use of the assertion APIs.

This should use 3 is statements that pass a/b/c and their expected counterparts. Or, in the case of my review where a/b/c were array elements, should use Assert.deepEqual to do the array comparisons.

I expect we'll have plenty of pre-existing uses of this, but I wonder if we could add a warning for this pattern to discourage future uses.