From e8708bd7f15549e34e840354b6aa8011f6f41e8b Mon Sep 17 00:00:00 2001 From: Gabriel Bota Date: Mon, 6 May 2024 10:46:19 +0200 Subject: [PATCH] fix or skip tests that are not fit for running on worker threads --- test/common/index.mjs | 2 ++ test/es-module/test-esm-loader-mock.mjs | 7 ++++++- test/es-module/test-esm-named-exports.js | 4 +++- test/es-module/test-esm-named-exports.mjs | 9 +++++---- test/es-module/test-esm-virtual-json.mjs | 3 ++- .../es-module-loaders/builtin-named-exports.mjs | 13 ++++++++----- .../not-found-assert-loader.mjs | 17 +++++++---------- 7 files changed, 33 insertions(+), 22 deletions(-) diff --git a/test/common/index.mjs b/test/common/index.mjs index ca2994f6e1360f..ec9d6ce8b2432c 100644 --- a/test/common/index.mjs +++ b/test/common/index.mjs @@ -50,6 +50,7 @@ const { skipIfDumbTerminal, skipIfEslintMissing, skipIfInspectorDisabled, + skipIfWorker, spawnPromisified, } = common; @@ -104,5 +105,6 @@ export { skipIfDumbTerminal, skipIfEslintMissing, skipIfInspectorDisabled, + skipIfWorker, spawnPromisified, }; diff --git a/test/es-module/test-esm-loader-mock.mjs b/test/es-module/test-esm-loader-mock.mjs index 164d0ac3775039..0d39f549581a54 100644 --- a/test/es-module/test-esm-loader-mock.mjs +++ b/test/es-module/test-esm-loader-mock.mjs @@ -1,6 +1,11 @@ -import '../common/index.mjs'; +import { skipIfWorker } from '../common/index.mjs'; import assert from 'node:assert/strict'; import { mock } from '../fixtures/es-module-loaders/mock.mjs'; +// Importing mock.mjs above will call `register` to modify the loaders chain. +// Modifying the loader chain is not supported currently when running from a worker thread. +// Relevant PR: https://github.com/nodejs/node/pull/52706 +// See comment: https://github.com/nodejs/node/pull/52706/files#r1585144580 +skipIfWorker(); mock('node:events', { EventEmitter: 'This is mocked!' diff --git a/test/es-module/test-esm-named-exports.js b/test/es-module/test-esm-named-exports.js index 2c6f67288aa57c..00b7aebbfd1f46 100644 --- a/test/es-module/test-esm-named-exports.js +++ b/test/es-module/test-esm-named-exports.js @@ -1,7 +1,9 @@ // Flags: --import ./test/fixtures/es-module-loaders/builtin-named-exports.mjs 'use strict'; -require('../common'); +const common = require('../common'); +common.skipIfWorker(); + const { readFile, __fromLoader } = require('fs'); const assert = require('assert'); diff --git a/test/es-module/test-esm-named-exports.mjs b/test/es-module/test-esm-named-exports.mjs index bbe9c96b92d9b8..6e584b05aa204f 100644 --- a/test/es-module/test-esm-named-exports.mjs +++ b/test/es-module/test-esm-named-exports.mjs @@ -1,9 +1,10 @@ // Flags: --import ./test/fixtures/es-module-loaders/builtin-named-exports.mjs -import '../common/index.mjs'; -import { readFile, __fromLoader } from 'fs'; +import { skipIfWorker } from '../common/index.mjs'; +import * as fs from 'fs'; import assert from 'assert'; import ok from '../fixtures/es-modules/test-esm-ok.mjs'; +skipIfWorker(); assert(ok); -assert(readFile); -assert(__fromLoader); +assert(fs.readFile); +assert(fs.__fromLoader); diff --git a/test/es-module/test-esm-virtual-json.mjs b/test/es-module/test-esm-virtual-json.mjs index a42b037fc1f200..1064a6af5026cf 100644 --- a/test/es-module/test-esm-virtual-json.mjs +++ b/test/es-module/test-esm-virtual-json.mjs @@ -1,7 +1,8 @@ -import '../common/index.mjs'; +import { skipIfWorker } from '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; import { register } from 'node:module'; import assert from 'node:assert'; +skipIfWorker(); async function resolve(referrer, context, next) { const result = await next(referrer, context); diff --git a/test/fixtures/es-module-loaders/builtin-named-exports.mjs b/test/fixtures/es-module-loaders/builtin-named-exports.mjs index 123b12c26bf0c9..4e22f631eba416 100644 --- a/test/fixtures/es-module-loaders/builtin-named-exports.mjs +++ b/test/fixtures/es-module-loaders/builtin-named-exports.mjs @@ -1,3 +1,4 @@ +import { isMainThread } from '../../common/index.mjs'; import * as fixtures from '../../common/fixtures.mjs'; import { createRequire, register } from 'node:module'; @@ -10,8 +11,10 @@ Object.defineProperty(globalThis, GET_BUILTIN, { configurable: false, }); -register(fixtures.fileURL('es-module-loaders/builtin-named-exports-loader.mjs'), { - data: { - GET_BUILTIN, - }, -}); +if (isMainThread) { + register(fixtures.fileURL('es-module-loaders/builtin-named-exports-loader.mjs'), { + data: { + GET_BUILTIN, + }, + }); +} diff --git a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs index bf66efbd0810e5..7d53e31df918a7 100644 --- a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs +++ b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs @@ -1,16 +1,13 @@ import assert from 'node:assert'; // A loader that asserts that the defaultResolve will throw "not found" -// (skipping the top-level main of course, and the built-in ones needed for run-worker). -let mainLoad = true; export async function resolve(specifier, { importAttributes }, next) { - if (mainLoad || specifier === 'path' || specifier === 'worker_threads') { - mainLoad = false; - return next(specifier); + if (specifier.startsWith('./not-found')) { + await assert.rejects(next(specifier), { code: 'ERR_MODULE_NOT_FOUND' }); + return { + url: 'node:fs', + importAttributes, + }; } - await assert.rejects(next(specifier), { code: 'ERR_MODULE_NOT_FOUND' }); - return { - url: 'node:fs', - importAttributes, - }; + return next(specifier); }