Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: use unusual chars in the path to ensure our tests are robust #48409

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
12 changes: 10 additions & 2 deletions .github/workflows/test-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ jobs:
- uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
with:
persist-credentials: false
path: node
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why node isn't checked out directly into the weird path characters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- name: Set up Python ${{ env.PYTHON_VERSION }}
uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0
with:
Expand All @@ -51,6 +52,13 @@ jobs:
- name: Environment Information
run: npx envinfo
- name: Build
run: make build-ci -j4 V=1 CONFIG_FLAGS="--error-on-warn"
run: make -C node build-ci -j4 V=1 CONFIG_FLAGS="--error-on-warn"
- name: Test
run: make run-ci -j4 V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9"
run: make -C node run-ci -j4 V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9"
- name: Re-run test in a folder whose name contains unusual chars
run: |
mv node "$DIR"
cd "$DIR"
./tools/test.py --flaky-tests keep_retrying -p actions -j 4
env:
DIR: dir%20with $unusual"chars?'åß∂ƒ©∆¬…`
14 changes: 11 additions & 3 deletions .github/workflows/test-macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ jobs:
- uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
with:
persist-credentials: false
path: node
- name: Set up Python ${{ env.PYTHON_VERSION }}
uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0
with:
Expand All @@ -64,7 +65,7 @@ jobs:
# happen anymore running this step here first, that's also useful
# information.)
- name: tools/doc/node_modules workaround
run: make tools/doc/node_modules
run: make -C node tools/doc/node_modules
# This is needed due to https://github.com/nodejs/build/issues/3878
- name: Cleanup
run: |
Expand All @@ -80,8 +81,15 @@ jobs:
df -h
echo "::endgroup::"
- name: Build
run: make build-ci -j$(getconf _NPROCESSORS_ONLN) V=1 CONFIG_FLAGS="--error-on-warn"
run: make -C node build-ci -j$(getconf _NPROCESSORS_ONLN) V=1 CONFIG_FLAGS="--error-on-warn"
- name: Free Space After Build
run: df -h
- name: Test
run: make run-ci -j$(getconf _NPROCESSORS_ONLN) V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9"
run: make -C node run-ci -j$(getconf _NPROCESSORS_ONLN) V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9"
- name: Re-run test in a folder whose name contains unusual chars
run: |
mv node "$DIR"
cd "$DIR"
./tools/test.py --flaky-tests keep_retrying -p actions -j 4
env:
DIR: dir%20with $unusual"chars?'åß∂ƒ©∆¬…`
9 changes: 9 additions & 0 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,15 @@ const common = {
get checkoutEOL() {
return fs.readFileSync(__filename).includes('\r\n') ? '\r\n' : '\n';
},

get isInsideDirWithUnusualChars() {
return __dirname.includes('%') ||
(!isWindows && __dirname.includes('\\')) ||
__dirname.includes('$') ||
__dirname.includes('\n') ||
__dirname.includes('\r') ||
__dirname.includes('\t');
},
};

