Test Without an Assertion
ID |
javascript.test_without_assertion |
Severity |
critical |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
JavaScript |
Tags |
assertion, reliability, testing |
Description
Reports a test whose body states no expectation. It exercises the code under test and then accepts whatever came back, so it passes as long as nothing throws: it cannot fail on a wrong answer, only on a crash.
"States an expectation" is answered widely on purpose, because a wrong "no" is a false positive on a test that does check. A known assertion API anywhere in the body counts (expect, assert, should, sinon.assert), so does a call merely named as one (assertRendered(…), verifyPayload(…)), and so does a call to a function declared in the same file whose own body carries either of those — the shape of a suite that delegates its checking to one local helper.
Not reported: a test the source says is not to run (it.skip, test.todo, xit); a declaration with no body at all, which is how Mocha spells a pending test; and a test whose body is empty, which the implicitly_skipped_test rule reports instead.
Rationale
// Bad
it('adds an item to the cart', () => {
const cart = emptyCart();
addItem(cart, 'pen');
});-
The test passes today, and it will keep passing after
addItemstops adding anything at all. -
The suite counts it and the coverage report credits every line it runs, so both instruments say the behaviour is covered.
-
It is worse than no test, because it occupies the place where a reader looks for one: nobody writes the missing test, since a test with that title already exists.
-
When it does eventually fail, it fails on an exception, so the report names an error rather than the expectation that was violated.
Remediation
State the expectation the test was written for.
// Good
it('adds an item to the cart', () => {
const cart = emptyCart();
addItem(cart, 'pen');
expect(total(cart)).toBe(1);
});A test whose only purpose is that a call does not throw should say so, with an assertion that states it:
// Good — the expectation is that nothing is raised
it('accepts an empty catalogue', () => {
expect(() => importCatalogue([])).not.toThrow();
});A suite that imports its assertions from a shared module in another file is the known false positive of this rule: the delegation is only followed inside one file.