FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: unify --require and --import behavior when isolation none · nodejs/node@2874542 · GitHub

/ node Public

Commit 2874542

Browse files
pmarchini
authored and
targos
committed
test_runner: unify --require and --import behavior when isolation none
PR-URL: #57924 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent 6544663 commit 2874542

6 files changed

Lines changed: 96 additions & 168 deletions

File tree

‎lib/internal/main/test_runner.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ const {
55
} = primordials;
66

77
const {
8-
prepareMainThreadExecution,
98
markBootstrapComplete,
9+
prepareTestRunnerMainExecution,
1010
} = require('internal/process/pre_execution');
1111
const { isUsingInspector } = require('internal/util/inspector');
1212
const { run } = require('internal/test_runner/runner');
@@ -16,10 +16,12 @@ let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
1616
debug = fn;
1717
});
1818

19-
prepareMainThreadExecution(false);
20-
markBootstrapComplete();
21-
2219
const options = parseCommandLine();
20+
const isTestIsolationDisabled = options.isolation === 'none';
21+
// We set initializeModules to false as we want to load user modules in the test runner run function
22+
// if we are running with --test-isolation=none
23+
prepareTestRunnerMainExecution(!isTestIsolationDisabled);
24+
markBootstrapComplete();
2325

2426
if (isUsingInspector() && options.isolation === 'process') {
2527
process.emitWarning('Using the inspector with --test forces running at a concurrency of 1. ' +

‎lib/internal/process/pre_execution.js‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,15 @@ function prepareMainThreadExecution(expandArgv1 = false, initializeModules = tru
5050
});
5151
}
5252

