Repository navigation
Mocha-like root hooks for test runner #54675
Description
Activity
- addedfeature requestIssues requesting new Node.js features.Issues requesting new Node.js features.
on Aug 31, 2024 Could you write an example (demo code) of what you are looking for vs what is provided?
Edit: Didn't mean to self assign, meant to apply
test_runnerlabel.Reacted by Eric Fortis- addedtest_runnerIssues and PRs related to the test runner subsystem.Issues and PRs related to the test runner subsystem.
on Aug 31, 2024 my-dir/setup.js
import { before, after } from 'node:test' before(() => { console.log('LAUNCH my server only once') }) after(() => { console.log('CLOSE my server only once') })
my-dir/foo.test.js
import { equal } from 'node:assert/strict' import { describe, it } from 'node:test' describe('foo', () => { it('1 is 1', () => equal(1, 1)) })
my-dir/bar.test.js
import { equal } from 'node:assert/strict' import { describe, it } from 'node:test' describe('bar', () => { it('2 is 2', () => equal(2, 2)) })
node --test --import ./setup.js
Its output shows that the
beforeandafterhooks get executed on every file.LAUNCH my server only once CLOSE my server only once ▶ bar ✔ 2 is 2 (0.349167ms) ▶ bar (0.671375ms) LAUNCH my server only once CLOSE my server only once ▶ foo ✔ 1 is 1 (0.367583ms) ▶ foo (0.696583ms) ℹ tests 2 ℹ suites 2 ℹ pass 2 ℹ fail 0 ℹ cancelled 0 ℹ skipped 0 ℹ todo 0 ℹ duration_ms 65.239458The feature I propose is that
beforeandafterhooks that are outsidedescriberun only once, similar to Mocha's global hooks.Desired output:
LAUNCH my server only once ▶ bar ✔ 2 is 2 (0.349167ms) ▶ bar (0.671375ms) ▶ foo ✔ 1 is 1 (0.367583ms) ▶ foo (0.696583ms) ℹ tests 2 ℹ suites 2 ℹ pass 2 ℹ fail 0 ℹ cancelled 0 ℹ skipped 0 ℹ todo 0 ℹ duration_ms 65.239458 CLOSE my server only onceCorrect me if I'm wrong, but wouldn't that be the same just running code before your test and then after without the use of hooks?
@redyetidev That example yes. Here's one that exports something (a browser page instance) from the setup to the tests.
my-dir/setup.js
import { launch } from 'puppeteer' import { before, after } from 'node:test' let browser, page before(async () => { console.log('LAUNCH puppeteer only once') browser = await launch() page = await browser.newPage() }) after(() => { console.log('CLOSE puppeteer only once') browser?.close() }) export default { page }
my-dir/foo.test.js
import { ok } from 'node:assert/strict' import { describe, it } from 'node:test' import P from './setup.js' describe('foo', () => { it('foo', () => ok(P.page.setViewport)) })
my-dir/bar.test.js
import { ok } from 'node:assert/strict' import { describe, it } from 'node:test' import P from './setup.js' describe('bar', () => { it('bar', () => ok(P.page.setViewport)) })
CC @nodejs/test_runner
IMO theres not too much benefit to this feature, as it's pretty-much the equivalent of
runStuffBeforeTesting() test() / run() / ... runStuffAfterTesting()
But WDYT?
The benefit I propose is just speed, consider that
runStuffBeforeTestingcould be slow, such as launching puppeteer. Multiply that by 200 tests.it sounds like you want a beforeAll, if before is actually beforeEach already?
it sounds like you want a beforeAll, if before is actually beforeEach already?
I just don't see the benefit of that versus writing code before and after running tests? But it does seem like that's what is being asked.
@ljharb
Yes, abeforeAll, or a context-sensitivebefore.
Currently, abeforein or outside adescribesuite makes no difference. In Mocha, if it's outside adescribeit's a root hook.
@redyetidev
Consider a setup that exports an instance, such as the second example. In that case, reusing the browser-page on every test saves a significant amount of time. I agree there's no functional benefit to this proposal, it's just a speed one.1 remaining item
Have you tried the new isolation mode added in #53927 and released in Node v22.8.0? I think it would behave as you wantz
FWIW this release is planned for tomorrow, so you won't be able to use this feature today (unless you build from
main)Reacted by Eric FortisI think @mcollina is right here. Also, mocha root hooks are not global when run in parallel mode (which is similar to the default behavior in node:test). If anything, I think we'd want something similar to mocha's global fixtures, which is probably a duplicate of #49732.
Reacted by Eric Fortis, Benjamin Gruenbaum and Moshe AtlowIt looks like
beforeandafteroutside adescribesuite do not get executed.$NODE --test --experimental-test-isolation=none --import ./setup.js --test-concurrency=1 ▶ bar ✔ bar0 (0.526125ms) ✔ bar1 (0.060583ms) ▶ bar (1.666959ms) ▶ foo ✔ foo0 (0.086416ms) ✔ foo1 (0.110709ms) ▶ foo (0.301167ms) ℹ tests 4 ℹ suites 2 ℹ pass 4 ℹ fail 0 ℹ cancelled 0 ℹ skipped 0 ℹ todo 0 ℹ duration_ms 14.778708setup.js
import { before, after } from 'node:test' before(() => { console.log('Before All') }) after( () => { console.log('After All') })
$NODE -v # v23.0.0-pre (0c771c35)
Reacted by Wei ZhuIt works with
-r. There may be a bug specific to--import.Reacted by Eric FortisThis appears to fix the issue related to
--importand the existing test suite still passes:diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 9590ef8dcf..e2237dd73f 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -34,6 +34,7 @@ const { spawn } = require('child_process'); const { finished } = require('internal/streams/end-of-stream'); const { resolve } = require('path'); const { DefaultDeserializer, DefaultSerializer } = require('v8'); +const { getOptionValue } = require('internal/options'); const { Interface } = require('internal/readline/interface'); const { deserializeError } = require('internal/error_serdes'); const { Buffer } = require('buffer'); @@ -697,6 +698,11 @@ function run(options = kEmptyObject) { root.harness.bootstrapPromise = promise; + const userImports = getOptionValue('--import'); + for (let i = 0; i < userImports.length; i++) { + await cascadedLoader.import(userImports[i], parentURL, kEmptyObject); + } + for (let i = 0; i < testFiles.length; ++i) { const testFile = testFiles[i]; const fileURL = pathToFileURL(testFile);
Reacted by Eric Fortis and Benjamin Gruenbaum@cjihrig Thank you! Confirmed.
afterruns before the tests, butbeforeis the important one and works well.$NODE --test --experimental-test-isolation=none --import ./setup.js Before All After All ▶ bar ✔ bar0 (0.312333ms) ✔ bar1 (0.069458ms) ▶ bar (1.144417ms) ▶ foo ✔ foo0 (0.123875ms) ✔ foo1 (0.093833ms) ▶ foo (0.329041ms) ℹ tests 4 ℹ suites 2Reacted by Colin Ihrig and Aviv KellerThat might be a bug within itself.
That might be a bug within itself.
To be clear,
after()running before the tests is a bug.Reacted by Aviv KellerTo be clear,
after()running before the tests is a bug.👍 I'll look into it
Reacted by Colin Ihrig and Eric Fortisafterruns before the tests, butbeforeis the important one and works well.After looking into it, I don't think this is the case.
import * as common from '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; import { test } from 'node:test'; const testArguments = [ '--test', '--experimental-test-isolation=none', ] const testFiles = [ fixtures.path('test-runner', 'no-isolation', 'one.test.js'), fixtures.path('test-runner', 'no-isolation', 'two.test.js'), ] const hookFixture = fixtures.path('test-runner', 'no-isolation', 'global-hooks.js'); const order = [ 'before(): global', 'before one: <root>', 'suite one', 'before two: <root>', 'suite two', 'beforeEach(): global', 'beforeEach one: suite one - test', 'beforeEach two: suite one - test', 'suite one - test', 'afterEach(): global', 'afterEach one: suite one - test', 'afterEach two: suite one - test', 'beforeEach(): global', 'beforeEach one: test one', 'beforeEach two: test one', 'test one', 'afterEach(): global', 'afterEach one: test one', 'afterEach two: test one', 'before suite two: suite two', 'beforeEach(): global', 'beforeEach one: suite two - test', 'beforeEach two: suite two - test', 'suite two - test', 'afterEach(): global', 'afterEach one: suite two - test', 'afterEach two: suite two - test', 'after(): global', 'after one: <root>', 'after two: <root>', ] test('Using --require to define global hooks works', async (t) => { const spawned = await common.spawnPromisified(process.execPath, [ ...testArguments, '--require', hookFixture, ...testFiles ]); t.assert.ok(spawned.stdout.includes(order.join('\n'))); });
I'm opening a PR now to fix the
--importand add this testReacted by Colin Ihrig, Eric Fortis and Toni Villena@redyetidev I see what you meant. I didn't know that importing from the setup.js had no side effects.
Thank you for your assistance.
Reacted by Aviv KellerCan this be closed?
Reacted by Eric FortisI think we can close this in favor of #49732.
Reacted by Moshe Atlow
What is the problem this feature will solve?
Currently,
beforeandafteroutside adescriberun on every file, so projects that could have a global setup and teardown, such as server, database, or puppeteer browser, could run tests faster by executing a globalbeforeandafteronly once.What is the feature you are proposing to solve the problem?
Ensuring
beforeandafteroutside any test run only once, similar to Mocha Rook HooksWhat alternatives have you considered?
Alternatives:
mocha