Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 53 additions & 35 deletions test/commands/issue/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@
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 {
cleanupTestDir,
createTestConfigDir,
lockConfigDir,
lockFetch,
} from "../../helpers.js";

describe("buildCommandHint", () => {
test("suggests <org>/ID for numeric IDs", () => {
Expand Down Expand Up @@ -49,13 +54,22 @@

let testConfigDir: string;
let originalFetch: typeof globalThis.fetch;
let savedConfigDir: string | undefined;
let unlockEnv: (() => void) | undefined;
let unlockFetchFn: (() => void) | undefined;

beforeEach(async () => {
savedConfigDir = process.env[CONFIG_DIR_ENV_VAR];
// 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;

// Lock the env var so concurrent test files cannot change it during our test.
// This prevents the DB singleton from auto-invalidating mid-test.
unlockEnv = lockConfigDir(testConfigDir);

originalFetch = globalThis.fetch;
await setAuthToken("test-token");
// Pre-populate region cache for orgs used in tests to avoid region resolution API calls
Expand All @@ -66,14 +80,23 @@
});

afterEach(async () => {
// Unlock fetch first (if locked by this test)
unlockFetchFn?.();
unlockFetchFn = undefined;
globalThis.fetch = originalFetch;
// Unlock env var, then restore
unlockEnv?.();
if (savedConfigDir !== undefined) {
process.env[CONFIG_DIR_ENV_VAR] = savedConfigDir;
} else {
delete process.env[CONFIG_DIR_ENV_VAR];
}
await cleanupTestDir(testConfigDir);
});

describe("resolveOrgAndIssueId", () => {
test("throws for numeric ID (org cannot be resolved)", async () => {
// @ts-expect-error - partial mock
globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {
unlockFetchFn = lockFetch(async (input, init) => {
const req = new Request(input, init);
const url = req.url;

Expand All @@ -100,7 +123,7 @@
return new Response(JSON.stringify({ detail: "Not found" }), {
status: 404,
});
};
});

// Numeric IDs don't have org context, so resolveOrgAndIssueId should throw
await expect(
Expand All @@ -113,8 +136,7 @@
});

test("resolves explicit org prefix (org/ISSUE-ID)", async () => {
// @ts-expect-error - partial mock
globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {
unlockFetchFn = lockFetch(async (input, init) => {
const req = new Request(input, init);
const url = req.url;

Expand All @@ -140,7 +162,7 @@
return new Response(JSON.stringify({ detail: "Not found" }), {
status: 404,
});
};
});

const result = await resolveOrgAndIssueId({
issueArg: "my-org/PROJECT-ABC",
Expand All @@ -165,8 +187,7 @@
""
);

// @ts-expect-error - partial mock
globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {
unlockFetchFn = lockFetch(async (input, init) => {
const req = new Request(input, init);
const url = req.url;

Expand All @@ -192,7 +213,7 @@
return new Response(JSON.stringify({ detail: "Not found" }), {
status: 404,
});
};
});

const result = await resolveOrgAndIssueId({
issueArg: "f-g",
Expand All @@ -205,8 +226,7 @@
});

test("resolves explicit org prefix with project-suffix (e.g., 'org1/dashboard-4y')", async () => {
// @ts-expect-error - partial mock
globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {
unlockFetchFn = lockFetch(async (input, init) => {
const req = new Request(input, init);
const url = req.url;

Expand All @@ -233,7 +253,7 @@
return new Response(JSON.stringify({ detail: "Not found" }), {
status: 404,
});
};
});

const result = await resolveOrgAndIssueId({
issueArg: "org1/dashboard-4y",
Expand All @@ -249,8 +269,7 @@
const { setDefaults } = await import("../../../src/lib/db/defaults.js");
await setDefaults("my-org", "my-project");

// @ts-expect-error - partial mock
globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {
unlockFetchFn = lockFetch(async (input, init) => {
const req = new Request(input, init);
const url = req.url;

Expand All @@ -276,7 +295,7 @@
return new Response(JSON.stringify({ detail: "Not found" }), {
status: 404,
});
};
});

const result = await resolveOrgAndIssueId({
issueArg: "G",
Expand Down Expand Up @@ -310,8 +329,7 @@
);
await clearProjectAliases();

// @ts-expect-error - partial mock
globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {
unlockFetchFn = lockFetch(async (input, init) => {
const req = new Request(input, init);
const url = req.url;

Expand Down Expand Up @@ -371,7 +389,7 @@
return new Response(JSON.stringify({ detail: "Not found" }), {
status: 404,
});
};
});

const result = await resolveOrgAndIssueId({
issueArg: "craft-g",
Expand All @@ -389,8 +407,7 @@
);
await clearProjectAliases();

// @ts-expect-error - partial mock
globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {
unlockFetchFn = lockFetch(async (input, init) => {
const req = new Request(input, init);
const url = req.url;

Expand Down Expand Up @@ -430,7 +447,7 @@
return new Response(JSON.stringify({ detail: "Not found" }), {
status: 404,
});
};
});

await expect(
resolveOrgAndIssueId({
Expand All @@ -449,8 +466,7 @@

await setOrgRegion("org2", DEFAULT_SENTRY_URL);

// @ts-expect-error - partial mock
globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {
unlockFetchFn = lockFetch(async (input, init) => {
const req = new Request(input, init);
const url = req.url;

Expand Down Expand Up @@ -506,7 +522,7 @@
return new Response(JSON.stringify({ detail: "Not found" }), {
status: 404,
});
};
});

await expect(
resolveOrgAndIssueId({
Expand All @@ -514,18 +530,19 @@
cwd: testConfigDir,
command: "explain",
})
).rejects.toThrow("multiple organizations");

Check failure on line 533 in test/commands/issue/utils.test.ts

View workflow job for this annotation

GitHub Actions / Unit Tests

error: expect(received).toThrow(expected)

Expected substring: "multiple organizations" Received message: "Project 'common' not found is required.\n\nSpecify it using:\n sentry issue explain <org>/common-g\n\nOr:\n - No project with this slug found in any accessible organization" at <anonymous> (/home/runner/work/cli/cli/test/commands/issue/utils.test.ts:533:15)
});

test("short suffix auth error (401) propagates", async () => {
const { setDefaults } = await import("../../../src/lib/db/defaults.js");
await setDefaults("my-org", "my-project");

// @ts-expect-error - partial mock
globalThis.fetch = async () =>
new Response(JSON.stringify({ detail: "Unauthorized" }), {
status: 401,
});
unlockFetchFn = lockFetch(
async () =>
new Response(JSON.stringify({ detail: "Unauthorized" }), {
status: 401,
})
);

// Auth errors should propagate
await expect(
Expand All @@ -541,11 +558,12 @@
const { setDefaults } = await import("../../../src/lib/db/defaults.js");
await setDefaults("my-org", "my-project");

// @ts-expect-error - partial mock
globalThis.fetch = async () =>
new Response(JSON.stringify({ detail: "Internal Server Error" }), {
status: 500,
});
unlockFetchFn = lockFetch(
async () =>
new Response(JSON.stringify({ detail: "Internal Server Error" }), {
status: 500,
})
);

// Server errors should propagate
await expect(
Expand Down
104 changes: 104 additions & 0 deletions test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,107 @@ type FetchMockFn =
export function mockFetch(fn: FetchMockFn): typeof fetch {
return fn as unknown as typeof fetch;
}

/**
* Module-level lock state for SENTRY_CONFIG_DIR.
*
* We replace process.env with a Proxy that intercepts reads, writes,
* and deletes of SENTRY_CONFIG_DIR. When locked, the proxy returns the
* locked value and silently ignores mutations. When unlocked, all
* operations pass through to the real env object.
*
* A Proxy is used instead of Object.defineProperty because:
* - configurable:true descriptors can be removed by `delete` (other
* test files do `delete process.env[CONFIG_DIR_ENV_VAR]` in afterEach)
* - configurable:false descriptors cause `delete` to throw in Bun,
* breaking other test files
*
* The Proxy is installed once and persists for the process lifetime.
* Lock/unlock just toggles the module-level state variables.
*/
let configDirLocked = false;
let configDirLockedValue: string | undefined;
let envProxyInstalled = false;

function installEnvProxy(): void {
if (envProxyInstalled) return;
const realEnv = process.env;
process.env = new Proxy(realEnv, {
get(target, prop, receiver) {
if (prop === "SENTRY_CONFIG_DIR" && configDirLocked) {
return configDirLockedValue;
}
return Reflect.get(target, prop, receiver);
},
set(target, prop, value) {
if (prop === "SENTRY_CONFIG_DIR" && configDirLocked) {
return true; // Silently ignore
}
return Reflect.set(target, prop, value);
},
deleteProperty(target, prop) {
if (prop === "SENTRY_CONFIG_DIR" && configDirLocked) {
return true; // Silently ignore
}
return Reflect.deleteProperty(target, prop);
},
});
envProxyInstalled = true;
}

/**
* Lock SENTRY_CONFIG_DIR so concurrent test files cannot change it.
*
* Bun runs test files concurrently in a single process, sharing
* `process.env`. Between any `await` point inside our test, another
* file's beforeEach/afterEach can mutate the env var. The DB singleton
* auto-invalidates when the config dir changes, so even a momentary
* mutation causes getDatabase() to open the wrong DB and lose data.
*
* Uses a Proxy on process.env that intercepts get/set/delete of the
* config dir env var. When locked, returns the locked value and ignores
* mutations. When unlocked, all operations pass through normally.
*
* @param configDir - The config directory path to lock
* @returns Unlock function to call in afterEach
*/
export function lockConfigDir(configDir: string): () => void {
installEnvProxy();
configDirLocked = true;
configDirLockedValue = configDir;

return () => {
configDirLocked = false;
};
}

/**
* Lock globalThis.fetch to a mock handler so concurrent test files
* cannot replace it between async boundaries.
*
* Uses configurable: true since concurrent test files use assignment
* (not delete) for fetch, and other test files (e.g. api-client) need
* to assign their own mock via globalThis.fetch = ... without going
* through this lock.
*
* @param fn - The fetch mock implementation
* @returns Unlock function to call in afterEach
*/
export function lockFetch(
fn: (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>
): () => void {
Object.defineProperty(globalThis, "fetch", {
get() {
return fn;
},
set() {
// Silently ignore writes from other test files
},
configurable: true,
enumerable: true,
});

return () => {
delete (globalThis as Record<string, unknown>).fetch;
};
}
7 changes: 7 additions & 0 deletions test/lib/api-client.multiregion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import { cleanupTestDir, createTestConfigDir } from "../helpers.js";

let testConfigDir: string;
let originalFetch: typeof globalThis.fetch;
let savedConfigDir: string | undefined;

beforeEach(async () => {
savedConfigDir = process.env[CONFIG_DIR_ENV_VAR];
testConfigDir = await createTestConfigDir("test-multiregion-");
process.env[CONFIG_DIR_ENV_VAR] = testConfigDir;

Expand All @@ -42,6 +44,11 @@ afterEach(async () => {
// Restore original fetch
globalThis.fetch = originalFetch;
closeDatabase();
if (savedConfigDir !== undefined) {
process.env[CONFIG_DIR_ENV_VAR] = savedConfigDir;
} else {
delete process.env[CONFIG_DIR_ENV_VAR];
}
await cleanupTestDir(testConfigDir);
});

Expand Down
7 changes: 7 additions & 0 deletions test/lib/api-client.seer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ import { cleanupTestDir, createTestConfigDir } from "../helpers.js";
// Test config directory
let testConfigDir: string;
let originalFetch: typeof globalThis.fetch;
let savedConfigDir: string | undefined;

beforeEach(async () => {
savedConfigDir = process.env[CONFIG_DIR_ENV_VAR];
testConfigDir = await createTestConfigDir("test-seer-api-");
process.env[CONFIG_DIR_ENV_VAR] = testConfigDir;

Expand All @@ -35,6 +37,11 @@ beforeEach(async () => {
afterEach(async () => {
// Restore original fetch
globalThis.fetch = originalFetch;
if (savedConfigDir !== undefined) {
process.env[CONFIG_DIR_ENV_VAR] = savedConfigDir;
} else {
delete process.env[CONFIG_DIR_ENV_VAR];
}
await cleanupTestDir(testConfigDir);
});

Expand Down
Loading
Loading