Use test plan for compartment tests #316
Conversation
kriskowal
force-pushed
the
kris/plan-compartment-tests
branch
from
c85b132
to
718a7c2
Compare
|
I don't completely get this. What has to be true of an async test so that we can be sure that the test didn't go sideways somewhere? i.e. How did you decide which tests needed |
|
@Chris-Hibbert All tests should have |
| @@ -7,7 +7,7 @@ import { resolveNode, makeNodeImporter } from './node.js'; | |||
|
|
|||
| const { test } = tap; | |||
|
|
|||
| test('import for side effect', async t => { | |||
| test('import for side effect', async _t => { | |||
There was a problem hiding this comment.
All of the tests in this file (from which t.end() were removed) are missing t.plan(). From the description at the top level, They should either retain t.end() (i.e. revert the file), or have t.plan() added.
| /* eslint max-lines: 0 */ | ||
|
|
There was a problem hiding this comment.
The tests in this file look good.
|
Thanks! Sorry for the noise. I’ve gone through the import tests and gotten them all to follow the rule. |
|
But please wait for @Chris-Hibbert 's approval. He knows the testing issues better than I. |
This brings the compartment tests in conformance with the style @Chris-Hibbert and I discussed to best signal test completion and ensure that all planned assertions have an opportunity to fail the test. Using async tests precludes the need for calling
endexplicitly (it is implied by the resolution of the returned promise), butplanis necessary to ensure a test that goes off the rails fails.