Skip to content

Commit 278496a

Browse files
authored
test(node): Ensure runner does not create unused tmp files (#17392)
Noticed locally that I sometimes got unexpected un-removed `tmp_xxx` dirs in node-integration tests. After some debugging, I figured out the reason: We clean the tmp dir up in `afterAll` of the test. However, if all tests inside are skipped, `afterAll` is not executed. So if you run tests in a filtered way - e.g. `yarn test -t "xxx"` - it would create the tmp dirs, but not delete them. This PR fixes this by moving the creation of the tmp dir into `beforeAll`, which equally is not executed when all tests are skipped.
1 parent 4abfe01 commit 278496a

File tree

1 file changed

+67
-57
lines changed
  • dev-packages/node-integration-tests/utils

1 file changed

+67
-57
lines changed

dev-packages/node-integration-tests/utils/runner.ts

Lines changed: 67 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { execSync, spawn, spawnSync } from 'child_process';
1717
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs';
1818
import { basename, join } from 'path';
1919
import { inspect } from 'util';
20-
import { afterAll, describe, test } from 'vitest';
20+
import { afterAll, beforeAll, describe, test } from 'vitest';
2121
import {
2222
assertEnvelopeHeader,
2323
assertSentryCheckIn,
@@ -195,74 +195,79 @@ export function createEsmAndCjsTests(
195195
// If additionalDependencies are provided, we also create a nested package.json and install them there.
196196
const uniqueId = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`;
197197
const tmpDirPath = join(cwd, `tmp_${uniqueId}`);
198-
mkdirSync(tmpDirPath);
199-
200-
// Copy ESM files as-is into tmp dir
201198
const esmScenarioBasename = basename(scenarioPath);
202199
const esmInstrumentBasename = basename(instrumentPath);
203200
const esmScenarioPathForRun = join(tmpDirPath, esmScenarioBasename);
204201
const esmInstrumentPathForRun = join(tmpDirPath, esmInstrumentBasename);
205-
writeFileSync(esmScenarioPathForRun, readFileSync(mjsScenarioPath, 'utf8'));
206-
writeFileSync(esmInstrumentPathForRun, readFileSync(mjsInstrumentPath, 'utf8'));
207-
208-
// Pre-create CJS converted files inside tmp dir
209202
const cjsScenarioPath = join(tmpDirPath, esmScenarioBasename.replace('.mjs', '.cjs'));
210203
const cjsInstrumentPath = join(tmpDirPath, esmInstrumentBasename.replace('.mjs', '.cjs'));
211-
convertEsmFileToCjs(esmScenarioPathForRun, cjsScenarioPath);
212-
convertEsmFileToCjs(esmInstrumentPathForRun, cjsInstrumentPath);
213-
214-
// Create a minimal package.json with requested dependencies (if any) and install them
215-
const additionalDependencies = options?.additionalDependencies ?? {};
216-
if (Object.keys(additionalDependencies).length > 0) {
217-
const packageJson = {
218-
name: 'tmp-integration-test',
219-
private: true,
220-
version: '0.0.0',
221-
dependencies: additionalDependencies,
222-
} as const;
223-
224-
writeFileSync(join(tmpDirPath, 'package.json'), JSON.stringify(packageJson, null, 2));
225-
226-
try {
227-
const deps = Object.entries(additionalDependencies).map(([name, range]) => {
228-
if (!range || typeof range !== 'string') {
229-
throw new Error(`Invalid version range for "${name}": ${String(range)}`);
230-
}
231-
return `${name}@${range}`;
232-
});
233204

234-
if (deps.length > 0) {
235-
// Prefer npm for temp installs to avoid Yarn engine strictness; see https://github.com/vercel/ai/issues/7777
236-
// We rely on the generated package.json dependencies and run a plain install.
237-
const result = spawnSync('npm', ['install', '--silent', '--no-audit', '--no-fund'], {
238-
cwd: tmpDirPath,
239-
encoding: 'utf8',
205+
function createTmpDir(): void {
206+
mkdirSync(tmpDirPath);
207+
208+
// Copy ESM files as-is into tmp dir
209+
writeFileSync(esmScenarioPathForRun, readFileSync(mjsScenarioPath, 'utf8'));
210+
writeFileSync(esmInstrumentPathForRun, readFileSync(mjsInstrumentPath, 'utf8'));
211+
212+
// Pre-create CJS converted files inside tmp dir
213+
convertEsmFileToCjs(esmScenarioPathForRun, cjsScenarioPath);
214+
convertEsmFileToCjs(esmInstrumentPathForRun, cjsInstrumentPath);
215+
216+
// Create a minimal package.json with requested dependencies (if any) and install them
217+
const additionalDependencies = options?.additionalDependencies ?? {};
218+
if (Object.keys(additionalDependencies).length > 0) {
219+
const packageJson = {
220+
name: 'tmp-integration-test',
221+
private: true,
222+
version: '0.0.0',
223+
dependencies: additionalDependencies,
224+
} as const;
225+
226+
writeFileSync(join(tmpDirPath, 'package.json'), JSON.stringify(packageJson, null, 2));
227+
228+
try {
229+
const deps = Object.entries(additionalDependencies).map(([name, range]) => {
230+
if (!range || typeof range !== 'string') {
231+
throw new Error(`Invalid version range for "${name}": ${String(range)}`);
232+
}
233+
return `${name}@${range}`;
240234
});
241235

242-
if (process.env.DEBUG) {
243-
// eslint-disable-next-line no-console
244-
console.log('[additionalDependencies via npm]', deps.join(' '));
245-
// eslint-disable-next-line no-console
246-
console.log('[npm stdout]', result.stdout);
247-
// eslint-disable-next-line no-console
248-
console.log('[npm stderr]', result.stderr);
249-
}
236+
if (deps.length > 0) {
237+
// Prefer npm for temp installs to avoid Yarn engine strictness; see https://github.com/vercel/ai/issues/7777
238+
// We rely on the generated package.json dependencies and run a plain install.
239+
const result = spawnSync('npm', ['install', '--silent', '--no-audit', '--no-fund'], {
240+
cwd: tmpDirPath,
241+
encoding: 'utf8',
242+
});
250243

251-
if (result.error) {
252-
throw new Error(`Failed to install additionalDependencies in tmp dir ${tmpDirPath}: ${result.error.message}`);
253-
}
254-
if (typeof result.status === 'number' && result.status !== 0) {
255-
throw new Error(
256-
`Failed to install additionalDependencies in tmp dir ${tmpDirPath} (exit ${result.status}):\n${
257-
result.stderr || result.stdout || '(no output)'
258-
}`,
259-
);
244+
if (process.env.DEBUG) {
245+
// eslint-disable-next-line no-console
246+
console.log('[additionalDependencies via npm]', deps.join(' '));
247+
// eslint-disable-next-line no-console
248+
console.log('[npm stdout]', result.stdout);
249+
// eslint-disable-next-line no-console
250+
console.log('[npm stderr]', result.stderr);
251+
}
252+
253+
if (result.error) {
254+
throw new Error(
255+
`Failed to install additionalDependencies in tmp dir ${tmpDirPath}: ${result.error.message}`,
256+
);
257+
}
258+
if (typeof result.status === 'number' && result.status !== 0) {
259+
throw new Error(
260+
`Failed to install additionalDependencies in tmp dir ${tmpDirPath} (exit ${result.status}):\n${
261+
result.stderr || result.stdout || '(no output)'
262+
}`,
263+
);
264+
}
260265
}
266+
} catch (e) {
267+
// eslint-disable-next-line no-console
268+
console.error('Failed to install additionalDependencies:', e);
269+
throw e;
261270
}
262-
} catch (e) {
263-
// eslint-disable-next-line no-console
264-
console.error('Failed to install additionalDependencies:', e);
265-
throw e;
266271
}
267272
}
268273

@@ -281,6 +286,11 @@ export function createEsmAndCjsTests(
281286
callback(() => createRunner(cjsScenarioPath).withFlags('--require', cjsInstrumentPath), cjsTestFn, 'cjs');
282287
});
283288

289+
// Create tmp directory
290+
beforeAll(() => {
291+
createTmpDir();
292+
});
293+
284294
// Clean up the tmp directory after both esm and cjs suites have run
285295
afterAll(() => {
286296
try {

0 commit comments

Comments
 (0)