Skip to content

Commit 5d7d58c

Browse files
BYKBYK
committed
fix(test): use os.tmpdir() for test temp directories
Tests were creating temp directories in $HOME (~/.sentry-cli-test-*) and never cleaning up on crashes/SIGKILL, leaving dozens of leftover dirs. The intermediate fix moved them to a project-local .test-tmp/ directory, but the correct location is the OS temp directory. Changes: - preload.ts: wipe $TMPDIR/sentry-cli-test/ at each run start, create preload dir there. One-time migration cleans old $HOME leftovers. - helpers.ts: export shared TEST_TMP_DIR constant using os.tmpdir() - fixture.ts: remove dead tmpdir() function and unused imports - upgrade tests: import TEST_TMP_DIR from helpers instead of computing it independently with resolve() - Fix 3 test files using wrong env var (SENTRY_CLI_CONFIG_DIR vs SENTRY_CONFIG_DIR) and deleting it in afterEach - Convert 5 test files to useTestConfigDir() for proper env restore - Fix 2 DSN test files creating dirs directly in $HOME - Redirect upgrade lock/release tests to temp dir
1 parent 629012c commit 5d7d58c

16 files changed

+155
-227
lines changed

‎.gitignore‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ coverage-isolated
1717
*.lcov
1818

1919
# test artifacts
20-
.test-tmp
2120
*.junit.xml
2221

2322
# logs

‎test/commands/cli/upgrade.test.ts‎

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,20 @@
1212
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
1313
import { mkdirSync, rmSync } from "node:fs";
1414
import { unlink } from "node:fs/promises";
15-
import { homedir } from "node:os";
1615
import { join } from "node:path";
1716
import { run } from "@stricli/core";
1817
import { app } from "../../../src/app.js";
1918
import type { SentryContext } from "../../../src/context.js";
2019
import { CLI_VERSION } from "../../../src/lib/constants.js";
20+
import {
21+
clearInstallInfo,
22+
setInstallInfo,
23+
} from "../../../src/lib/db/install-info.js";
2124
import {
2225
getReleaseChannel,
2326
setReleaseChannel,
2427
} from "../../../src/lib/db/release-channel.js";
25-
import { useTestConfigDir } from "../../helpers.js";
28+
import { TEST_TMP_DIR, useTestConfigDir } from "../../helpers.js";
2629