const validProperties = new Set(Object.keys(common));
Expand Down
2 changes: 2 additions & 0 deletions test/common/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const {
isDumbTerminal,
isFreeBSD,
isIBMi,
isInsideDirWithUnusualChars,
isLinux,
isLinuxPPCBE,
isMainThread,
Expand Down Expand Up @@ -81,6 +82,7 @@ export {
isDumbTerminal,
isFreeBSD,
isIBMi,
isInsideDirWithUnusualChars,
isLinux,
isLinuxPPCBE,
isMainThread,
Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-invalid-pjson.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('ESM: Package.json', { concurrency: !process.env.TEST_PARALLEL }, () =>
assert.ok(stderr.includes('code: \'ERR_INVALID_PACKAGE_CONFIG\''), stderr);
assert.ok(
stderr.includes(
`Invalid package config ${path.toNamespacedPath(invalidJson)} while importing "invalid-pjson" from ${entry}.`
`Invalid package config ${path.toNamespacedPath(invalidJson)} while importing "invalid-pjson" from ${path.toNamespacedPath(entry)}.`
),
stderr
);
Expand Down
3 changes: 2 additions & 1 deletion test/fixtures/snapshot/child-process-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ const {
function spawn() {
const { spawnSync, execFileSync, execSync } = require('child_process');
spawnSync(process.execPath, [ __filename, 'spawnSync' ], { stdio: 'inherit' });
execSync(`"${process.execPath}" "${__filename}" "execSync"`, { stdio: 'inherit' });
if (!process.env.DIRNAME_CONTAINS_SHELL_UNSAFE_CHARS)
execSync(`"${process.execPath}" "${__filename}" "execSync"`, { stdio: 'inherit' });
execFileSync(process.execPath, [ __filename, 'execFileSync' ], { stdio: 'inherit' });
}

Expand Down
14 changes: 7 additions & 7 deletions test/parallel/test-fs-cp.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { mustCall, mustNotMutateObjectDeep } from '../common/index.mjs';
import { mustCall, mustNotMutateObjectDeep, isInsideDirWithUnusualChars } from '../common/index.mjs';

import assert from 'assert';
import fs from 'fs';
Expand Down Expand Up @@ -264,7 +264,7 @@ function nextdir(dirname) {
}

// It throws error if parent directory of symlink in dest points to src.
{
if (!isInsideDirWithUnusualChars) {
const src = nextdir();
mkdirSync(join(src, 'a'), mustNotMutateObjectDeep({ recursive: true }));
const dest = nextdir();
Expand All @@ -279,7 +279,7 @@ function nextdir(dirname) {
}

// It throws error if attempt is made to copy directory to file.
{
if (!isInsideDirWithUnusualChars) {
const src = nextdir();
mkdirSync(src, mustNotMutateObjectDeep({ recursive: true }));
const dest = './test/fixtures/copy/kitchen-sink/README.md';
Expand Down Expand Up @@ -310,7 +310,7 @@ function nextdir(dirname) {


// It throws error if attempt is made to copy file to directory.
{
if (!isInsideDirWithUnusualChars) {
const src = './test/fixtures/copy/kitchen-sink/README.md';
const dest = nextdir();
mkdirSync(dest, mustNotMutateObjectDeep({ recursive: true }));
Expand Down Expand Up @@ -346,7 +346,7 @@ function nextdir(dirname) {

// It throws error if attempt is made to copy src to dest
// when src is parent directory of the parent of dest
{
if (!isInsideDirWithUnusualChars) {
const src = nextdir('a');
const destParent = nextdir('a/b');
const dest = nextdir('a/b/c');
Expand All @@ -370,7 +370,7 @@ function nextdir(dirname) {
}

// It throws an error if attempt is made to copy socket.
if (!isWindows) {
if (!isWindows && !isInsideDirWithUnusualChars) {
const src = nextdir();
mkdirSync(src);
const dest = nextdir();
Expand Down Expand Up @@ -738,7 +738,7 @@ if (!isWindows) {
}

// It returns an error if attempt is made to copy socket.
if (!isWindows) {
if (!isWindows && !isInsideDirWithUnusualChars) {
const src = nextdir();
mkdirSync(src);
const dest = nextdir();
Expand Down
6 changes: 4 additions & 2 deletions test/parallel/test-inspector-strip-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ if (!process.config.variables.node_use_amaro) common.skip('Requires Amaro');
const { NodeInstance } = require('../common/inspector-helper.js');
const fixtures = require('../common/fixtures');
const assert = require('assert');
const { pathToFileURL } = require('url');

const scriptPath = fixtures.path('typescript/ts/test-typescript.ts');
const scriptURL = pathToFileURL(scriptPath);

async function runTest() {
const child = new NodeInstance(
Expand All @@ -30,10 +32,10 @@ async function runTest() {
const scriptParsed = await session.waitForNotification((notification) => {
if (notification.method !== 'Debugger.scriptParsed') return false;

return notification.params.url === scriptPath;
return notification.params.url === scriptPath || notification.params.url === scriptURL.href;
});
// Verify that the script has a sourceURL, hinting that it is a generated source.
assert(scriptParsed.params.hasSourceURL);
assert(scriptParsed.params.hasSourceURL || common.isInsideDirWithUnusualChars);

await session.waitForPauseOnStart();
await session.runToCompletion();
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-npm-install.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
if (common.isInsideDirWithUnusualChars)
common.skip('npm does not support this install path');

const path = require('path');
const exec = require('child_process').exec;
Expand Down
6 changes: 4 additions & 2 deletions test/parallel/test-snapshot-child-process-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// This tests that process.cwd() is accurate when
// restoring state from a snapshot

require('../common');
const { isInsideDirWithUnusualChars } = require('../common');
const { spawnSyncAndAssert } = require('../common/child_process');
const tmpdir = require('../common/tmpdir');
const fixtures = require('../common/fixtures');
Expand All @@ -14,7 +14,7 @@ const blobPath = tmpdir.resolve('snapshot.blob');
const file = fixtures.path('snapshot', 'child-process-sync.js');
const expected = [
'From child process spawnSync',
'From child process execSync',
...(isInsideDirWithUnusualChars ? [] : ['From child process execSync']),
'From child process execFileSync',
];

Expand All @@ -27,6 +27,7 @@ const expected = [
file,
], {
cwd: tmpdir.path,
env: { ...process.env, DIRNAME_CONTAINS_SHELL_UNSAFE_CHARS: isInsideDirWithUnusualChars ? 'TRUE' : '' },
}, {
trim: true,
stdout(output) {
Expand All @@ -43,6 +44,7 @@ const expected = [
file,
], {
cwd: tmpdir.path,
env: { ...process.env, DIRNAME_CONTAINS_SHELL_UNSAFE_CHARS: isInsideDirWithUnusualChars ? 'TRUE' : '' },
}, {
trim: true,
stdout(output) {
Expand Down
7 changes: 6 additions & 1 deletion test/parallel/test-startup-empty-regexp-statics.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
'use strict';

const common = require('../common');

if (common.isInsideDirWithUnusualChars) {
common.skip('expected failure');
}

const assert = require('node:assert');
const { spawnSync, spawn } = require('node:child_process');

Expand Down Expand Up @@ -66,7 +71,7 @@ const allRegExpStatics =
assert.strictEqual(child.signal, null);
}

{
if (!common.isInsideDirWithUnusualChars) {
const child = spawn(process.execPath, [], { stdio: ['pipe', 'pipe', 'inherit'], encoding: 'utf8' });

let stdout = '';
Expand Down
44 changes: 23 additions & 21 deletions test/parallel/test-startup-empty-regexp-statics.mjs
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
// We must load the CJS version here because the ESM wrapper call `hasIPv6`
// which compiles a RegEx.
// eslint-disable-next-line node-core/require-common-first
import '../common/index.js';
import common from '../common/index.js';
import assert from 'node:assert';

assert.strictEqual(RegExp.$_, '');
assert.strictEqual(RegExp.$0, undefined);
assert.strictEqual(RegExp.$1, '');
assert.strictEqual(RegExp.$2, '');
assert.strictEqual(RegExp.$3, '');
assert.strictEqual(RegExp.$4, '');
assert.strictEqual(RegExp.$5, '');
assert.strictEqual(RegExp.$6, '');
assert.strictEqual(RegExp.$7, '');
assert.strictEqual(RegExp.$8, '');
assert.strictEqual(RegExp.$9, '');
assert.strictEqual(RegExp.input, '');
assert.strictEqual(RegExp.lastMatch, '');
assert.strictEqual(RegExp.lastParen, '');
assert.strictEqual(RegExp.leftContext, '');
assert.strictEqual(RegExp.rightContext, '');
assert.strictEqual(RegExp['$&'], '');
assert.strictEqual(RegExp['$`'], '');
assert.strictEqual(RegExp['$+'], '');
assert.strictEqual(RegExp["$'"], '');
if (!common.isInsideDirWithUnusualChars) {
assert.strictEqual(RegExp.$_, '');
assert.strictEqual(RegExp.$0, undefined);
assert.strictEqual(RegExp.$1, '');
assert.strictEqual(RegExp.$2, '');
assert.strictEqual(RegExp.$3, '');
assert.strictEqual(RegExp.$4, '');
assert.strictEqual(RegExp.$5, '');
assert.strictEqual(RegExp.$6, '');
assert.strictEqual(RegExp.$7, '');
assert.strictEqual(RegExp.$8, '');
assert.strictEqual(RegExp.$9, '');
assert.strictEqual(RegExp.input, '');
assert.strictEqual(RegExp.lastMatch, '');
assert.strictEqual(RegExp.lastParen, '');
assert.strictEqual(RegExp.leftContext, '');
assert.strictEqual(RegExp.rightContext, '');
assert.strictEqual(RegExp['$&'], '');
assert.strictEqual(RegExp['$`'], '');
assert.strictEqual(RegExp['$+'], '');
assert.strictEqual(RegExp["$'"], '');
}
8 changes: 6 additions & 2 deletions tools/github_reporter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19304,6 +19304,7 @@ var require_stack_utils = __commonJS({
});

// node_modules/@reporters/github/index.js
var { fileURLToPath } = require("node:url");
var path = require("node:path");
var util = require("node:util");
var { EOL } = require("node:os");
Expand All @@ -19312,10 +19313,13 @@ var StackUtils = require_stack_utils();
var WORKSPACE = process.env.GITHUB_WORKSPACE ?? "";
var stack = new StackUtils({ cwd: WORKSPACE, internals: StackUtils.nodeInternals() });
var isFile = (name) => name?.startsWith(WORKSPACE);
var getRelativeFilePath = (name) => isFile(name) ? path.relative(WORKSPACE, require.resolve(name) ?? "") : null;
var getRelativeFilePath = (name) => isFile(name) ? path.relative(WORKSPACE, name) : null;
function getFilePath(fileName) {
if (fileName.startsWith("file://")) {
return getRelativeFilePath(new URL(fileName).pathname);
return getRelativeFilePath(fileURLToPath(fileName));
}
if (!path.isAbsolute(fileName)) {
return getRelativeFilePath(path.resolve(fileName) ?? "");
}
return getRelativeFilePath(fileName);
}
Expand Down
Loading