Code After The done() Handshake
ID |
javascript.test_code_after_done |
Severity |
high |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
JavaScript |
Tags |
async, reliability, testing |
Description
Reports code that follows the done() handshake in a test or hook callback. Calling done() tells the runner the test is over: it records the outcome, tears the fixture down and moves on. The statements after it still execute, but outside the test the reader believes they belong to.
The call is only read as the handshake when its name is a parameter of the enclosing test or hook callback, so a project’s own done() function is not mistaken for it.
Only the statements beside it in the same block count, which is what makes the if (err) { done(err); return; } guard correct rather than reported. A break or a continue reads the same way.
Rationale
// Bad
it('loads the row', (done) => {
fetchRow().then((row) => {
done();
expect(row.id).toBe(7); // decided after the test was recorded as passed
});
});-
The assertion cannot fail the test that contains it: it throws into an already-settled test.
-
The runner either swallows that throw or attributes it to whatever test is running by then, so a real regression surfaces as a confusing failure somewhere else.
-
The file reads as checking
row.idand does not check it, which is exactly the kind of gap a test suite is supposed to rule out. -
Fixture teardown may already have run, so the values the trailing code inspects are not the ones the test set up.
Remediation
Call done() last, after everything the test has to check.
// Good
it('loads the row', (done) => {
fetchRow().then((row) => {
expect(row.id).toBe(7);
done();
});
});