From 50c854bbfe4d399d4b9c285dce2af4032f5b4836 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 28 Jul 2022 12:54:09 +0300 Subject: [PATCH] test_runner: fix top level `describe` queuing PR-URL: https://github.com/nodejs/node/pull/43998 Reviewed-By: Antoine du Hamel Reviewed-By: Benjamin Gruenbaum --- lib/internal/test_runner/test.js | 17 ++- test/message/test_runner_describe_it.js | 55 +++++++-- test/message/test_runner_describe_it.out | 141 +++++++++++++---------- test/message/test_runner_output.js | 22 ++++ test/message/test_runner_output.out | 23 +++- test/parallel/test-runner-concurrency.js | 39 ++++++- 6 files changed, 208 insertions(+), 89 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 89b8c196038549..14011f0c28ad30 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -561,7 +561,12 @@ class Suite extends Test { try { const context = { signal: this.signal }; - this.buildSuite = this.runInAsyncScope(this.fn, context, [context]); + this.buildSuite = PromisePrototypeThen( + PromiseResolve(this.runInAsyncScope(this.fn, context, [context])), + undefined, + (err) => { + this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure)); + }); } catch (err) { this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure)); } @@ -569,17 +574,9 @@ class Suite extends Test { this.buildPhaseFinished = true; } - start() { - return this.run(); - } - async run() { - try { - await this.buildSuite; - } catch (err) { - this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure)); - } this.parent.activeSubtests++; + await this.buildSuite; this.startTime = hrtime(); if (this[kShouldAbort]()) { diff --git a/test/message/test_runner_describe_it.js b/test/message/test_runner_describe_it.js index c272fb38a749f6..24b83041d4ad03 100644 --- a/test/message/test_runner_describe_it.js +++ b/test/message/test_runner_describe_it.js @@ -149,17 +149,6 @@ describe('level 0a', { concurrency: 4 }, () => { return p0a; }); -describe('top level', { concurrency: 2 }, () => { - it('+long running', async () => { - return new Promise((resolve, reject) => { - setTimeout(resolve, 3000).unref(); - }); - }); - - describe('+short running', async () => { - it('++short running', async () => {}); - }); -}); describe('invalid subtest - pass but subtest fails', () => { setImmediate(() => { @@ -339,3 +328,47 @@ describe('timeouts', () => { setTimeout(done, 10); }); }); + +describe('successful thenable', () => { + it('successful thenable', () => { + let thenCalled = false; + return { + get then() { + if (thenCalled) throw new Error(); + thenCalled = true; + return (successHandler) => successHandler(); + }, + }; + }); + + it('rejected thenable', () => { + let thenCalled = false; + return { + get then() { + if (thenCalled) throw new Error(); + thenCalled = true; + return (_, errorHandler) => errorHandler(new Error('custom error')); + }, + }; + }); + + let thenCalled = false; + return { + get then() { + if (thenCalled) throw new Error(); + thenCalled = true; + return (successHandler) => successHandler(); + }, + }; +}); + +describe('rejected thenable', () => { + let thenCalled = false; + return { + get then() { + if (thenCalled) throw new Error(); + thenCalled = true; + return (_, errorHandler) => errorHandler(new Error('custom error')); + }, + }; +}); diff --git a/test/message/test_runner_describe_it.out b/test/message/test_runner_describe_it.out index 7961345b976f73..cef7f8c94a8098 100644 --- a/test/message/test_runner_describe_it.out +++ b/test/message/test_runner_describe_it.out @@ -24,6 +24,7 @@ not ok 3 - sync fail todo # TODO * * * + * ... # Subtest: sync fail todo with message not ok 4 - sync fail todo with message # TODO this is a failing todo @@ -74,6 +75,7 @@ not ok 8 - sync throw fail * * * + * ... # Subtest: async skip pass ok 9 - async skip pass # SKIP @@ -100,6 +102,7 @@ not ok 11 - async throw fail * * * + * ... # Subtest: async skip fail not ok 12 - async skip fail @@ -128,6 +131,7 @@ not ok 13 - async assertion fail * * * + * ... # Subtest: resolve pass ok 14 - resolve pass @@ -149,6 +153,7 @@ not ok 15 - reject fail * * * + * ... # Subtest: unhandled rejection - passes but warns ok 16 - unhandled rejection - passes but warns @@ -237,45 +242,23 @@ ok 23 - level 0a --- duration_ms: * ... -# Subtest: top level - # Subtest: +long running - ok 1 - +long running - --- - duration_ms: * - ... - # Subtest: +short running - # Subtest: ++short running - ok 1 - ++short running - --- - duration_ms: * - ... - 1..1 - ok 2 - +short running - --- - duration_ms: * - ... - 1..2 -ok 24 - top level - --- - duration_ms: * - ... # Subtest: invalid subtest - pass but subtest fails -ok 25 - invalid subtest - pass but subtest fails +ok 24 - invalid subtest - pass but subtest fails --- duration_ms: * ... # Subtest: sync skip option -ok 26 - sync skip option # SKIP +ok 25 - sync skip option # SKIP --- duration_ms: * ... # Subtest: sync skip option with message -ok 27 - sync skip option with message # SKIP this is skipped +ok 26 - sync skip option with message # SKIP this is skipped --- duration_ms: * ... # Subtest: sync skip option is false fail -not ok 28 - sync skip option is false fail +not ok 27 - sync skip option is false fail --- duration_ms: * failureType: 'testCodeFailure' @@ -291,67 +274,67 @@ not ok 28 - sync skip option is false fail * ... # Subtest: -ok 29 - +ok 28 - --- duration_ms: * ... # Subtest: functionOnly -ok 30 - functionOnly +ok 29 - functionOnly --- duration_ms: * ... # Subtest: -ok 31 - +ok 30 - --- duration_ms: * ... # Subtest: test with only a name provided -ok 32 - test with only a name provided +ok 31 - test with only a name provided --- duration_ms: * ... # Subtest: -ok 33 - +ok 32 - --- duration_ms: * ... # Subtest: -ok 34 - # SKIP +ok 33 - # SKIP --- duration_ms: * ... # Subtest: test with a name and options provided -ok 35 - test with a name and options provided # SKIP +ok 34 - test with a name and options provided # SKIP --- duration_ms: * ... # Subtest: functionAndOptions -ok 36 - functionAndOptions # SKIP +ok 35 - functionAndOptions # SKIP --- duration_ms: * ... # Subtest: escaped description \\ \# \\\#\\ -ok 37 - escaped description \\ \# \\\#\\ +ok 36 - escaped description \\ \# \\\#\\ --- duration_ms: * ... # Subtest: escaped skip message -ok 38 - escaped skip message # SKIP \#skip +ok 37 - escaped skip message # SKIP \#skip --- duration_ms: * ... # Subtest: escaped todo message -ok 39 - escaped todo message # TODO \#todo +ok 38 - escaped todo message # TODO \#todo --- duration_ms: * ... # Subtest: callback pass -ok 40 - callback pass +ok 39 - callback pass --- duration_ms: * ... # Subtest: callback fail -not ok 41 - callback fail +not ok 40 - callback fail --- duration_ms: * failureType: 'testCodeFailure' @@ -362,22 +345,22 @@ not ok 41 - callback fail * ... # Subtest: sync t is this in test -ok 42 - sync t is this in test +ok 41 - sync t is this in test --- duration_ms: * ... # Subtest: async t is this in test -ok 43 - async t is this in test +ok 42 - async t is this in test --- duration_ms: * ... # Subtest: callback t is this in test -ok 44 - callback t is this in test +ok 43 - callback t is this in test --- duration_ms: * ... # Subtest: callback also returns a Promise -not ok 45 - callback also returns a Promise +not ok 44 - callback also returns a Promise --- duration_ms: * failureType: 'callbackAndPromisePresent' @@ -385,7 +368,7 @@ not ok 45 - callback also returns a Promise code: 'ERR_TEST_FAILURE' ... # Subtest: callback throw -not ok 46 - callback throw +not ok 45 - callback throw --- duration_ms: * failureType: 'testCodeFailure' @@ -401,7 +384,7 @@ not ok 46 - callback throw * ... # Subtest: callback called twice -not ok 47 - callback called twice +not ok 46 - callback called twice --- duration_ms: * failureType: 'multipleCallbackInvocations' @@ -412,12 +395,12 @@ not ok 47 - callback called twice * ... # Subtest: callback called twice in different ticks -ok 48 - callback called twice in different ticks +ok 47 - callback called twice in different ticks --- duration_ms: * ... # Subtest: callback called twice in future tick -not ok 49 - callback called twice in future tick +not ok 48 - callback called twice in future tick --- duration_ms: * failureType: 'uncaughtException' @@ -427,7 +410,7 @@ not ok 49 - callback called twice in future tick * ... # Subtest: callback async throw -not ok 50 - callback async throw +not ok 49 - callback async throw --- duration_ms: * failureType: 'uncaughtException' @@ -437,12 +420,12 @@ not ok 50 - callback async throw * ... # Subtest: callback async throw after done -ok 51 - callback async throw after done +ok 50 - callback async throw after done --- duration_ms: * ... # Subtest: custom inspect symbol fail -not ok 52 - custom inspect symbol fail +not ok 51 - custom inspect symbol fail --- duration_ms: * failureType: 'testCodeFailure' @@ -450,7 +433,7 @@ not ok 52 - custom inspect symbol fail code: 'ERR_TEST_FAILURE' ... # Subtest: custom inspect symbol that throws fail -not ok 53 - custom inspect symbol that throws fail +not ok 52 - custom inspect symbol that throws fail --- duration_ms: * failureType: 'testCodeFailure' @@ -501,7 +484,7 @@ not ok 53 - custom inspect symbol that throws fail * ... 1..2 -not ok 54 - subtest sync throw fails +not ok 53 - subtest sync throw fails --- duration_ms: * failureType: 'subtestsFailed' @@ -518,7 +501,7 @@ not ok 54 - subtest sync throw fails code: 'ERR_TEST_FAILURE' ... 1..1 -not ok 55 - describe sync throw fails +not ok 54 - describe sync throw fails --- duration_ms: * failureType: 'testCodeFailure' @@ -546,7 +529,7 @@ not ok 55 - describe sync throw fails code: 'ERR_TEST_FAILURE' ... 1..1 -not ok 56 - describe async throw fails +not ok 55 - describe async throw fails --- duration_ms: * failureType: 'testCodeFailure' @@ -573,7 +556,7 @@ not ok 56 - describe async throw fails error: 'test timed out after 5ms' code: 'ERR_TEST_FAILURE' stack: |- - * + async Promise.all (index 0) ... # Subtest: timed out callback test not ok 2 - timed out callback test @@ -594,15 +577,51 @@ not ok 56 - describe async throw fails duration_ms: * ... 1..4 -not ok 57 - timeouts +not ok 56 - timeouts --- duration_ms: * failureType: 'subtestsFailed' error: '2 subtests failed' code: 'ERR_TEST_FAILURE' ... +# Subtest: successful thenable + # Subtest: successful thenable + ok 1 - successful thenable + --- + duration_ms: * + ... + # Subtest: rejected thenable + not ok 2 - rejected thenable + --- + duration_ms: * + failureType: 'testCodeFailure' + error: 'custom error' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + ... + 1..2 +not ok 57 - successful thenable + --- + duration_ms: * + failureType: 'subtestsFailed' + error: '1 subtest failed' + code: 'ERR_TEST_FAILURE' + ... +# Subtest: rejected thenable +not ok 58 - rejected thenable + --- + duration_ms: * + failureType: 'testCodeFailure' + error: 'custom error' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + ... # Subtest: invalid subtest fail -not ok 58 - invalid subtest fail +not ok 59 - invalid subtest fail --- duration_ms: * failureType: 'parentAlreadyFinished' @@ -611,16 +630,16 @@ not ok 58 - invalid subtest fail stack: |- * ... -1..58 +1..59 # Warning: Test "unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "async unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from async unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "immediate throw - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from immediate throw fail" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. -# tests 58 -# pass 23 -# fail 21 +# tests 59 +# pass 22 +# fail 23 # cancelled 0 # skipped 9 # todo 5 diff --git a/test/message/test_runner_output.js b/test/message/test_runner_output.js index 33e17e6a082ca7..8fce194f56d2b7 100644 --- a/test/message/test_runner_output.js +++ b/test/message/test_runner_output.js @@ -349,3 +349,25 @@ test('large timeout async test is ok', { timeout: 30_000_000 }, async (t) => { test('large timeout callback test is ok', { timeout: 30_000_000 }, (t, done) => { setTimeout(done, 10); }); + +test('successful thenable', () => { + let thenCalled = false; + return { + get then() { + if (thenCalled) throw new Error(); + thenCalled = true; + return (successHandler) => successHandler(); + }, + }; +}); + +test('rejected thenable', () => { + let thenCalled = false; + return { + get then() { + if (thenCalled) throw new Error(); + thenCalled = true; + return (_, errorHandler) => errorHandler('custom error'); + }, + }; +}); diff --git a/test/message/test_runner_output.out b/test/message/test_runner_output.out index 4e987f8e9f4b94..49a19fe302bbb0 100644 --- a/test/message/test_runner_output.out +++ b/test/message/test_runner_output.out @@ -588,8 +588,21 @@ ok 60 - large timeout callback test is ok --- duration_ms: * ... +# Subtest: successful thenable +ok 61 - successful thenable + --- + duration_ms: * + ... +# Subtest: rejected thenable +not ok 62 - rejected thenable + --- + duration_ms: * + failureType: 'testCodeFailure' + error: 'custom error' + code: 'ERR_TEST_FAILURE' + ... # Subtest: invalid subtest fail -not ok 61 - invalid subtest fail +not ok 63 - invalid subtest fail --- duration_ms: * failureType: 'parentAlreadyFinished' @@ -598,16 +611,16 @@ not ok 61 - invalid subtest fail stack: |- * ... -1..61 +1..63 # Warning: Test "unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "async unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from async unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "immediate throw - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from immediate throw fail" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. -# tests 61 -# pass 26 -# fail 18 +# tests 63 +# pass 27 +# fail 19 # cancelled 2 # skipped 10 # todo 5 diff --git a/test/parallel/test-runner-concurrency.js b/test/parallel/test-runner-concurrency.js index 802cff3e9be375..8d756971d68551 100644 --- a/test/parallel/test-runner-concurrency.js +++ b/test/parallel/test-runner-concurrency.js @@ -1,6 +1,6 @@ 'use strict'; -require('../common'); -const { describe, it } = require('node:test'); +const common = require('../common'); +const { describe, it, test } = require('node:test'); const assert = require('assert'); describe('Concurrency option (boolean) = true ', { concurrency: true }, () => { @@ -27,3 +27,38 @@ describe( }); } ); + +{ + // Make sure tests run in order when root concurrency is 1 (default) + const tree = []; + const expectedTestTree = common.mustCall(() => { + assert.deepStrictEqual(tree, [ + 'suite 1', 'nested', 'suite 2', + '1', '2', 'nested 1', 'nested 2', + 'test', 'test 1', 'test 2', + ]); + }); + + describe('suite 1', () => { + tree.push('suite 1'); + it('1', () => tree.push('1')); + it('2', () => tree.push('2')); + + describe('nested', () => { + tree.push('nested'); + it('nested 1', () => tree.push('nested 1')); + it('nested 2', () => tree.push('nested 2')); + }); + }); + + test('test', async (t) => { + tree.push('test'); + await t.test('test1', () => tree.push('test 1')); + await t.test('test 2', () => tree.push('test 2')); + }); + + describe('suite 2', () => { + tree.push('suite 2'); + it('should run after other suites', expectedTestTree); + }); +}