2730
/** Store original fetch for restoration */
2831
let originalFetch: typeof globalThis.fetch;
@@ -648,16 +651,25 @@ describe("sentry cli upgrade — curl full upgrade path (Bun.spawn spy)", () =>
648651
let spawnedArgs: string[][];
649652
let restoreStderr: (() => void) | undefined;
650653

651-
/** Default install paths (default curl dir) */
652-
const defaultBinDir = join(homedir(), ".sentry", "bin");
654+
/** Redirect curl install paths to temp dir instead of ~/.sentry/bin/ */
655+
const spawnBinDir = join(TEST_TMP_DIR, "upgrade-spawn-bin");
656+
const binName = process.platform === "win32" ? "sentry.exe" : "sentry";
657+
const spawnInstallPath = join(spawnBinDir, binName);
653658

654659
beforeEach(() => {
655660
testDir = join(
656-
"/tmp",
661+
TEST_TMP_DIR,
657662
`upgrade-spawn-test-${Date.now()}-${Math.random().toString(36).slice(2)}`
658663
);
659664
mkdirSync(testDir, { recursive: true });
660-
mkdirSync(defaultBinDir, { recursive: true });
665+
mkdirSync(spawnBinDir, { recursive: true });
666+
// Redirect getCurlInstallPaths() to temp dir
667+
clearInstallInfo();
668+
setInstallInfo({
669+
method: "curl",
670+
path: spawnInstallPath,
671+
version: "0.0.0",
672+
});
661673

662674
originalFetch = globalThis.fetch;
663675
originalSpawn = Bun.spawn;
@@ -677,15 +689,15 @@ describe("sentry cli upgrade — curl full upgrade path (Bun.spawn spy)", () =>
677689
Bun.spawn = originalSpawn;
678690
rmSync(testDir, { recursive: true, force: true });
679691

680-
// Clean up any temp binary files written to the default curl install path
681-
const binName = process.platform === "win32" ? "sentry.exe" : "sentry";
692+
// Clean up any temp binary files written to the redirected install path
682693
for (const suffix of ["", ".download", ".old", ".lock"]) {
683694
try {
684-
await unlink(join(defaultBinDir, `${binName}${suffix}`));
695+
await unlink(join(spawnBinDir, `${binName}${suffix}`));
685696
} catch {
686697
// Ignore
687698
}
688699
}
700+
clearInstallInfo();
689701
});
690702

691703
/**
@@ -856,15 +868,25 @@ describe("sentry cli upgrade — migrateToStandaloneForNightly (Bun.spawn spy)",
856868
let originalSpawn: typeof Bun.spawn;
857869
let restoreStderr: (() => void) | undefined;
858870

859-
const defaultBinDir = join(homedir(), ".sentry", "bin");
871+
/** Redirect curl install paths to temp dir instead of ~/.sentry/bin/ */
872+
const migrateBinDir = join(TEST_TMP_DIR, "upgrade-migrate-bin");
873+
const migrateBinName = process.platform === "win32" ? "sentry.exe" : "sentry";
874+
const migrateInstallPath = join(migrateBinDir, migrateBinName);
860875

861876
beforeEach(() => {
862877
testDir = join(
863-
"/tmp",
878+
TEST_TMP_DIR,
864879
`upgrade-migrate-test-${Date.now()}-${Math.random().toString(36).slice(2)}`
865880
);
866881
mkdirSync(testDir, { recursive: true });
867-
mkdirSync(defaultBinDir, { recursive: true });
882+
mkdirSync(migrateBinDir, { recursive: true });
883+
// Redirect getCurlInstallPaths() to temp dir
884+
clearInstallInfo();
885+
setInstallInfo({
886+
method: "curl",
887+
path: migrateInstallPath,
888+
version: "0.0.0",
889+
});
868890

869891
originalFetch = globalThis.fetch;
870892
originalSpawn = Bun.spawn;
@@ -881,14 +903,14 @@ describe("sentry cli upgrade — migrateToStandaloneForNightly (Bun.spawn spy)",
881903
Bun.spawn = originalSpawn;
882904
rmSync(testDir, { recursive: true, force: true });
883905

884-
const binName = process.platform === "win32" ? "sentry.exe" : "sentry";
885906
for (const suffix of ["", ".download", ".old", ".lock"]) {
886907
try {
887-
await unlink(join(defaultBinDir, `${binName}${suffix}`));
908+
await unlink(join(migrateBinDir, `${migrateBinName}${suffix}`));
888909
} catch {
889910
// Ignore
890911
}
891912
}
913+
clearInstallInfo();
892914
});
893915

894916
test("migrates npm install to standalone binary for nightly channel", async () => {

‎test/commands/project/list.test.ts‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import type { ParsedOrgProject } from "../../../src/lib/arg-parsing.js";
3232
import { DEFAULT_SENTRY_URL } from "../../../src/lib/constants.js";
3333
import { clearAuth, setAuthToken } from "../../../src/lib/db/auth.js";
3434
import { setDefaults } from "../../../src/lib/db/defaults.js";
35-
import { CONFIG_DIR_ENV_VAR } from "../../../src/lib/db/index.js";
3635
import {
3736
getPaginationCursor,
3837
resolveOrgCursor,
@@ -45,22 +44,10 @@ import {
4544
ValidationError,
4645
} from "../../../src/lib/errors.js";
4746
import type { SentryProject } from "../../../src/types/index.js";
48-
import { cleanupTestDir, createTestConfigDir } from "../../helpers.js";
47+
import { useTestConfigDir } from "../../helpers.js";
4948
import { DEFAULT_NUM_RUNS } from "../../model-based/helpers.js";
5049

51-
// Test config directory for DB-dependent tests
52-
let testConfigDir: string;
53-
54-
beforeEach(async () => {
55-
testConfigDir = await createTestConfigDir("test-project-list-", {
56-
isolateProjectRoot: true,
57-
});
58-
process.env[CONFIG_DIR_ENV_VAR] = testConfigDir;
59-
});
60-
61-
afterEach(async () => {
62-
await cleanupTestDir(testConfigDir);
63-
});
50+
useTestConfigDir("test-project-list-", { isolateProjectRoot: true });
6451

6552
/** Create a minimal project for testing */
6653
function makeProject(

‎test/fixture.ts‎

Lines changed: 4 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -4,58 +4,9 @@
44
* Shared utilities for creating isolated test environments.
55
*/
66

7-
import { mkdirSync, rmSync, writeFileSync } from "node:fs";
8-
import { homedir } from "node:os";
7+
import { mkdirSync } from "node:fs";
98
import { join } from "node:path";
109

11-
/**
12-
* Create a temporary directory for testing
13-
*
14-
* @param options - Configuration for the temp directory
15-
* @returns Disposable temp directory object
16-
*/
17-
export async function tmpdir(options?: {
18-
files?: Record<string, string>;
19-
env?: Record<string, string>;
20-
}) {
21-
const dir = join(
22-
homedir(),
23-
`.sentry-cli-test-${Math.random().toString(36).slice(2)}`
24-
);
25-
mkdirSync(dir, { recursive: true });
26-
27-
// Create files if specified
28-
if (options?.files) {
29-
for (const [filepath, content] of Object.entries(options.files)) {
30-
const fullPath = join(dir, filepath);
31-
const dirPath = fullPath.slice(0, fullPath.lastIndexOf("/"));
32-
if (dirPath && dirPath !== dir) {
33-
mkdirSync(dirPath, { recursive: true });
34-
}
35-
writeFileSync(fullPath, content);
36-
}
37-
}
38-
39-
// Create .env file if specified
40-
if (options?.env) {
41-
const envContent = Object.entries(options.env)
42-
.map(([key, value]) => `${key}=${value}`)
43-
.join("\n");
44-
writeFileSync(join(dir, ".env"), envContent);
45-
}
46-
47-
return {
48-
path: dir,
49-
[Symbol.asyncDispose]: async () => {
50-
try {
51-
rmSync(dir, { recursive: true, force: true });
52-
} catch {
53-
// Ignore cleanup errors
54-
}
55-
},
56-
};
57-
}
58-
5910
/**
6011
* Mock process for capturing CLI output
6112
*/
@@ -187,6 +138,9 @@ export function createE2EContext(
187138
if (prevDir !== undefined) {
188139
process.env[CONFIG_DIR_ENV_VAR] = prevDir;
189140
} else {
141+
// This delete is acceptable here — it's a scoped try/finally restore,
142+
// not a test lifecycle hook. preload.ts always sets the var so this
143+
// branch rarely fires.
190144
delete process.env[CONFIG_DIR_ENV_VAR];
191145
}
192146
}

‎test/helpers.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@
77
import { afterEach, beforeEach } from "bun:test";
88
import { mkdirSync } from "node:fs";
99
import { mkdtemp, rm } from "node:fs/promises";
10-
import { join, resolve } from "node:path";
10+
import { tmpdir } from "node:os";
11+
import { join } from "node:path";
1112
import { CONFIG_DIR_ENV_VAR, closeDatabase } from "../src/lib/db/index.js";
1213

13-
const TEST_TMP_DIR = resolve(import.meta.dir, "../.test-tmp");
14+
/** Shared temp directory for all test files. Lives under the OS temp dir. */
15+
export const TEST_TMP_DIR = join(tmpdir(), "sentry-cli-test");
1416
mkdirSync(TEST_TMP_DIR, { recursive: true });
1517

1618
type TestConfigDirOptions = {
@@ -24,7 +26,7 @@ type TestConfigDirOptions = {
2426

2527
/**
2628
* Creates a unique temporary directory for test isolation.
27-
* Uses a project-local temp directory to avoid read-only system /tmp issues.
29+
* Uses a namespaced subdirectory under the OS temp directory.
2830
*
2931
* @param prefix - Directory name prefix (default: "sentry-test-")
3032
* @param options - Configuration options
@@ -111,6 +113,9 @@ export function useTestConfigDir(
111113
// Deleting process.env.SENTRY_CONFIG_DIR causes failures in test files
112114
// that load after this afterEach runs, because their module-level code
113115
// (or beforeEach hooks) may read the env var and get undefined.
116+
// Note: preload.ts always sets SENTRY_CONFIG_DIR, so savedConfigDir is
117+
// always defined in practice. The else branch is intentionally omitted
118+
// to avoid the "delete process.env" anti-pattern.
114119
if (savedConfigDir !== undefined) {
115120
process.env[CONFIG_DIR_ENV_VAR] = savedConfigDir;
116121
}

‎test/lib/api-client.coverage.test.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,27 +40,23 @@ import {
4040
updateIssueStatus,
4141
} from "../../src/lib/api-client.js";
4242
import { setAuthToken } from "../../src/lib/db/auth.js";
43-
import { CONFIG_DIR_ENV_VAR } from "../../src/lib/db/index.js";
4443
import { setOrgRegion } from "../../src/lib/db/regions.js";
4544
import { ApiError, AuthError } from "../../src/lib/errors.js";
46-
import { cleanupTestDir, createTestConfigDir, mockFetch } from "../helpers.js";
45+
import { mockFetch, useTestConfigDir } from "../helpers.js";
4746

4847
// --- Shared test setup ---
4948

50-
let testConfigDir: string;
49+
useTestConfigDir("test-api-coverage-");
5150
let originalFetch: typeof globalThis.fetch;
5251

5352
beforeEach(async () => {
54-
testConfigDir = await createTestConfigDir("test-api-coverage-");
55-
process.env[CONFIG_DIR_ENV_VAR] = testConfigDir;
5653
originalFetch = globalThis.fetch;
5754
await setAuthToken("test-token");
5855
await setOrgRegion("test-org", "https://sentry.io");
5956
});
6057

61-
afterEach(async () => {
58+
afterEach(() => {
6259
globalThis.fetch = originalFetch;
63-
await cleanupTestDir(testConfigDir);
6460
});
6561

6662
// --- Helpers ---

‎test/lib/api-client.multiregion.test.ts‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,17 @@ import {
1313
listOrganizationsInRegion,
1414
} from "../../src/lib/api-client.js";
1515
import { setAuthToken } from "../../src/lib/db/auth.js";
16-
import { CONFIG_DIR_ENV_VAR, closeDatabase } from "../../src/lib/db/index.js";
1716
import {
1817
clearOrgRegions,
1918
getAllOrgRegions,
2019
setOrgRegion,
2120
} from "../../src/lib/db/regions.js";
22-
import { cleanupTestDir, createTestConfigDir } from "../helpers.js";
21+
import { useTestConfigDir } from "../helpers.js";
2322

24-
let testConfigDir: string;
23+
useTestConfigDir("test-multiregion-");
2524
let originalFetch: typeof globalThis.fetch;
2625

2726
beforeEach(async () => {
28-
testConfigDir = await createTestConfigDir("test-multiregion-");
29-
process.env[CONFIG_DIR_ENV_VAR] = testConfigDir;
30-
3127
// Save original fetch
3228
originalFetch = globalThis.fetch;
3329

@@ -38,11 +34,9 @@ beforeEach(async () => {
3834
await clearOrgRegions();
3935
});
4036

41-
afterEach(async () => {
37+
afterEach(() => {
4238
// Restore original fetch
4339
globalThis.fetch = originalFetch;
44-
closeDatabase();
45-
await cleanupTestDir(testConfigDir);
4640
});
4741

4842
/**

‎test/lib/api-client.seer-trial.test.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,26 +11,21 @@ import {
1111
startProductTrial,
1212
} from "../../src/lib/api-client.js";
1313
import { setAuthToken } from "../../src/lib/db/auth.js";
14-
import { CONFIG_DIR_ENV_VAR } from "../../src/lib/db/index.js";
1514
import { setOrgRegion } from "../../src/lib/db/regions.js";
16-
import { cleanupTestDir, createTestConfigDir, mockFetch } from "../helpers.js";
15+
import { mockFetch, useTestConfigDir } from "../helpers.js";
1716

18-
let testConfigDir: string;
17+
useTestConfigDir("test-product-trial-api-");
1918
let originalFetch: typeof globalThis.fetch;
2019

2120
beforeEach(async () => {
22-
testConfigDir = await createTestConfigDir("test-product-trial-api-");
23-
process.env[CONFIG_DIR_ENV_VAR] = testConfigDir;
24-
2521
originalFetch = globalThis.fetch;
2622

2723
await setAuthToken("test-token");
2824
await setOrgRegion("test-org", "https://sentry.io");
2925
});
3026

31-
afterEach(async () => {
27+
afterEach(() => {
3228
globalThis.fetch = originalFetch;
33-
await cleanupTestDir(testConfigDir);
3429
});
3530

3631
describe("getProductTrials", () => {

‎test/lib/api-client.seer.test.ts‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,13 @@ import {
1111
triggerSolutionPlanning,
1212
} from "../../src/lib/api-client.js";
1313
import { setAuthToken } from "../../src/lib/db/auth.js";
14-
import { CONFIG_DIR_ENV_VAR } from "../../src/lib/db/index.js";
1514
import { setOrgRegion } from "../../src/lib/db/regions.js";
16-
import { cleanupTestDir, createTestConfigDir } from "../helpers.js";
15+
import { useTestConfigDir } from "../helpers.js";
1716

18-
// Test config directory
19-
let testConfigDir: string;
17+
useTestConfigDir("test-seer-api-");
2018
let originalFetch: typeof globalThis.fetch;
2119

2220
beforeEach(async () => {
23-
testConfigDir = await createTestConfigDir("test-seer-api-");
24-
process.env[CONFIG_DIR_ENV_VAR] = testConfigDir;
25-
2621
// Save original fetch
2722
originalFetch = globalThis.fetch;
2823

@@ -32,10 +27,9 @@ beforeEach(async () => {
3227
await setOrgRegion("test-org", "https://sentry.io");
3328
});
3429

35-
afterEach(async () => {
30+
afterEach(() => {
3631
// Restore original fetch
3732
globalThis.fetch = originalFetch;
38-
await cleanupTestDir(testConfigDir);
3933
});
4034

4135
describe("triggerRootCauseAnalysis", () => {

0 commit comments

Comments
 (0)