From 6626cd4df797ada13f486f07870d248f1707aadd Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Wed, 26 Jul 2023 09:14:48 +0300 Subject: [PATCH] test_runner: make enqueue explicit --- lib/internal/test_runner/harness.js | 14 ++++----- lib/internal/test_runner/test.js | 24 ++++++++++----- .../test-runner/output/describe_it.js | 4 +-- .../test-runner/output/describe_it.snapshot | 29 ++++++------------- .../test-runner/output/hooks.snapshot | 6 ---- .../output/junit_reporter.snapshot | 20 ++++++------- 6 files changed, 44 insertions(+), 53 deletions(-) diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 357347627fcc2b..cb4ced46e406ed 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -2,7 +2,6 @@ const { ArrayPrototypeForEach, FunctionPrototypeBind, - PromiseResolve, SafeMap, } = primordials; const { getCallerLocation } = internalBinding('util'); @@ -202,20 +201,21 @@ function getGlobalRoot() { return globalRoot; } -async function startSubtest(subtest) { +async function startSubtest(subtest, parent) { await reportersSetup; getGlobalRoot().harness.bootstrapComplete = true; - await subtest.start(); + if (parent instanceof Suite) { + subtest.enqueue(); + } else { + await subtest.start(); + } } function runInParentContext(Factory) { function run(name, options, fn, overrides) { const parent = testResources.get(executionAsyncId()) || getGlobalRoot(); const subtest = parent.createSubtest(Factory, name, options, fn, overrides); - if (!(parent instanceof Suite)) { - return startSubtest(subtest); - } - return PromiseResolve(); + return startSubtest(subtest, parent); } const test = (name, options, fn) => { diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 083cb2a94a88df..5d9fc7e7a00196 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -318,6 +318,7 @@ class Test extends AsyncResource { this.fn = fn; this.harness = null; // Configured on the root test by the test harness. this.mock = null; + this.enqueued = null; this.cancelled = false; this.skipped = skip !== undefined && skip !== false; this.isTodo = todo !== undefined && todo !== false; @@ -529,18 +530,25 @@ class Test extends AsyncResource { return this.run(); } + enqueue() { + if (this.enqueued) { + return this.enqueued.promise; + } + this.reporter.enqueue(this.nesting, kFilename, this.name); + this.enqueued = createDeferredPromise(); + this.enqueued.test = this; + this.parent.addPendingSubtest(this.enqueued); + return this.enqueued.promise; + } + start() { // If there is enough available concurrency to run the test now, then do // it. Otherwise, return a Promise to the caller and mark the test as // pending for later execution. - this.reporter.enqueue(this.nesting, this.loc, this.name); - if (!this.parent.hasConcurrency()) { - const deferred = createDeferredPromise(); - - deferred.test = this; - this.parent.addPendingSubtest(deferred); - return deferred.promise; + if (this.enqueued || !this.parent.hasConcurrency()) { + return this.enqueue(); } + this.reporter.enqueue(this.nesting, kFilename, this.name); return this.dequeue(); } @@ -938,7 +946,7 @@ class Suite extends Test { } await this.runHook('before', hookArgs); - + this.processPendingSubtests(); stopPromise = stopTest(this.timeout, this.signal); const subtests = this.skipped || this.error ? [] : this.subtests; const promise = SafePromiseAll(subtests, (subtests) => subtests.start()); diff --git a/test/fixtures/test-runner/output/describe_it.js b/test/fixtures/test-runner/output/describe_it.js index ba6a1aed064614..1e9ac7c401b94a 100644 --- a/test/fixtures/test-runner/output/describe_it.js +++ b/test/fixtures/test-runner/output/describe_it.js @@ -290,12 +290,12 @@ describe('subtest sync throw fails', () => { }); describe('describe sync throw fails', () => { - it('should not run', () => {}); + it('should run', () => {}); throw new Error('thrown from describe'); }); describe('describe async throw fails', async () => { - it('should not run', () => {}); + it('should run', () => {}); throw new Error('thrown from describe'); }); diff --git a/test/fixtures/test-runner/output/describe_it.snapshot b/test/fixtures/test-runner/output/describe_it.snapshot index ced1d9b03c7a6f..0466a576642ecd 100644 --- a/test/fixtures/test-runner/output/describe_it.snapshot +++ b/test/fixtures/test-runner/output/describe_it.snapshot @@ -211,9 +211,6 @@ ok 21 - immediate resolve pass * * * - new Promise () - * - * ... # Subtest: mixing describe/it and test should work ok 2 - mixing describe/it and test should work @@ -483,7 +480,7 @@ not ok 50 - custom inspect symbol that throws fail * * * - new Promise () + * * * ... @@ -518,14 +515,10 @@ not ok 51 - subtest sync throw fails code: 'ERR_TEST_FAILURE' ... # Subtest: describe sync throw fails - # Subtest: should not run - not ok 1 - should not run + # Subtest: should run + ok 1 - should run --- - duration_ms: ZERO - location: '/test/fixtures/test-runner/output/describe_it.js:(LINE):3' - failureType: 'cancelledByParent' - error: 'test did not finish before its parent and was cancelled' - code: 'ERR_TEST_FAILURE' + duration_ms: * ... 1..1 not ok 52 - describe sync throw fails @@ -549,14 +542,10 @@ not ok 52 - describe sync throw fails * ... # Subtest: describe async throw fails - # Subtest: should not run - not ok 1 - should not run + # Subtest: should run + ok 1 - should run --- - duration_ms: ZERO - location: '/test/fixtures/test-runner/output/describe_it.js:(LINE):3' - failureType: 'cancelledByParent' - error: 'test did not finish before its parent and was cancelled' - code: 'ERR_TEST_FAILURE' + duration_ms: * ... 1..1 not ok 53 - describe async throw fails @@ -711,9 +700,9 @@ not ok 58 - invalid subtest fail # 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 67 # suites 11 -# pass 31 +# pass 33 # fail 19 -# cancelled 4 +# cancelled 2 # skipped 9 # todo 4 # duration_ms * diff --git a/test/fixtures/test-runner/output/hooks.snapshot b/test/fixtures/test-runner/output/hooks.snapshot index 07dcf8563f7488..c13b5846539153 100644 --- a/test/fixtures/test-runner/output/hooks.snapshot +++ b/test/fixtures/test-runner/output/hooks.snapshot @@ -222,9 +222,6 @@ not ok 5 - afterEach throws * * * - new Promise () - * - * ... # Subtest: 2 ok 2 - 2 @@ -258,9 +255,6 @@ not ok 6 - afterEach when test fails * * * - new Promise () - * - * ... # Subtest: 2 not ok 2 - 2 diff --git a/test/fixtures/test-runner/output/junit_reporter.snapshot b/test/fixtures/test-runner/output/junit_reporter.snapshot index 876882bf7f436c..5fe246c1899b76 100644 --- a/test/fixtures/test-runner/output/junit_reporter.snapshot +++ b/test/fixtures/test-runner/output/junit_reporter.snapshot @@ -19,7 +19,7 @@ * * * - at async startSubtest (node:internal/test_runner/harness:208:3), + at async startSubtest (node:internal/test_runner/harness:210:5), code: 'ERR_TEST_FAILURE' } @@ -37,7 +37,7 @@ * * * - at async startSubtest (node:internal/test_runner/harness:208:3), + at async startSubtest (node:internal/test_runner/harness:210:5), code: 'ERR_TEST_FAILURE' } @@ -62,7 +62,7 @@ * * * - at async startSubtest (node:internal/test_runner/harness:208:3), + at async startSubtest (node:internal/test_runner/harness:210:5), code: 'ERR_TEST_FAILURE' } @@ -83,7 +83,7 @@ * * * - at async startSubtest (node:internal/test_runner/harness:208:3), + at async startSubtest (node:internal/test_runner/harness:210:5), code: 'ERR_TEST_FAILURE' } @@ -101,7 +101,7 @@ * * * - at async startSubtest (node:internal/test_runner/harness:208:3), + at async startSubtest (node:internal/test_runner/harness:210:5), code: 'ERR_TEST_FAILURE' } @@ -148,7 +148,7 @@ true !== false * * * - at async startSubtest (node:internal/test_runner/harness:208:3), + at async startSubtest (node:internal/test_runner/harness:210:5), code: 'ERR_TEST_FAILURE' } @@ -174,7 +174,7 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fail * * * - at Test.dequeue (node:internal/test_runner/test:529:17), + at Test.dequeue (node:internal/test_runner/test:530:17), code: 'ERR_TEST_FAILURE' } @@ -221,7 +221,7 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fail * * * - at async startSubtest (node:internal/test_runner/harness:208:3), + at async startSubtest (node:internal/test_runner/harness:210:5), code: 'ERR_TEST_FAILURE' } @@ -272,7 +272,7 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fail * * * - at async startSubtest (node:internal/test_runner/harness:208:3), + at async startSubtest (node:internal/test_runner/harness:210:5), code: 'ERR_TEST_FAILURE' } @@ -359,7 +359,7 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at first * * * - at Test.dequeue (node:internal/test_runner/test:529:17), + at Test.dequeue (node:internal/test_runner/test:530:17), code: 'ERR_TEST_FAILURE' }