53+
function prepareTestRunnerMainExecution(loadUserModules = true) {
54+
return prepareExecution({
55+
expandArgv1: false,
56+
initializeModules: true,
57+
isMainThread: true,
58+
forceDefaultLoader: !loadUserModules,
59+
});
60+
}
61+
5362
function prepareWorkerThreadExecution() {
5463
prepareExecution({
5564
expandArgv1: false,
@@ -87,7 +96,7 @@ function prepareShadowRealmExecution() {
8796
}
8897

8998
function prepareExecution(options) {
90-
const { expandArgv1, initializeModules, isMainThread } = options;
99+
const { expandArgv1, initializeModules, isMainThread, forceDefaultLoader } = options;
91100

92101
refreshRuntimeOptions();
93102

@@ -147,7 +156,7 @@ function prepareExecution(options) {
147156
}
148157

149158
if (initializeModules) {
150-
setupUserModules();
159+
setupUserModules(forceDefaultLoader);
151160
}
152161

153162
return mainEntry;
@@ -712,6 +721,7 @@ module.exports = {
712721
prepareMainThreadExecution,
713722
prepareWorkerThreadExecution,
714723
prepareShadowRealmExecution,
724+
prepareTestRunnerMainExecution,
715725
markBootstrapComplete,
716726
loadPreloadModules,
717727
initializeFrozenIntrinsics,

‎lib/internal/test_runner/runner.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ const {
8888
const { Glob } = require('internal/fs/glob');
8989
const { once } = require('events');
9090
const { validatePath } = require('internal/fs/utils');
91+
const { loadPreloadModules } = require('internal/process/pre_execution');
9192
const {
9293
triggerUncaughtException,
9394
exitCodes: { kGenericUserError },
@@ -692,6 +693,7 @@ function run(options = kEmptyObject) {
692693
};
693694
const root = createTestTree(rootTestOptions, globalOptions);
694695
let testFiles = files ?? createTestFileList(globPatterns, cwd);
696+
const { isTestRunner } = globalOptions;
695697

696698
if (shard) {
697699
testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
@@ -765,6 +767,16 @@ function run(options = kEmptyObject) {
765767
SafePromiseAllReturnVoid([root.harness.bootstrapPromise, promise]) :
766768
promise;
767769

770+
// We need to setup the user modules in the test runner if we are running with
771+
// --test-isolation=none and --test in order to avoid loading the user modules
772+
// BEFORE the creation of the root test (that would cause them to get lost).
773+
if (isTestRunner) {
774+
// If we are not coming from the test runner entry point, the user-required and imported
775+
// modules have already been loaded.
776+
// Since it's possible to delete modules from require.cache, a CommonJS module
777+
// could otherwise be executed twice.
778+
loadPreloadModules();
779+
}
768780
const userImports = getOptionValue('--import');
769781
for (let i = 0; i < userImports.length; i++) {
770782
await cascadedLoader.import(userImports[i], parentURL, kEmptyObject);

‎test/known_issues/test-runner-no-isolation-hooks.mjs‎

Lines changed: 0 additions & 68 deletions
This file was deleted.

‎test/parallel/test-runner-global-setup-teardown.mjs‎

Lines changed: 54 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -388,11 +388,16 @@ async function runTest(
388388
const cjsPath = join(testFixtures, 'global-setup-teardown', 'required-module.cjs');
389389
const esmpFile = fixtures.fileURL('test-runner', 'global-setup-teardown', 'imported-module.mjs');
390390

391-
it('should run required module before globalSetup', async () => {
391+
// The difference in behavior is due to how --require and --import are handled by
392+
// the main entry point versus the test runner entry point.
393+
// When isolation is 'none', both --require and --import are handled by the test runner.
394+
const shouldRequireAfterSetup = runnerEnabled && isolation === 'none';
395+
const shouldImportAfterSetup = runnerEnabled;
396+
397+
it(`should run required module ${shouldRequireAfterSetup ? 'after' : 'before'} globalSetup`, async () => {
392398
const setupFlagPath = tmpdir.resolve('setup-for-required.tmp');
393399
const teardownFlagPath = tmpdir.resolve('teardown-for-required.tmp');
394400

395-
// Create a setup file for test-file.js to find
396401
fs.writeFileSync(setupFlagPath, '');
397402

398403
const { stdout } = await runTest({
@@ -415,108 +420,63 @@ async function runTest(
415420
assert.match(stdout, /Global setup executed/);
416421
assert.match(stdout, /Global teardown executed/);
417422

418-
// Verify that the required module was executed before the global setup
419423
const requiredExecutedPosition = stdout.indexOf('Required module executed');
420424
const globalSetupExecutedPosition = stdout.indexOf('Global setup executed');
421-
assert.ok(requiredExecutedPosition < globalSetupExecutedPosition,
422-
'Required module should have been executed before global setup');
423425

424-
// After all tests complete, the teardown should have run
426+
assert.ok(
427+
shouldRequireAfterSetup ?
428+
requiredExecutedPosition > globalSetupExecutedPosition :
429+
requiredExecutedPosition < globalSetupExecutedPosition,
430+
`Required module should have been executed ${shouldRequireAfterSetup ? 'after' : 'before'} global setup`
431+
);
432+
425433
assert.ok(fs.existsSync(teardownFlagPath), 'Teardown flag file should exist');
426434
const content = fs.readFileSync(teardownFlagPath, 'utf8');
427435
assert.strictEqual(content, 'Teardown was executed');
428-
429-
// Setup flag should have been removed by teardown
430436
assert.ok(!fs.existsSync(setupFlagPath), 'Setup flag file should have been removed');
431437
});
432438

433-
// This difference in behavior is due to the way --import is being handled by
434-
// run_main entry point or test_runner entry point
435-
if (runnerEnabled) {
436-
it('should run imported module after globalSetup', async () => {
437-
const setupFlagPath = tmpdir.resolve('setup-for-imported.tmp');
438-
const teardownFlagPath = tmpdir.resolve('teardown-for-imported.tmp');
439-
440-
// Create a setup file for test-file.js to find
441-
fs.writeFileSync(setupFlagPath, 'non-empty');
442-
443-
const { stdout } = await runTest({
444-
isolation,
445-
globalSetupFile: 'basic-setup-teardown.mjs',
446-
importPath: './imported-module.js',
447-
env: {
448-
SETUP_FLAG_PATH: setupFlagPath,
449-
TEARDOWN_FLAG_PATH: teardownFlagPath
450-
},
451-
additionalFlags: [
452-
`--import=${esmpFile}`,
453-
],
454-
runnerEnabled
455-
});
456-
457-
assert.match(stdout, /pass 2/);
458-
assert.match(stdout, /fail 0/);
459-
assert.match(stdout, /Imported module executed/);
460-
assert.match(stdout, /Global setup executed/);
461-
assert.match(stdout, /Global teardown executed/);
462-
463-
// Verify that the imported module was executed after the global setup
464-
const globalSetupExecutedPosition = stdout.indexOf('Global setup executed');
465-
const importedExecutedPosition = stdout.indexOf('Imported module executed');
466-
assert.ok(globalSetupExecutedPosition < importedExecutedPosition,
467-
'Imported module should be executed after global setup');
468-
469-
// After all tests complete, the teardown should have run
470-
assert.ok(fs.existsSync(teardownFlagPath), 'Teardown flag file should exist');
471-
const content = fs.readFileSync(teardownFlagPath, 'utf8');
472-
assert.strictEqual(content, 'Teardown was executed');
473-
474-
// Setup flag should have been removed by teardown
475-
assert.ok(!fs.existsSync(setupFlagPath), 'Setup flag file should have been removed');
476-
});
477-
} else {
478-
it('should run imported module before globalSetup', async () => {
479-
const setupFlagPath = tmpdir.resolve('setup-for-imported.tmp');
480-
const teardownFlagPath = tmpdir.resolve('teardown-for-imported.tmp');
481-
482-
// Create a setup file for test-file.js to find
483-
fs.writeFileSync(setupFlagPath, 'non-empty');
484-
485-
const { stdout } = await runTest({
486-
isolation,
487-
globalSetupFile: 'basic-setup-teardown.mjs',
488-
importPath: './imported-module.js',
489-
env: {
490-
SETUP_FLAG_PATH: setupFlagPath,
491-
TEARDOWN_FLAG_PATH: teardownFlagPath
492-
},
493-
additionalFlags: [
494-
`--import=${esmpFile}`,
495-
],
496-
runnerEnabled
497-
});
498-
499-
assert.match(stdout, /pass 2/);
500-
assert.match(stdout, /fail 0/);
501-
assert.match(stdout, /Imported module executed/);
502-
assert.match(stdout, /Global setup executed/);
503-
assert.match(stdout, /Global teardown executed/);
504-
505-
// Verify that the imported module was executed before the global setup
506-
const importedExecutedPosition = stdout.indexOf('Imported module executed');
507-
const globalSetupExecutedPosition = stdout.indexOf('Global setup executed');
508-
assert.ok(importedExecutedPosition < globalSetupExecutedPosition,
509-
'Imported module should be executed before global setup');
510-
511-
// After all tests complete, the teardown should have run
512-
assert.ok(fs.existsSync(teardownFlagPath), 'Teardown flag file should exist');
513-
const content = fs.readFileSync(teardownFlagPath, 'utf8');
514-
assert.strictEqual(content, 'Teardown was executed');
515-
516-
// Setup flag should have been removed by teardown
517-
assert.ok(!fs.existsSync(setupFlagPath), 'Setup flag file should have been removed');
439+
it(`should run imported module ${shouldImportAfterSetup ? 'after' : 'before'} globalSetup`, async () => {
440+
const setupFlagPath = tmpdir.resolve('setup-for-imported.tmp');
441+
const teardownFlagPath = tmpdir.resolve('teardown-for-imported.tmp');
442+
443+
fs.writeFileSync(setupFlagPath, 'non-empty');
444+
445+
const { stdout } = await runTest({
446+
isolation,
447+
globalSetupFile: 'basic-setup-teardown.mjs',
448+
importPath: './imported-module.js',
449+
env: {
450+
SETUP_FLAG_PATH: setupFlagPath,
451+
TEARDOWN_FLAG_PATH: teardownFlagPath
452+
},
453+
additionalFlags: [
454+
`--import=${esmpFile}`,
455+
],
456+
runnerEnabled
518457
});
519-
}
458+
459+
assert.match(stdout, /pass 2/);
460+
assert.match(stdout, /fail 0/);
461+
assert.match(stdout, /Imported module executed/);
462+
assert.match(stdout, /Global setup executed/);
463+
assert.match(stdout, /Global teardown executed/);
464+
465+
const importedExecutedPosition = stdout.indexOf('Imported module executed');
466+
const globalSetupExecutedPosition = stdout.indexOf('Global setup executed');
467+
468+
assert.ok(
469+
shouldImportAfterSetup ?
470+
importedExecutedPosition > globalSetupExecutedPosition :
471+
importedExecutedPosition < globalSetupExecutedPosition,
472+
`Imported module should have been executed ${shouldImportAfterSetup ? 'after' : 'before'} global setup`
473+
);
474+
475+
assert.ok(fs.existsSync(teardownFlagPath), 'Teardown flag file should exist');
476+
const content = fs.readFileSync(teardownFlagPath, 'utf8');
477+
assert.strictEqual(content, 'Teardown was executed');
478+
assert.ok(!fs.existsSync(setupFlagPath), 'Setup flag file should have been removed');
479+
});
520480

521481
it('should execute globalSetup and globalTeardown correctly with imported module containing tests', async () => {
522482
const setupFlagPath = tmpdir.resolve('setup-executed.tmp');

‎test/parallel/test-runner-no-isolation-hooks.mjs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,15 @@ test('use --import (ESM) to define global hooks', async (t) => {
6868

6969
t.assert.equal(testHookOutput, order);
7070
});
71+
72+
test('use --require to define global hooks', async (t) => {
73+
const { stdout } = await common.spawnPromisified(process.execPath, [
74+
...testArguments,
75+
'--require', fixtures.path('test-runner', 'no-isolation', 'global-hooks.cjs'),
76+
...testFiles,
77+
]);
78+
79+
const testHookOutput = stdout.split('\n▶')[0];
80+
81+
t.assert.equal(testHookOutput, order);
82+
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL