Skip to content

Commit 413493c

Browse files
MoLowdanielleadams
authored andcommitted
test_runner: avoid running twice tests in describe
PR-URL: #46888 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 524eaf5 commit 413493c

3 files changed

Lines changed: 29 additions & 28 deletions

File tree

‎doc/api/test.md‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,8 @@ changes:
767767
to this function is a [`TestContext`][] object. If the test uses callbacks,
768768
the callback function is passed as the second argument. **Default:** A no-op
769769
function.
770-
* Returns: {Promise} Resolved with `undefined` once the test completes.
770+
* Returns: {Promise} Resolved with `undefined` once
771+
the test completes, or immediately if the test runs within [`describe()`][].
771772

772773
The `test()` function is the value imported from the `test` module. Each
773774
invocation of this function results in reporting the test to the {TestsStream}.
@@ -776,10 +777,12 @@ The `TestContext` object passed to the `fn` argument can be used to perform
776777
actions related to the current test. Examples include skipping the test, adding
777778
additional diagnostic information, or creating subtests.
778779

779-
`test()` returns a `Promise` that resolves once the test completes. The return
780-
value can usually be discarded for top level tests. However, the return value
781-
from subtests should be used to prevent the parent test from finishing first
782-
and cancelling the subtest as shown in the following example.
780+
`test()` returns a `Promise` that resolves once the test completes.
781+
if `test()` is called within a `describe()` block, it resolve immediately.
782+
The return value can usually be discarded for top level tests.
783+
However, the return value from subtests should be used to prevent the parent
784+
test from finishing first and cancelling the subtest
785+
as shown in the following example.
783786

784787
```js
785788
test('top level test', async (t) => {
@@ -1700,6 +1703,7 @@ added: v18.7.0
17001703
[`context.diagnostic`]: #contextdiagnosticmessage
17011704
[`context.skip`]: #contextskipmessage
17021705
[`context.todo`]: #contexttodomessage
1706+
[`describe()`]: #describename-options-fn
17031707
[`run()`]: #runoptions
17041708
[`test()`]: #testname-options-fn
17051709
[describe options]: #describename-options-fn

‎lib/internal/test_runner/harness.js‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22
const{
33
ArrayPrototypeForEach,
4+
PromiseResolve,
45
SafeMap,
56
}=primordials;
67
const{
@@ -185,31 +186,27 @@ async function startSubtest(subtest) {
185186
awaitsubtest.start();
186187
}
187188

188-
functiontest(name,options,fn){
189-
constparent=testResources.get(executionAsyncId())||getGlobalRoot();
190-
constsubtest=parent.createSubtest(Test,name,options,fn);
191-
returnstartSubtest(subtest);
192-
}
193-
194-
functionrunInParentContext(Factory){
189+
functionrunInParentContext(Factory,addShorthands=true){
195190
functionrun(name,options,fn,overrides){
196191
constparent=testResources.get(executionAsyncId())||getGlobalRoot();
197192
constsubtest=parent.createSubtest(Factory,name,options,fn,overrides);
198-
if(parent===getGlobalRoot()){
199-
startSubtest(subtest);
193+
if(!(parentinstanceofSuite)){
194+
returnstartSubtest(subtest);
200195
}
196+
returnPromiseResolve();
201197
}
202198

203-
constcb=(name,options,fn)=>{
204-
run(name,options,fn);
205-
};
199+
consttest=(name,options,fn)=>run(name,options,fn);
200+
if(!addShorthands){
201+
returntest;
202+
}
206203

207204
ArrayPrototypeForEach(['skip','todo','only'],(keyword)=>{
208-
cb[keyword]=(name,options,fn)=>{
205+
test[keyword]=(name,options,fn)=>{
209206
run(name,options,fn,{[keyword]: true});
210207
};
211208
});
212-
returncb;
209+
returntest;
213210
}
214211

215212
functionhook(hook){
@@ -221,7 +218,7 @@ function hook(hook) {
221218

222219
module.exports={
223220
createTestTree,
224-
test,
221+
test: runInParentContext(Test,false),
225222
describe: runInParentContext(Suite),
226223
it: runInParentContext(Test),
227224
before: hook('before'),

‎test/message/test_runner_hooks.js‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ describe('describe hooks', () => {
3232
});
3333

3434
it('1',()=>testArr.push('1'));
35-
it('2',()=>testArr.push('2'));
35+
test('2',()=>testArr.push('2'));
3636

3737
describe('nested',()=>{
3838
before(function(){
@@ -48,44 +48,44 @@ describe('describe hooks', () => {
4848
testArr.push('afterEach '+this.name);
4949
});
5050
it('nested 1',()=>testArr.push('nested 1'));
51-
it('nested 2',()=>testArr.push('nested 2'));
51+
test('nested 2',()=>testArr.push('nested 2'));
5252
});
5353
});
5454

5555
describe('before throws',()=>{
5656
before(()=>{thrownewError('before');});
5757
it('1',()=>{});
58-
it('2',()=>{});
58+
test('2',()=>{});
5959
});
6060

6161
describe('after throws',()=>{
6262
after(()=>{thrownewError('after');});
6363
it('1',()=>{});
64-
it('2',()=>{});
64+
test('2',()=>{});
6565
});
6666

6767
describe('beforeEach throws',()=>{
6868
beforeEach(()=>{thrownewError('beforeEach');});
6969
it('1',()=>{});
70-
it('2',()=>{});
70+
test('2',()=>{});
7171
});
7272

7373
describe('afterEach throws',()=>{
7474
afterEach(()=>{thrownewError('afterEach');});
7575
it('1',()=>{});
76-
it('2',()=>{});
76+
test('2',()=>{});
7777
});
7878

7979
describe('afterEach when test fails',()=>{
8080
afterEach(common.mustCall(2));
8181
it('1',()=>{thrownewError('test');});
82-
it('2',()=>{});
82+
test('2',()=>{});
8383
});
8484

8585
describe('afterEach throws and test fails',()=>{
8686
afterEach(()=>{thrownewError('afterEach');});
8787
it('1',()=>{thrownewError('test');});
88-
it('2',()=>{});
88+
test('2',()=>{});
8989
});
9090

9191
test('test hooks',async(t)=>{

0 commit comments

Comments
 (0)