From b0f50c344e30a83095d19f696b44b5537b075a81 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 13 Feb 2026 16:12:00 +0000 Subject: [PATCH 1/7] fix(tests): centralize test config dir lifecycle to prevent env var pollution Bun runs test files sequentially in a single thread. Several test files unconditionally deleted process.env.SENTRY_CONFIG_DIR in afterEach hooks, causing the next file's module-level code to read undefined and crash with TypeError. A runner image upgrade changed file discovery order, exposing this latent bug. Introduces useTestConfigDir() helper in test/helpers.ts that creates a unique temp directory in beforeEach and restores (never deletes) the env var in afterEach. Migrates all 11 affected test files to use it. --- AGENTS.md | 31 +++++++++++++++ test/commands/cli/fix.test.ts | 61 ++++++---------------------- test/commands/issue/utils.test.ts | 38 ++++++++---------- test/helpers.ts | 46 +++++++++++++++++++++ test/lib/api-client.test.ts | 14 ++----- test/lib/config.test.ts | 39 ++++-------------- test/lib/db/concurrent.test.ts | 33 +++++----------- test/lib/db/install-info.test.ts | 18 ++------- test/lib/db/project-cache.test.ts | 19 ++------- test/lib/db/schema.test.ts | 66 +++++++------------------------ test/lib/region.test.ts | 14 +------ test/lib/version-check.test.ts | 22 ++++------- 12 files changed, 157 insertions(+), 244 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0a8a2013..e73fba2b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -383,6 +383,37 @@ Minimal comments, maximum clarity. Comments explain **intent and reasoning**, no | Unit tests | `*.test.ts` | `test/` (mirrors `src/`) | | E2E tests | `*.test.ts` | `test/e2e/` | +### Test Environment Isolation (CRITICAL) + +Tests that need a database or config directory **must** use `useTestConfigDir()` from `test/helpers.ts`. This helper: +- Creates a unique temp directory in `beforeEach` +- Sets `SENTRY_CONFIG_DIR` to point at it +- **Restores** (never deletes) the env var in `afterEach` +- Closes the database and cleans up temp files + +**NEVER** do any of these in test files: +- `delete process.env.SENTRY_CONFIG_DIR` — This pollutes other test files that load after yours +- `const baseDir = process.env[CONFIG_DIR_ENV_VAR]!` at module scope — This captures a value that may be stale +- Manual `beforeEach`/`afterEach` that sets/deletes `SENTRY_CONFIG_DIR` + +**Why**: Bun runs test files **sequentially in one thread** (load → run all tests → load next file). If your `afterEach` deletes the env var, the next file's module-level code reads `undefined`, causing `TypeError: The "paths[0]" property must be of type string`. + +```typescript +// CORRECT: Use the helper +import { useTestConfigDir } from "../helpers.js"; + +const getConfigDir = useTestConfigDir("my-test-prefix-"); + +// If you need the directory path in a test: +test("example", () => { + const dir = getConfigDir(); +}); + +// WRONG: Manual env var management +beforeEach(() => { process.env.SENTRY_CONFIG_DIR = tmpDir; }); +afterEach(() => { delete process.env.SENTRY_CONFIG_DIR; }); // BUG! +``` + ### Property-Based Testing Use property-based tests when verifying invariants that should hold for **any valid input**. diff --git a/test/commands/cli/fix.test.ts b/test/commands/cli/fix.test.ts index 82364b73..029949fd 100644 --- a/test/commands/cli/fix.test.ts +++ b/test/commands/cli/fix.test.ts @@ -3,19 +3,16 @@ */ import { Database } from "bun:sqlite"; -import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; -import { mkdirSync, rmSync } from "node:fs"; +import { describe, expect, mock, test } from "bun:test"; import { join } from "node:path"; import { fixCommand } from "../../../src/commands/cli/fix.js"; -import { - CONFIG_DIR_ENV_VAR, - closeDatabase, -} from "../../../src/lib/db/index.js"; +import { closeDatabase } from "../../../src/lib/db/index.js"; import { EXPECTED_TABLES, generatePreMigrationTableDDL, initSchema, } from "../../../src/lib/db/schema.js"; +import { useTestConfigDir } from "../../helpers.js"; /** * Generate DDL for creating a database with pre-migration tables. @@ -64,46 +61,12 @@ function createDatabaseWithMissingTables( ).run(); } -let testDir: string; -let originalConfigDir: string | undefined; - -beforeEach(() => { - // Save original config dir - originalConfigDir = process.env[CONFIG_DIR_ENV_VAR]; - - // Close any existing database connection - closeDatabase(); - - // Create unique test directory - const baseDir = originalConfigDir ?? "/tmp/sentry-cli-test"; - testDir = join( - baseDir, - `fix-test-${Date.now()}-${Math.random().toString(36).slice(2)}` - ); - mkdirSync(testDir, { recursive: true }); - process.env[CONFIG_DIR_ENV_VAR] = testDir; -}); - -afterEach(() => { - closeDatabase(); - // Restore original config dir - if (originalConfigDir) { - process.env[CONFIG_DIR_ENV_VAR] = originalConfigDir; - } else { - delete process.env[CONFIG_DIR_ENV_VAR]; - } - // Clean up test directory - try { - rmSync(testDir, { recursive: true, force: true }); - } catch { - // Ignore cleanup errors - } -}); +const getTestDir = useTestConfigDir("fix-test-"); describe("sentry cli fix", () => { test("reports no issues for healthy database", async () => { // Create healthy database - const db = new Database(join(testDir, "cli.db")); + const db = new Database(join(getTestDir(), "cli.db")); initSchema(db); db.close(); @@ -124,7 +87,7 @@ describe("sentry cli fix", () => { test("detects and reports missing columns in dry-run mode", async () => { // Create database with pre-migration tables (missing v4 columns) - const db = new Database(join(testDir, "cli.db")); + const db = new Database(join(getTestDir(), "cli.db")); createPreMigrationDatabase(db); db.close(); @@ -148,7 +111,7 @@ describe("sentry cli fix", () => { test("fixes missing columns when not in dry-run mode", async () => { // Create database with pre-migration tables (missing v4 columns) - const db = new Database(join(testDir, "cli.db")); + const db = new Database(join(getTestDir(), "cli.db")); createPreMigrationDatabase(db); db.close(); @@ -169,7 +132,7 @@ describe("sentry cli fix", () => { // Verify the column was actually added closeDatabase(); - const verifyDb = new Database(join(testDir, "cli.db")); + const verifyDb = new Database(join(getTestDir(), "cli.db")); const cols = verifyDb.query("PRAGMA table_info(dsn_cache)").all() as Array<{ name: string; }>; @@ -188,7 +151,7 @@ describe("sentry cli fix", () => { // that was previously missing tables (now fixed by auto-repair at startup). test("handles database that was auto-repaired at startup", async () => { // Create database missing dsn_cache - initSchema will create it when command runs - const db = new Database(join(testDir, "cli.db")); + const db = new Database(join(getTestDir(), "cli.db")); createDatabaseWithMissingTables(db, ["dsn_cache"]); db.close(); @@ -210,7 +173,7 @@ describe("sentry cli fix", () => { // Verify the table was created (by initSchema auto-repair) closeDatabase(); - const verifyDb = new Database(join(testDir, "cli.db")); + const verifyDb = new Database(join(getTestDir(), "cli.db")); const tables = verifyDb .query( "SELECT name FROM sqlite_master WHERE type='table' AND name='dsn_cache'" @@ -221,7 +184,7 @@ describe("sentry cli fix", () => { }); test("shows database path in output", async () => { - const db = new Database(join(testDir, "cli.db")); + const db = new Database(join(getTestDir(), "cli.db")); initSchema(db); db.close(); @@ -237,6 +200,6 @@ describe("sentry cli fix", () => { const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("Database:"); - expect(output).toContain(testDir); + expect(output).toContain(getTestDir()); }); }); diff --git a/test/commands/issue/utils.test.ts b/test/commands/issue/utils.test.ts index ad3ac173..2b1e3d52 100644 --- a/test/commands/issue/utils.test.ts +++ b/test/commands/issue/utils.test.ts @@ -13,9 +13,8 @@ import { } from "../../../src/commands/issue/utils.js"; import { DEFAULT_SENTRY_URL } from "../../../src/lib/constants.js"; import { setAuthToken } from "../../../src/lib/db/auth.js"; -import { CONFIG_DIR_ENV_VAR } from "../../../src/lib/db/index.js"; import { setOrgRegion } from "../../../src/lib/db/regions.js"; -import { cleanupTestDir, createTestConfigDir } from "../../helpers.js"; +import { useTestConfigDir } from "../../helpers.js"; describe("buildCommandHint", () => { test("suggests /ID for numeric IDs", () => { @@ -47,15 +46,13 @@ describe("buildCommandHint", () => { }); }); -let testConfigDir: string; +const getConfigDir = useTestConfigDir("test-issue-utils-", { + isolateProjectRoot: true, +}); + let originalFetch: typeof globalThis.fetch; beforeEach(async () => { - // Use isolateProjectRoot to prevent DSN detection from scanning the real project - testConfigDir = await createTestConfigDir("test-issue-utils-", { - isolateProjectRoot: true, - }); - process.env[CONFIG_DIR_ENV_VAR] = testConfigDir; originalFetch = globalThis.fetch; await setAuthToken("test-token"); // Pre-populate region cache for orgs used in tests to avoid region resolution API calls @@ -65,9 +62,8 @@ beforeEach(async () => { await setOrgRegion("org1", DEFAULT_SENTRY_URL); }); -afterEach(async () => { +afterEach(() => { globalThis.fetch = originalFetch; - await cleanupTestDir(testConfigDir); }); describe("resolveOrgAndIssueId", () => { @@ -106,7 +102,7 @@ describe("resolveOrgAndIssueId", () => { await expect( resolveOrgAndIssueId({ issueArg: "123456789", - cwd: testConfigDir, + cwd: getConfigDir(), command: "explain", }) ).rejects.toThrow("Organization"); @@ -144,7 +140,7 @@ describe("resolveOrgAndIssueId", () => { const result = await resolveOrgAndIssueId({ issueArg: "my-org/PROJECT-ABC", - cwd: testConfigDir, + cwd: getConfigDir(), command: "explain", }); @@ -196,7 +192,7 @@ describe("resolveOrgAndIssueId", () => { const result = await resolveOrgAndIssueId({ issueArg: "f-g", - cwd: testConfigDir, + cwd: getConfigDir(), command: "explain", }); @@ -237,7 +233,7 @@ describe("resolveOrgAndIssueId", () => { const result = await resolveOrgAndIssueId({ issueArg: "org1/dashboard-4y", - cwd: testConfigDir, + cwd: getConfigDir(), command: "explain", }); @@ -280,7 +276,7 @@ describe("resolveOrgAndIssueId", () => { const result = await resolveOrgAndIssueId({ issueArg: "G", - cwd: testConfigDir, + cwd: getConfigDir(), command: "explain", }); @@ -298,7 +294,7 @@ describe("resolveOrgAndIssueId", () => { await expect( resolveOrgAndIssueId({ issueArg: "G", - cwd: testConfigDir, + cwd: getConfigDir(), command: "explain", }) ).rejects.toThrow("Cannot resolve issue suffix"); @@ -375,7 +371,7 @@ describe("resolveOrgAndIssueId", () => { const result = await resolveOrgAndIssueId({ issueArg: "craft-g", - cwd: testConfigDir, + cwd: getConfigDir(), command: "explain", }); @@ -435,7 +431,7 @@ describe("resolveOrgAndIssueId", () => { await expect( resolveOrgAndIssueId({ issueArg: "nonexistent-g", - cwd: testConfigDir, + cwd: getConfigDir(), command: "explain", }) ).rejects.toThrow("not found"); @@ -511,7 +507,7 @@ describe("resolveOrgAndIssueId", () => { await expect( resolveOrgAndIssueId({ issueArg: "common-g", - cwd: testConfigDir, + cwd: getConfigDir(), command: "explain", }) ).rejects.toThrow("multiple organizations"); @@ -531,7 +527,7 @@ describe("resolveOrgAndIssueId", () => { await expect( resolveOrgAndIssueId({ issueArg: "G", - cwd: testConfigDir, + cwd: getConfigDir(), command: "explain", }) ).rejects.toThrow(); @@ -551,7 +547,7 @@ describe("resolveOrgAndIssueId", () => { await expect( resolveOrgAndIssueId({ issueArg: "G", - cwd: testConfigDir, + cwd: getConfigDir(), command: "explain", }) ).rejects.toThrow("500"); diff --git a/test/helpers.ts b/test/helpers.ts index c2f27312..98a1f37c 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -4,9 +4,11 @@ * Shared utilities for test setup and teardown. */ +import { afterEach, beforeEach } from "bun:test"; import { mkdirSync } from "node:fs"; import { mkdtemp, rm } from "node:fs/promises"; import { join, resolve } from "node:path"; +import { CONFIG_DIR_ENV_VAR, closeDatabase } from "../src/lib/db/index.js"; const TEST_TMP_DIR = resolve(import.meta.dir, "../.test-tmp"); mkdirSync(TEST_TMP_DIR, { recursive: true }); @@ -73,3 +75,47 @@ type FetchMockFn = export function mockFetch(fn: FetchMockFn): typeof fetch { return fn as unknown as typeof fetch; } + +/** + * Sets up an isolated test config directory with proper env var lifecycle. + * + * Registers beforeEach/afterEach hooks that create a unique config directory, + * point SENTRY_CONFIG_DIR at it, and restore the original value on teardown. + * This eliminates the fragile pattern of manually managing process.env in + * each test file, which caused cross-file pollution when afterEach hooks + * deleted the env var while other files were still loading. + * + * Must be called at module scope or inside a describe() block. + * + * @param prefix - Directory name prefix for the temp directory + * @param options - Configuration options (e.g., isolateProjectRoot) + * @returns Getter function for the current test's config directory path + */ +export function useTestConfigDir( + prefix = "sentry-test-", + options?: TestConfigDirOptions +): () => string { + let dir: string; + let savedConfigDir: string | undefined; + + beforeEach(async () => { + savedConfigDir = process.env[CONFIG_DIR_ENV_VAR]; + closeDatabase(); + dir = await createTestConfigDir(prefix, options); + process.env[CONFIG_DIR_ENV_VAR] = dir; + }); + + afterEach(async () => { + closeDatabase(); + // Always restore the previous value — never delete. + // Deleting process.env.SENTRY_CONFIG_DIR causes failures in test files + // that load after this afterEach runs, because their module-level code + // (or beforeEach hooks) may read the env var and get undefined. + if (savedConfigDir !== undefined) { + process.env[CONFIG_DIR_ENV_VAR] = savedConfigDir; + } + await cleanupTestDir(dir); + }); + + return () => dir; +} diff --git a/test/lib/api-client.test.ts b/test/lib/api-client.test.ts index 18838a09..15ec7d57 100644 --- a/test/lib/api-client.test.ts +++ b/test/lib/api-client.test.ts @@ -8,11 +8,10 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { buildSearchParams, rawApiRequest } from "../../src/lib/api-client.js"; import { setAuthToken } from "../../src/lib/db/auth.js"; -import { CONFIG_DIR_ENV_VAR } from "../../src/lib/db/index.js"; -import { cleanupTestDir, createTestConfigDir } from "../helpers.js"; +import { useTestConfigDir } from "../helpers.js"; + +useTestConfigDir("test-api-"); -// Test config directory -let testConfigDir: string; let originalFetch: typeof globalThis.fetch; /** @@ -26,9 +25,6 @@ type RequestLog = { }; beforeEach(async () => { - testConfigDir = await createTestConfigDir("test-api-"); - process.env[CONFIG_DIR_ENV_VAR] = testConfigDir; - // Set required env var for OAuth refresh process.env.SENTRY_CLIENT_ID = "test-client-id"; @@ -39,11 +35,9 @@ beforeEach(async () => { await setAuthToken("initial-token", 3600, "test-refresh-token"); }); -afterEach(async () => { +afterEach(() => { // Restore original fetch globalThis.fetch = originalFetch; - - await cleanupTestDir(testConfigDir); }); /** diff --git a/test/lib/config.test.ts b/test/lib/config.test.ts index e2b747f5..fed2c62e 100644 --- a/test/lib/config.test.ts +++ b/test/lib/config.test.ts @@ -4,8 +4,8 @@ * Integration tests for SQLite-based config storage. */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync, writeFileSync } from "node:fs"; +import { describe, expect, test } from "bun:test"; +import { writeFileSync } from "node:fs"; import { join } from "node:path"; import { clearAuth, @@ -38,37 +38,14 @@ import { setCachedProject, setCachedProjectByDsnKey, } from "../../src/lib/db/project-cache.js"; +import { useTestConfigDir } from "../helpers.js"; /** - * Test isolation: Each test gets its own config directory within - * the main test directory created by preload.ts. - * - * We don't delete test subdirectories in afterEach because tests - * run in parallel and the deletion can race with other tests that - * are still running. The parent test directory is cleaned up on - * process exit by preload.ts. + * Test isolation: Each test gets its own config directory via useTestConfigDir(). + * The helper creates a unique temp directory in beforeEach and restores + * the env var in afterEach (never deleting it). */ -const testBaseDir = process.env[CONFIG_DIR_ENV_VAR]!; - -beforeEach(() => { - // Close any previous database connection - closeDatabase(); - - // Create a unique subdirectory for this test - const testConfigDir = join( - testBaseDir, - `test-${Math.random().toString(36).slice(2)}` - ); - mkdirSync(testConfigDir, { recursive: true }); - process.env[CONFIG_DIR_ENV_VAR] = testConfigDir; -}); - -afterEach(() => { - // Close database to release file handles - closeDatabase(); - // Note: We don't delete the test directory here because tests run in parallel. - // The parent test directory is cleaned up on process exit by preload.ts. -}); +const getConfigDir = useTestConfigDir("test-config-"); describe("auth token management", () => { test("setAuthToken stores token", async () => { @@ -565,7 +542,7 @@ describe("getDbPath", () => { test("returns the database file path", () => { const path = getDbPath(); expect(path).toContain("cli.db"); - expect(path).toContain(testBaseDir); + expect(path).toContain(getConfigDir()); }); }); diff --git a/test/lib/db/concurrent.test.ts b/test/lib/db/concurrent.test.ts index 45b8dabc..a522fff6 100644 --- a/test/lib/db/concurrent.test.ts +++ b/test/lib/db/concurrent.test.ts @@ -8,8 +8,7 @@ * CLI usage (e.g., multiple terminals, CI jobs, editor integrations). */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync } from "node:fs"; +import { beforeEach, describe, expect, test } from "bun:test"; import { join } from "node:path"; import { getCachedDsn } from "../../../src/lib/db/dsn-cache.js"; import { @@ -17,9 +16,9 @@ import { closeDatabase, } from "../../../src/lib/db/index.js"; import { getCachedProject } from "../../../src/lib/db/project-cache.js"; +import { useTestConfigDir } from "../../helpers.js"; const WORKER_SCRIPT = join(import.meta.dir, "concurrent-worker.ts"); -const TEST_BASE_DIR = process.env[CONFIG_DIR_ENV_VAR]!; type WorkerResult = { workerId: string; @@ -88,17 +87,9 @@ async function spawnWorkersConcurrently( } describe("concurrent database access", () => { - let testConfigDir: string; + const getConfigDir = useTestConfigDir("concurrent-"); beforeEach(async () => { - closeDatabase(); - testConfigDir = join( - TEST_BASE_DIR, - `concurrent-${Date.now()}-${Math.random().toString(36).slice(2)}` - ); - mkdirSync(testConfigDir, { recursive: true }); - process.env[CONFIG_DIR_ENV_VAR] = testConfigDir; - // Pre-create the database before spawning workers. // This ensures schema initialization completes before concurrent access, // which avoids lock contention during initial table creation. @@ -107,14 +98,10 @@ describe("concurrent database access", () => { closeDatabase(); }); - afterEach(() => { - closeDatabase(); - }); - test("multiple processes can write DSN cache entries simultaneously", async () => { const workerCount = 5; const results = await spawnWorkersConcurrently( - testConfigDir, + getConfigDir(), workerCount, "write-dsn" ); @@ -124,7 +111,7 @@ describe("concurrent database access", () => { // Verify all entries are present in the database closeDatabase(); // Close to re-open with fresh connection - process.env[CONFIG_DIR_ENV_VAR] = testConfigDir; + process.env[CONFIG_DIR_ENV_VAR] = getConfigDir(); for (let i = 0; i < workerCount; i++) { const directory = `/test/project-${i}`; @@ -137,7 +124,7 @@ describe("concurrent database access", () => { test("multiple processes can write project cache entries simultaneously", async () => { const workerCount = 5; const results = await spawnWorkersConcurrently( - testConfigDir, + getConfigDir(), workerCount, "write-project" ); @@ -147,7 +134,7 @@ describe("concurrent database access", () => { // Verify all entries are present closeDatabase(); - process.env[CONFIG_DIR_ENV_VAR] = testConfigDir; + process.env[CONFIG_DIR_ENV_VAR] = getConfigDir(); for (let i = 0; i < workerCount; i++) { const cached = await getCachedProject("org-456", `proj-${i}`); @@ -159,7 +146,7 @@ describe("concurrent database access", () => { test("mixed read/write operations from multiple processes succeed", async () => { const workerCount = 5; const results = await spawnWorkersConcurrently( - testConfigDir, + getConfigDir(), workerCount, "read-write" ); @@ -169,7 +156,7 @@ describe("concurrent database access", () => { // Each worker did 5 iterations, verify some entries closeDatabase(); - process.env[CONFIG_DIR_ENV_VAR] = testConfigDir; + process.env[CONFIG_DIR_ENV_VAR] = getConfigDir(); for (let w = 0; w < workerCount; w++) { for (let i = 0; i < 5; i++) { @@ -185,7 +172,7 @@ describe("concurrent database access", () => { // Run a larger batch to increase contention likelihood const workerCount = 10; const results = await spawnWorkersConcurrently( - testConfigDir, + getConfigDir(), workerCount, "write-dsn" ); diff --git a/test/lib/db/install-info.test.ts b/test/lib/db/install-info.test.ts index 7278e608..138c6fb0 100644 --- a/test/lib/db/install-info.test.ts +++ b/test/lib/db/install-info.test.ts @@ -2,27 +2,15 @@ * Install Info Storage Tests */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { closeDatabase } from "../../../src/lib/db/index.js"; +import { describe, expect, test } from "bun:test"; import { clearInstallInfo, getInstallInfo, setInstallInfo, } from "../../../src/lib/db/install-info.js"; -import { cleanupTestDir, createTestConfigDir } from "../../helpers.js"; +import { useTestConfigDir } from "../../helpers.js"; -let testConfigDir: string; - -beforeEach(async () => { - testConfigDir = await createTestConfigDir("test-install-info-"); - process.env.SENTRY_CONFIG_DIR = testConfigDir; -}); - -afterEach(async () => { - closeDatabase(); - delete process.env.SENTRY_CONFIG_DIR; - await cleanupTestDir(testConfigDir); -}); +useTestConfigDir("test-install-info-"); describe("getInstallInfo", () => { test("returns null when no install info stored", () => { diff --git a/test/lib/db/project-cache.test.ts b/test/lib/db/project-cache.test.ts index eb06d8bb..be2e2044 100644 --- a/test/lib/db/project-cache.test.ts +++ b/test/lib/db/project-cache.test.ts @@ -4,8 +4,7 @@ * Tests for caching project information by orgId:projectId or DSN public key. */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { closeDatabase } from "../../../src/lib/db/index.js"; +import { describe, expect, test } from "bun:test"; import { clearProjectCache, getCachedProject, @@ -13,21 +12,9 @@ import { setCachedProject, setCachedProjectByDsnKey, } from "../../../src/lib/db/project-cache.js"; -import { cleanupTestDir, createTestConfigDir } from "../../helpers.js"; +import { useTestConfigDir } from "../../helpers.js"; -let testConfigDir: string; - -beforeEach(async () => { - testConfigDir = await createTestConfigDir("test-project-cache-"); - process.env.SENTRY_CONFIG_DIR = testConfigDir; -}); - -afterEach(async () => { - // Close database to release file handles before cleanup - closeDatabase(); - delete process.env.SENTRY_CONFIG_DIR; - await cleanupTestDir(testConfigDir); -}); +useTestConfigDir("test-project-cache-"); describe("getCachedProject", () => { test("returns undefined when no cache entry exists", async () => { diff --git a/test/lib/db/schema.test.ts b/test/lib/db/schema.test.ts index c83aeb3b..6b322edd 100644 --- a/test/lib/db/schema.test.ts +++ b/test/lib/db/schema.test.ts @@ -3,13 +3,8 @@ */ import { Database } from "bun:sqlite"; -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync, rmSync } from "node:fs"; +import { describe, expect, test } from "bun:test"; import { join } from "node:path"; -import { - CONFIG_DIR_ENV_VAR, - closeDatabase, -} from "../../../src/lib/db/index.js"; import { CURRENT_SCHEMA_VERSION, EXPECTED_COLUMNS, @@ -21,6 +16,7 @@ import { repairSchema, tableExists, } from "../../../src/lib/db/schema.js"; +import { useTestConfigDir } from "../../helpers.js"; /** * Create a database with all tables but some missing (for testing repair). @@ -63,45 +59,11 @@ function createPreMigrationDatabase( ); } -let testDir: string; -let originalConfigDir: string | undefined; - -beforeEach(() => { - // Save original config dir - originalConfigDir = process.env[CONFIG_DIR_ENV_VAR]; - - // Close any existing database connection - closeDatabase(); - - // Create unique test directory - const baseDir = originalConfigDir ?? "/tmp/sentry-cli-test"; - testDir = join( - baseDir, - `schema-test-${Date.now()}-${Math.random().toString(36).slice(2)}` - ); - mkdirSync(testDir, { recursive: true }); - process.env[CONFIG_DIR_ENV_VAR] = testDir; -}); - -afterEach(() => { - closeDatabase(); - // Restore original config dir - if (originalConfigDir) { - process.env[CONFIG_DIR_ENV_VAR] = originalConfigDir; - } else { - delete process.env[CONFIG_DIR_ENV_VAR]; - } - // Clean up test directory - try { - rmSync(testDir, { recursive: true, force: true }); - } catch { - // Ignore cleanup errors - } -}); +const getTestDir = useTestConfigDir("schema-test-"); describe("tableExists", () => { test("returns true for existing table", () => { - const db = new Database(join(testDir, "test.db")); + const db = new Database(join(getTestDir(), "test.db")); db.exec("CREATE TABLE test_table (id INTEGER PRIMARY KEY)"); expect(tableExists(db, "test_table")).toBe(true); @@ -109,7 +71,7 @@ describe("tableExists", () => { }); test("returns false for non-existent table", () => { - const db = new Database(join(testDir, "test.db")); + const db = new Database(join(getTestDir(), "test.db")); expect(tableExists(db, "nonexistent")).toBe(false); db.close(); @@ -118,7 +80,7 @@ describe("tableExists", () => { describe("hasColumn", () => { test("returns true for existing column", () => { - const db = new Database(join(testDir, "test.db")); + const db = new Database(join(getTestDir(), "test.db")); db.exec("CREATE TABLE test_table (id INTEGER PRIMARY KEY, name TEXT)"); expect(hasColumn(db, "test_table", "id")).toBe(true); @@ -127,7 +89,7 @@ describe("hasColumn", () => { }); test("returns false for non-existent column", () => { - const db = new Database(join(testDir, "test.db")); + const db = new Database(join(getTestDir(), "test.db")); db.exec("CREATE TABLE test_table (id INTEGER PRIMARY KEY)"); expect(hasColumn(db, "test_table", "missing_column")).toBe(false); @@ -137,7 +99,7 @@ describe("hasColumn", () => { describe("getSchemaIssues", () => { test("returns empty array for healthy database", () => { - const db = new Database(join(testDir, "test.db")); + const db = new Database(join(getTestDir(), "test.db")); initSchema(db); const issues = getSchemaIssues(db); @@ -146,7 +108,7 @@ describe("getSchemaIssues", () => { }); test("detects missing table", () => { - const db = new Database(join(testDir, "test.db")); + const db = new Database(join(getTestDir(), "test.db")); // Create schema without dsn_cache using the helper createDatabaseWithMissingTables(db, ["dsn_cache"]); @@ -161,7 +123,7 @@ describe("getSchemaIssues", () => { }); test("detects missing column", () => { - const db = new Database(join(testDir, "test.db")); + const db = new Database(join(getTestDir(), "test.db")); // Create dsn_cache without v4 columns (pre-migration state) createPreMigrationDatabase(db, ["dsn_cache"]); @@ -185,7 +147,7 @@ describe("getSchemaIssues", () => { describe("repairSchema", () => { test("creates missing tables", () => { - const db = new Database(join(testDir, "test.db")); + const db = new Database(join(getTestDir(), "test.db")); db.exec("CREATE TABLE schema_version (version INTEGER PRIMARY KEY)"); db.query("INSERT INTO schema_version (version) VALUES (?)").run(1); @@ -204,7 +166,7 @@ describe("repairSchema", () => { }); test("adds missing columns", () => { - const db = new Database(join(testDir, "test.db")); + const db = new Database(join(getTestDir(), "test.db")); initSchema(db); // Remove migrated columns by recreating the table in pre-migration state @@ -228,7 +190,7 @@ describe("repairSchema", () => { }); test("returns empty result for healthy database", () => { - const db = new Database(join(testDir, "test.db")); + const db = new Database(join(getTestDir(), "test.db")); initSchema(db); const result = repairSchema(db); @@ -239,7 +201,7 @@ describe("repairSchema", () => { }); test("updates schema version after repair", () => { - const db = new Database(join(testDir, "test.db")); + const db = new Database(join(getTestDir(), "test.db")); db.exec("CREATE TABLE schema_version (version INTEGER PRIMARY KEY)"); db.query("INSERT INTO schema_version (version) VALUES (?)").run(1); diff --git a/test/lib/region.test.ts b/test/lib/region.test.ts index 493027c0..198400ed 100644 --- a/test/lib/region.test.ts +++ b/test/lib/region.test.ts @@ -5,27 +5,18 @@ */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync } from "node:fs"; -import { join } from "node:path"; import { setAuthToken } from "../../src/lib/db/auth.js"; -import { CONFIG_DIR_ENV_VAR, closeDatabase } from "../../src/lib/db/index.js"; import { setOrgRegion } from "../../src/lib/db/regions.js"; import { isMultiRegionEnabled, resolveOrgRegion, } from "../../src/lib/region.js"; import { getSentryBaseUrl } from "../../src/lib/sentry-urls.js"; +import { useTestConfigDir } from "../helpers.js"; -const testBaseDir = process.env[CONFIG_DIR_ENV_VAR]!; +useTestConfigDir("region-resolve-"); beforeEach(async () => { - closeDatabase(); - const testConfigDir = join( - testBaseDir, - `region-resolve-${Math.random().toString(36).slice(2)}` - ); - mkdirSync(testConfigDir, { recursive: true }); - process.env[CONFIG_DIR_ENV_VAR] = testConfigDir; // Clear any SENTRY_URL override for most tests delete process.env.SENTRY_URL; // Set up auth token for API tests @@ -33,7 +24,6 @@ beforeEach(async () => { }); afterEach(() => { - closeDatabase(); delete process.env.SENTRY_URL; }); diff --git a/test/lib/version-check.test.ts b/test/lib/version-check.test.ts index 6f52af54..44fc384e 100644 --- a/test/lib/version-check.test.ts +++ b/test/lib/version-check.test.ts @@ -13,7 +13,7 @@ import { maybeCheckForUpdateInBackground, shouldSuppressNotification, } from "../../src/lib/version-check.js"; -import { cleanupTestDir, createTestConfigDir } from "../helpers.js"; +import { useTestConfigDir } from "../helpers.js"; describe("shouldSuppressNotification", () => { test("suppresses for upgrade command", () => { @@ -46,24 +46,20 @@ describe("shouldSuppressNotification", () => { // Its probabilistic behavior is tested indirectly through maybeCheckForUpdateInBackground. describe("getUpdateNotification", () => { - let testConfigDir: string; + useTestConfigDir("test-version-notif-"); let savedNoUpdateCheck: string | undefined; - beforeEach(async () => { - testConfigDir = await createTestConfigDir("test-version-notif-"); - process.env.SENTRY_CONFIG_DIR = testConfigDir; + beforeEach(() => { // Save and clear the env var to test real implementation savedNoUpdateCheck = process.env.SENTRY_CLI_NO_UPDATE_CHECK; delete process.env.SENTRY_CLI_NO_UPDATE_CHECK; }); - afterEach(async () => { - delete process.env.SENTRY_CONFIG_DIR; + afterEach(() => { // Restore the env var if (savedNoUpdateCheck !== undefined) { process.env.SENTRY_CLI_NO_UPDATE_CHECK = savedNoUpdateCheck; } - await cleanupTestDir(testConfigDir); }); test("returns null when no version info is cached", () => { @@ -115,26 +111,22 @@ describe("abortPendingVersionCheck", () => { }); describe("maybeCheckForUpdateInBackground", () => { - let testConfigDir: string; + useTestConfigDir("test-version-bg-"); let savedNoUpdateCheck: string | undefined; - beforeEach(async () => { - testConfigDir = await createTestConfigDir("test-version-bg-"); - process.env.SENTRY_CONFIG_DIR = testConfigDir; + beforeEach(() => { // Save and clear the env var to test real implementation savedNoUpdateCheck = process.env.SENTRY_CLI_NO_UPDATE_CHECK; delete process.env.SENTRY_CLI_NO_UPDATE_CHECK; }); - afterEach(async () => { + afterEach(() => { // Abort any pending check to clean up abortPendingVersionCheck(); - delete process.env.SENTRY_CONFIG_DIR; // Restore the env var if (savedNoUpdateCheck !== undefined) { process.env.SENTRY_CLI_NO_UPDATE_CHECK = savedNoUpdateCheck; } - await cleanupTestDir(testConfigDir); }); test("does not throw when called", () => { From d15b2b9a0af61bc09b1dd04d20bca1778ddcb9cc Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 13 Feb 2026 16:15:58 +0000 Subject: [PATCH 2/7] chore: upgrade Bun from 1.3.3 to 1.3.9 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4f2ddebd..51953b33 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "engines": { "node": ">=22" }, - "packageManager": "bun@1.3.3", + "packageManager": "bun@1.3.9", "patchedDependencies": { "@stricli/core@1.2.5": "patches/@stricli%2Fcore@1.2.5.patch", "@sentry/core@10.38.0": "patches/@sentry%2Fcore@10.38.0.patch" From 3a4cfafe4c9d0c829204746d760dd7779df308c1 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 13 Feb 2026 16:33:17 +0000 Subject: [PATCH 3/7] fix(oauth): read SENTRY_CLIENT_ID at call time instead of module load The test preload deletes process.env.SENTRY_CLIENT_ID before oauth.ts loads, causing the module-level constant to capture an empty string. Tests that later set SENTRY_CLIENT_ID in beforeEach had no effect since the constant was already captured. This caused refreshAccessToken to throw ConfigError (not AuthError), which the ky afterResponse hook caught silently, preventing 401 retry from working. Replace the module-level constant with a getClientId() function that reads process.env at call time. --- src/lib/oauth.ts | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts index 5ac32ed3..1ecf0087 100644 --- a/src/lib/oauth.ts +++ b/src/lib/oauth.ts @@ -24,12 +24,20 @@ const SENTRY_URL = process.env.SENTRY_URL ?? "https://sentry.io"; * Build-time: Injected via Bun.build({ define: { SENTRY_CLIENT_ID: "..." } }) * Runtime: Can be overridden via SENTRY_CLIENT_ID env var (for self-hosted) * + * Read at call time (not module load time) so tests can set process.env.SENTRY_CLIENT_ID + * after module initialization. + * * @see script/build.ts */ declare const SENTRY_CLIENT_ID_BUILD: string | undefined; -const SENTRY_CLIENT_ID = - process.env.SENTRY_CLIENT_ID ?? - (typeof SENTRY_CLIENT_ID_BUILD !== "undefined" ? SENTRY_CLIENT_ID_BUILD : ""); +function getClientId(): string { + return ( + process.env.SENTRY_CLIENT_ID ?? + (typeof SENTRY_CLIENT_ID_BUILD !== "undefined" + ? SENTRY_CLIENT_ID_BUILD + : "") + ); +} // OAuth scopes requested for the CLI const SCOPES = [ @@ -85,7 +93,8 @@ async function fetchWithConnectionError( /** Request a device code from Sentry's device authorization endpoint */ function requestDeviceCode() { - if (!SENTRY_CLIENT_ID) { + const clientId = getClientId(); + if (!clientId) { throw new ConfigError( "SENTRY_CLIENT_ID is required for authentication", "Set SENTRY_CLIENT_ID environment variable or use a pre-built binary" @@ -99,7 +108,7 @@ function requestDeviceCode() { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded" }, body: new URLSearchParams({ - client_id: SENTRY_CLIENT_ID, + client_id: clientId, scope: SCOPES, }), } @@ -142,7 +151,7 @@ function pollForToken(deviceCode: string): Promise { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded" }, body: new URLSearchParams({ - client_id: SENTRY_CLIENT_ID, + client_id: getClientId(), device_code: deviceCode, grant_type: "urn:ietf:params:oauth:grant-type:device_code", }), @@ -313,7 +322,8 @@ export async function setApiToken(token: string): Promise { export function refreshAccessToken( refreshToken: string ): Promise { - if (!SENTRY_CLIENT_ID) { + const clientId = getClientId(); + if (!clientId) { throw new ConfigError( "SENTRY_CLIENT_ID is required for token refresh", "Set SENTRY_CLIENT_ID environment variable or use a pre-built binary" @@ -327,7 +337,7 @@ export function refreshAccessToken( method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded" }, body: new URLSearchParams({ - client_id: SENTRY_CLIENT_ID, + client_id: clientId, grant_type: "refresh_token", refresh_token: refreshToken, }), From 2d33279e51c9159a52a58e2e0531a0f2c0375541 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 13 Feb 2026 16:37:48 +0000 Subject: [PATCH 4/7] fix(tests): add /users/me/regions/ mock to resolveOrgAndIssueId tests The tests mocked globalThis.fetch for /organizations/ and /projects/ endpoints but not /users/me/regions/. When getUserRegions() got a 404 from the mock, ky threw an HTTPError which listOrganizations caught and fell back to the non-region path. On certain CI runner images, this error-fallback code path fails. Adding an explicit regions mock avoids the error path entirely and makes the tests deterministic. --- test/commands/issue/utils.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/commands/issue/utils.test.ts b/test/commands/issue/utils.test.ts index 2b1e3d52..3d0c7ac3 100644 --- a/test/commands/issue/utils.test.ts +++ b/test/commands/issue/utils.test.ts @@ -311,6 +311,14 @@ describe("resolveOrgAndIssueId", () => { const req = new Request(input, init); const url = req.url; + // getUserRegions - return empty regions to use fallback path + if (url.includes("/users/me/regions/")) { + return new Response(JSON.stringify({ regions: [] }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + // listOrganizations call if ( url.includes("/organizations/") && @@ -390,6 +398,14 @@ describe("resolveOrgAndIssueId", () => { const req = new Request(input, init); const url = req.url; + // getUserRegions - return empty regions to use fallback path + if (url.includes("/users/me/regions/")) { + return new Response(JSON.stringify({ regions: [] }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + // listOrganizations call if ( url.includes("/organizations/") && @@ -450,6 +466,14 @@ describe("resolveOrgAndIssueId", () => { const req = new Request(input, init); const url = req.url; + // getUserRegions - return empty regions to use fallback path + if (url.includes("/users/me/regions/")) { + return new Response(JSON.stringify({ regions: [] }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + // listOrganizations call if ( url.includes("/organizations/") && From 7c6aac4953edb28faa2679fa732b29c529a97782 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 13 Feb 2026 16:47:33 +0000 Subject: [PATCH 5/7] debug: add fetch logging to failing CI tests --- test/commands/issue/utils.test.ts | 53 ++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/test/commands/issue/utils.test.ts b/test/commands/issue/utils.test.ts index 3d0c7ac3..9e7b7fcd 100644 --- a/test/commands/issue/utils.test.ts +++ b/test/commands/issue/utils.test.ts @@ -306,13 +306,17 @@ describe("resolveOrgAndIssueId", () => { ); await clearProjectAliases(); + const fetchLog: string[] = []; + // @ts-expect-error - partial mock globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { const req = new Request(input, init); const url = req.url; + fetchLog.push(`FETCH: ${req.method} ${url}`); // getUserRegions - return empty regions to use fallback path if (url.includes("/users/me/regions/")) { + fetchLog.push(" → matched: /users/me/regions/"); return new Response(JSON.stringify({ regions: [] }), { status: 200, headers: { "Content-Type": "application/json" }, @@ -325,6 +329,7 @@ describe("resolveOrgAndIssueId", () => { !url.includes("/projects/") && !url.includes("/issues/") ) { + fetchLog.push(" → matched: listOrganizations"); return new Response( JSON.stringify([{ id: "1", slug: "my-org", name: "My Org" }]), { @@ -336,6 +341,7 @@ describe("resolveOrgAndIssueId", () => { // listProjects for my-org if (url.includes("organizations/my-org/projects/")) { + fetchLog.push(" → matched: listProjects for my-org"); return new Response( JSON.stringify([ { id: "123", slug: "craft", name: "Craft", platform: "javascript" }, @@ -354,6 +360,7 @@ describe("resolveOrgAndIssueId", () => { } if (url.includes("organizations/my-org/issues/CRAFT-G")) { + fetchLog.push(" → matched: issue CRAFT-G"); return new Response( JSON.stringify({ id: "777888999", @@ -372,19 +379,25 @@ describe("resolveOrgAndIssueId", () => { ); } + fetchLog.push(" → NO MATCH (returning 404)"); return new Response(JSON.stringify({ detail: "Not found" }), { status: 404, }); }; - const result = await resolveOrgAndIssueId({ - issueArg: "craft-g", - cwd: getConfigDir(), - command: "explain", - }); + try { + const result = await resolveOrgAndIssueId({ + issueArg: "craft-g", + cwd: getConfigDir(), + command: "explain", + }); - expect(result.org).toBe("my-org"); - expect(result.issueId).toBe("777888999"); + expect(result.org).toBe("my-org"); + expect(result.issueId).toBe("777888999"); + } catch (error) { + console.error("FETCH LOG:", fetchLog.join("\n")); + throw error; + } }); test("throws when project not found in any org", async () => { @@ -461,13 +474,17 @@ describe("resolveOrgAndIssueId", () => { await setOrgRegion("org2", DEFAULT_SENTRY_URL); + const fetchLog: string[] = []; + // @ts-expect-error - partial mock globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { const req = new Request(input, init); const url = req.url; + fetchLog.push(`FETCH: ${req.method} ${url}`); // getUserRegions - return empty regions to use fallback path if (url.includes("/users/me/regions/")) { + fetchLog.push(" → matched: /users/me/regions/"); return new Response(JSON.stringify({ regions: [] }), { status: 200, headers: { "Content-Type": "application/json" }, @@ -480,6 +497,7 @@ describe("resolveOrgAndIssueId", () => { !url.includes("/projects/") && !url.includes("/issues/") ) { + fetchLog.push(" → matched: listOrganizations"); return new Response( JSON.stringify([ { id: "1", slug: "org1", name: "Org 1" }, @@ -494,6 +512,7 @@ describe("resolveOrgAndIssueId", () => { // listProjects for org1 - has "common" project if (url.includes("organizations/org1/projects/")) { + fetchLog.push(" → matched: listProjects for org1"); return new Response( JSON.stringify([ { @@ -512,6 +531,7 @@ describe("resolveOrgAndIssueId", () => { // listProjects for org2 - also has "common" project if (url.includes("organizations/org2/projects/")) { + fetchLog.push(" → matched: listProjects for org2"); return new Response( JSON.stringify([ { id: "456", slug: "common", name: "Common", platform: "python" }, @@ -523,18 +543,29 @@ describe("resolveOrgAndIssueId", () => { ); } + fetchLog.push(" → NO MATCH (returning 404)"); return new Response(JSON.stringify({ detail: "Not found" }), { status: 404, }); }; - await expect( - resolveOrgAndIssueId({ + try { + await resolveOrgAndIssueId({ issueArg: "common-g", cwd: getConfigDir(), command: "explain", - }) - ).rejects.toThrow("multiple organizations"); + }); + // If we get here, the function didn't throw — log and fail + console.error("FETCH LOG (no throw):", fetchLog.join("\n")); + throw new Error("Expected resolveOrgAndIssueId to throw but it resolved"); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + if (!msg.includes("multiple organizations")) { + console.error("FETCH LOG (wrong error):", fetchLog.join("\n")); + console.error("Actual error:", msg); + } + expect(msg).toContain("multiple organizations"); + } }); test("short suffix auth error (401) propagates", async () => { From e9b10f5807b73a4f740edf53c508dfe305a57db5 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 13 Feb 2026 16:50:43 +0000 Subject: [PATCH 6/7] debug: add comprehensive logging to trace CI failure --- src/commands/issue/utils.ts | 22 ++++++++++++++++++++++ src/lib/api-client.ts | 23 +++++++++++++++++++++++ test/commands/issue/utils.test.ts | 18 ++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/src/commands/issue/utils.ts b/src/commands/issue/utils.ts index f3d3be8f..c8e47219 100644 --- a/src/commands/issue/utils.ts +++ b/src/commands/issue/utils.ts @@ -133,12 +133,34 @@ async function resolveProjectSearch( cwd ); if (aliasResult) { + // DEBUG + console.error( + "DEBUG resolveProjectSearch: alias hit for", + projectSlug, + "→", + JSON.stringify(aliasResult) + ); return aliasResult; } + // DEBUG + console.error( + "DEBUG resolveProjectSearch: no alias for", + projectSlug, + ", calling findProjectsBySlug" + ); + // 2. Search for project across all accessible orgs const projects = await findProjectsBySlug(projectSlug.toLowerCase()); + // DEBUG + console.error( + "DEBUG resolveProjectSearch: findProjectsBySlug returned", + projects.length, + "projects:", + JSON.stringify(projects.map((p) => ({ slug: p.slug, org: p.orgSlug }))) + ); + if (projects.length === 0) { throw new ContextError(`Project '${projectSlug}' not found`, commandHint, [ "No project with this slug found in any accessible organization", diff --git a/src/lib/api-client.ts b/src/lib/api-client.ts index e2146845..662ab182 100644 --- a/src/lib/api-client.ts +++ b/src/lib/api-client.ts @@ -614,19 +614,42 @@ export function listRepositories(orgSlug: string): Promise { export async function findProjectsBySlug( projectSlug: string ): Promise { + // DEBUG + console.error("DEBUG findProjectsBySlug: searching for", projectSlug); const orgs = await listOrganizations(); + // DEBUG + console.error( + "DEBUG findProjectsBySlug: listOrganizations returned", + orgs.length, + "orgs:", + JSON.stringify(orgs.map((o) => o.slug)) + ); // Search in parallel for performance const searchResults = await Promise.all( orgs.map(async (org) => { try { const projects = await listProjects(org.slug); + // DEBUG + console.error( + `DEBUG findProjectsBySlug: listProjects(${org.slug}) returned`, + projects.length, + "projects:", + JSON.stringify(projects.map((p) => p.slug)) + ); const match = projects.find((p) => p.slug === projectSlug); if (match) { return { ...match, orgSlug: org.slug }; } return null; } catch (error) { + // DEBUG + console.error( + `DEBUG findProjectsBySlug: listProjects(${org.slug}) threw:`, + error instanceof Error + ? `${error.constructor.name}: ${error.message}` + : String(error) + ); // Re-throw auth errors - user needs to login if (error instanceof AuthError) { throw error; diff --git a/test/commands/issue/utils.test.ts b/test/commands/issue/utils.test.ts index 9e7b7fcd..59b7095e 100644 --- a/test/commands/issue/utils.test.ts +++ b/test/commands/issue/utils.test.ts @@ -385,6 +385,24 @@ describe("resolveOrgAndIssueId", () => { }); }; + // Debug: Check DB state before calling + const { getDatabase: getDb } = await import("../../../src/lib/db/index.js"); + const testDb = getDb(); + const authRow = testDb + .query("SELECT * FROM auth WHERE id = 1") + .get() as Record | null; + const aliasRows = testDb + .query("SELECT * FROM project_aliases") + .all() as Record[]; + const regionRows = testDb + .query("SELECT * FROM org_regions") + .all() as Record[]; + console.error("DEBUG: auth =", JSON.stringify(authRow)); + console.error("DEBUG: aliases =", JSON.stringify(aliasRows)); + console.error("DEBUG: regions =", JSON.stringify(regionRows)); + console.error("DEBUG: SENTRY_CONFIG_DIR =", process.env.SENTRY_CONFIG_DIR); + console.error("DEBUG: cwd =", getConfigDir()); + try { const result = await resolveOrgAndIssueId({ issueArg: "craft-g", From 04bd8ea89ecc30cc9989d5d1d93662e7c8075a25 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 13 Feb 2026 17:00:17 +0000 Subject: [PATCH 7/7] fix(tests): isolate mock.module tests to prevent cross-file pollution Tests in resolver.test.ts and errors.test.ts used mock.module() to mock api-client.js, which persists across test files in Bun's single-threaded runner. When these files ran before utils.test.ts, the module mock intercepted listOrganizations/listProjects calls, returning stale mock data instead of going through the test's globalThis.fetch mock. Move these files to test/isolated/dsn/ so they run in the separate test:isolated script, matching the pattern used by resolve-target.test.ts. Also removes debug logging added in previous commits. --- src/commands/issue/utils.ts | 22 ------- src/lib/api-client.ts | 23 ------- test/commands/issue/utils.test.ts | 71 ++++----------------- test/{lib => isolated}/dsn/errors.test.ts | 0 test/{lib => isolated}/dsn/resolver.test.ts | 0 5 files changed, 11 insertions(+), 105 deletions(-) rename test/{lib => isolated}/dsn/errors.test.ts (100%) rename test/{lib => isolated}/dsn/resolver.test.ts (100%) diff --git a/src/commands/issue/utils.ts b/src/commands/issue/utils.ts index c8e47219..f3d3be8f 100644 --- a/src/commands/issue/utils.ts +++ b/src/commands/issue/utils.ts @@ -133,34 +133,12 @@ async function resolveProjectSearch( cwd ); if (aliasResult) { - // DEBUG - console.error( - "DEBUG resolveProjectSearch: alias hit for", - projectSlug, - "→", - JSON.stringify(aliasResult) - ); return aliasResult; } - // DEBUG - console.error( - "DEBUG resolveProjectSearch: no alias for", - projectSlug, - ", calling findProjectsBySlug" - ); - // 2. Search for project across all accessible orgs const projects = await findProjectsBySlug(projectSlug.toLowerCase()); - // DEBUG - console.error( - "DEBUG resolveProjectSearch: findProjectsBySlug returned", - projects.length, - "projects:", - JSON.stringify(projects.map((p) => ({ slug: p.slug, org: p.orgSlug }))) - ); - if (projects.length === 0) { throw new ContextError(`Project '${projectSlug}' not found`, commandHint, [ "No project with this slug found in any accessible organization", diff --git a/src/lib/api-client.ts b/src/lib/api-client.ts index 662ab182..e2146845 100644 --- a/src/lib/api-client.ts +++ b/src/lib/api-client.ts @@ -614,42 +614,19 @@ export function listRepositories(orgSlug: string): Promise { export async function findProjectsBySlug( projectSlug: string ): Promise { - // DEBUG - console.error("DEBUG findProjectsBySlug: searching for", projectSlug); const orgs = await listOrganizations(); - // DEBUG - console.error( - "DEBUG findProjectsBySlug: listOrganizations returned", - orgs.length, - "orgs:", - JSON.stringify(orgs.map((o) => o.slug)) - ); // Search in parallel for performance const searchResults = await Promise.all( orgs.map(async (org) => { try { const projects = await listProjects(org.slug); - // DEBUG - console.error( - `DEBUG findProjectsBySlug: listProjects(${org.slug}) returned`, - projects.length, - "projects:", - JSON.stringify(projects.map((p) => p.slug)) - ); const match = projects.find((p) => p.slug === projectSlug); if (match) { return { ...match, orgSlug: org.slug }; } return null; } catch (error) { - // DEBUG - console.error( - `DEBUG findProjectsBySlug: listProjects(${org.slug}) threw:`, - error instanceof Error - ? `${error.constructor.name}: ${error.message}` - : String(error) - ); // Re-throw auth errors - user needs to login if (error instanceof AuthError) { throw error; diff --git a/test/commands/issue/utils.test.ts b/test/commands/issue/utils.test.ts index 59b7095e..3d0c7ac3 100644 --- a/test/commands/issue/utils.test.ts +++ b/test/commands/issue/utils.test.ts @@ -306,17 +306,13 @@ describe("resolveOrgAndIssueId", () => { ); await clearProjectAliases(); - const fetchLog: string[] = []; - // @ts-expect-error - partial mock globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { const req = new Request(input, init); const url = req.url; - fetchLog.push(`FETCH: ${req.method} ${url}`); // getUserRegions - return empty regions to use fallback path if (url.includes("/users/me/regions/")) { - fetchLog.push(" → matched: /users/me/regions/"); return new Response(JSON.stringify({ regions: [] }), { status: 200, headers: { "Content-Type": "application/json" }, @@ -329,7 +325,6 @@ describe("resolveOrgAndIssueId", () => { !url.includes("/projects/") && !url.includes("/issues/") ) { - fetchLog.push(" → matched: listOrganizations"); return new Response( JSON.stringify([{ id: "1", slug: "my-org", name: "My Org" }]), { @@ -341,7 +336,6 @@ describe("resolveOrgAndIssueId", () => { // listProjects for my-org if (url.includes("organizations/my-org/projects/")) { - fetchLog.push(" → matched: listProjects for my-org"); return new Response( JSON.stringify([ { id: "123", slug: "craft", name: "Craft", platform: "javascript" }, @@ -360,7 +354,6 @@ describe("resolveOrgAndIssueId", () => { } if (url.includes("organizations/my-org/issues/CRAFT-G")) { - fetchLog.push(" → matched: issue CRAFT-G"); return new Response( JSON.stringify({ id: "777888999", @@ -379,43 +372,19 @@ describe("resolveOrgAndIssueId", () => { ); } - fetchLog.push(" → NO MATCH (returning 404)"); return new Response(JSON.stringify({ detail: "Not found" }), { status: 404, }); }; - // Debug: Check DB state before calling - const { getDatabase: getDb } = await import("../../../src/lib/db/index.js"); - const testDb = getDb(); - const authRow = testDb - .query("SELECT * FROM auth WHERE id = 1") - .get() as Record | null; - const aliasRows = testDb - .query("SELECT * FROM project_aliases") - .all() as Record[]; - const regionRows = testDb - .query("SELECT * FROM org_regions") - .all() as Record[]; - console.error("DEBUG: auth =", JSON.stringify(authRow)); - console.error("DEBUG: aliases =", JSON.stringify(aliasRows)); - console.error("DEBUG: regions =", JSON.stringify(regionRows)); - console.error("DEBUG: SENTRY_CONFIG_DIR =", process.env.SENTRY_CONFIG_DIR); - console.error("DEBUG: cwd =", getConfigDir()); - - try { - const result = await resolveOrgAndIssueId({ - issueArg: "craft-g", - cwd: getConfigDir(), - command: "explain", - }); + const result = await resolveOrgAndIssueId({ + issueArg: "craft-g", + cwd: getConfigDir(), + command: "explain", + }); - expect(result.org).toBe("my-org"); - expect(result.issueId).toBe("777888999"); - } catch (error) { - console.error("FETCH LOG:", fetchLog.join("\n")); - throw error; - } + expect(result.org).toBe("my-org"); + expect(result.issueId).toBe("777888999"); }); test("throws when project not found in any org", async () => { @@ -492,17 +461,13 @@ describe("resolveOrgAndIssueId", () => { await setOrgRegion("org2", DEFAULT_SENTRY_URL); - const fetchLog: string[] = []; - // @ts-expect-error - partial mock globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { const req = new Request(input, init); const url = req.url; - fetchLog.push(`FETCH: ${req.method} ${url}`); // getUserRegions - return empty regions to use fallback path if (url.includes("/users/me/regions/")) { - fetchLog.push(" → matched: /users/me/regions/"); return new Response(JSON.stringify({ regions: [] }), { status: 200, headers: { "Content-Type": "application/json" }, @@ -515,7 +480,6 @@ describe("resolveOrgAndIssueId", () => { !url.includes("/projects/") && !url.includes("/issues/") ) { - fetchLog.push(" → matched: listOrganizations"); return new Response( JSON.stringify([ { id: "1", slug: "org1", name: "Org 1" }, @@ -530,7 +494,6 @@ describe("resolveOrgAndIssueId", () => { // listProjects for org1 - has "common" project if (url.includes("organizations/org1/projects/")) { - fetchLog.push(" → matched: listProjects for org1"); return new Response( JSON.stringify([ { @@ -549,7 +512,6 @@ describe("resolveOrgAndIssueId", () => { // listProjects for org2 - also has "common" project if (url.includes("organizations/org2/projects/")) { - fetchLog.push(" → matched: listProjects for org2"); return new Response( JSON.stringify([ { id: "456", slug: "common", name: "Common", platform: "python" }, @@ -561,29 +523,18 @@ describe("resolveOrgAndIssueId", () => { ); } - fetchLog.push(" → NO MATCH (returning 404)"); return new Response(JSON.stringify({ detail: "Not found" }), { status: 404, }); }; - try { - await resolveOrgAndIssueId({ + await expect( + resolveOrgAndIssueId({ issueArg: "common-g", cwd: getConfigDir(), command: "explain", - }); - // If we get here, the function didn't throw — log and fail - console.error("FETCH LOG (no throw):", fetchLog.join("\n")); - throw new Error("Expected resolveOrgAndIssueId to throw but it resolved"); - } catch (error: unknown) { - const msg = error instanceof Error ? error.message : String(error); - if (!msg.includes("multiple organizations")) { - console.error("FETCH LOG (wrong error):", fetchLog.join("\n")); - console.error("Actual error:", msg); - } - expect(msg).toContain("multiple organizations"); - } + }) + ).rejects.toThrow("multiple organizations"); }); test("short suffix auth error (401) propagates", async () => { diff --git a/test/lib/dsn/errors.test.ts b/test/isolated/dsn/errors.test.ts similarity index 100% rename from test/lib/dsn/errors.test.ts rename to test/isolated/dsn/errors.test.ts diff --git a/test/lib/dsn/resolver.test.ts b/test/isolated/dsn/resolver.test.ts similarity index 100% rename from test/lib/dsn/resolver.test.ts rename to test/isolated/dsn/resolver.test.ts