Skip to content

Commit 44d604c

Browse files
authored
fix(test): fix CI hang, auth guard tests, and PR #610 test rewrite (#616)
## Summary Fixes the CI unit test hang and ~200 test failures caused by three merged PRs (#610, #611). Supersedes #612 with a cleaner approach. ### 1. CI Hang — telemetry `beforeExit` handler leak `initSentry(false)` in test `afterEach` hooks never cleaned up the `beforeExit` handler registered by a prior `initSentry(true)` call — the removal was gated on `client?.getOptions().enabled`. SDK-internal handlers from `enableLogs` and `sendClientReports` also accumulated on re-init. **Fix** (src/lib/telemetry.ts): - Close previous Sentry client before re-init (`Sentry.getClient()?.close(0)`) - Move handler removal outside the `enabled` guard - Gate `enableLogs` and `sendClientReports` on `enabled` (not just `libraryMode`) - Snapshot and clean up `ProcessSession` `beforeExit` listeners in telemetry.test.ts ### 2. Auth guard test failures (PR #611) `buildCommand` now calls `getAuthConfig()` before every command. Tests had no auth token. **Fix**: - Set fake `SENTRY_AUTH_TOKEN` in test/preload.ts (blocked by global fetch mock) - Add `auth: false` to all 34 `buildCommand` calls in command.test.ts - Save/clear/restore `SENTRY_AUTH_TOKEN` in 9 test files that verify unauthenticated behavior - Add `getAuthConfig` mock + `resolveOrgProjectFromArg` mock in log/list tests ### 3. Dead code tests (PR #610) PR #610 moved org resolution and DSN detection from `createSentryProject` to `wizard-runner`'s `resolvePreSpinnerOptions`. Tests that exercised those flows through `handleLocalOp` hit the new `!options.org` guard. **Fix**: Rewrote 15 tests to call `resolveOrgSlug` and `detectExistingProject` directly, plus 2 new tests for `createSentryProject`'s existing-project check. No test coverage lost. ### 4. Mtime test flakiness Replaced `Bun.sleep(10)` with explicit `utimes()` calls in dsn-cache and project-root-cache tests for deterministic mtime differences on Linux CI. ## Test plan - `bun test` — 630 pass, 4 fail (pre-existing: 3 resolve-target timeouts from unmocked fetch, 1 log/list failure from missing generated api-schema.json) - `bun run lint` — clean (1 pre-existing warning) Made with [Cursor](https://cursor.com)
1 parent 0d32239 commit 44d604c

18 files changed

+414
-224
lines changed

src/lib/telemetry.ts

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ const LIBRARY_EXCLUDED_INTEGRATIONS = new Set([
275275
...EXCLUDED_INTEGRATIONS,
276276
"OnUncaughtException", // process.on('uncaughtException')
277277
"OnUnhandledRejection", // process.on('unhandledRejection')
278-
"ProcessSession", // process.on('beforeExit')
278+
"ProcessSession", // process.on('beforeExit') — anonymous handler, no cleanup
279279
"Http", // diagnostics_channel + trace headers
280280
"NodeFetch", // diagnostics_channel + trace headers
281281
"FunctionToString", // wraps Function.prototype.toString
@@ -355,6 +355,13 @@ export function initSentry(
355355
const libraryMode = options?.libraryMode ?? false;
356356
const environment = getEnv().NODE_ENV ?? "development";
357357

358+
// Close the previous client to clean up its internal timers and beforeExit
359+
// handlers (client report flusher interval, log flush listener). Without
360+
// this, re-initializing the SDK (e.g., in tests) leaks setInterval handles
361+
// that keep the event loop alive and prevent the process from exiting.
362+
// close(0) removes listeners synchronously; we don't need to await the flush.
363+
Sentry.getClient()?.close(0);
364+
358365
const client = Sentry.init({
359366
dsn: SENTRY_CLI_DSN,
360367
enabled,
@@ -374,10 +381,12 @@ export function initSentry(
374381
},
375382
environment,
376383
// Enable Sentry structured logs for non-exception telemetry (e.g., unexpected input warnings).
377-
// Disabled in library mode — logs use timers/beforeExit that pollute the host process.
378-
enableLogs: !libraryMode,
379-
// Disable client reports in library mode — they use timers/beforeExit.
380-
...(libraryMode && { sendClientReports: false }),
384+
// Disabled when telemetry is off or in library mode — the SDK registers
385+
// beforeExit handlers for log flushing that keep the event loop alive.
386+
enableLogs: enabled && !libraryMode,
387+
// Disable client reports when telemetry is off or in library mode — the SDK
388+
// registers a setInterval + beforeExit handler that keep the event loop alive.
389+
...((libraryMode || !enabled) && { sendClientReports: false }),
381390
// Sample all events for CLI telemetry (low volume)
382391
tracesSampleRate: 1,
383392
sampleRate: 1,
@@ -405,6 +414,12 @@ export function initSentry(
405414
},
406415
});
407416

417+
// Always remove our own previous handler on re-init.
418+
if (currentBeforeExitHandler) {
419+
process.removeListener("beforeExit", currentBeforeExitHandler);
420+
currentBeforeExitHandler = null;
421+
}
422+
408423
if (client?.getOptions().enabled) {
409424
const isBun = typeof process.versions.bun !== "undefined";
410425
const runtime = isBun ? "bun" : "node";
@@ -441,14 +456,7 @@ export function initSentry(
441456
//
442457
// Skipped in library mode — the host owns the process lifecycle.
443458
// The library entry point calls client.flush() manually after completion.
444-
//
445-
// Replace previous handler on re-init (e.g., auto-login retry calls
446-
// withTelemetry → initSentry twice) to avoid duplicate handlers with
447-
// independent re-entry guards and stale client references.
448459
if (!libraryMode) {
449-
if (currentBeforeExitHandler) {
450-
process.removeListener("beforeExit", currentBeforeExitHandler);
451-
}
452460
currentBeforeExitHandler = createBeforeExitHandler(client);
453461
process.on("beforeExit", currentBeforeExitHandler);
454462
}

test/commands/auth/logout.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ describe("logoutCommand.func", () => {
125125
test("env token (SENTRY_TOKEN): shows correct env var name in error", async () => {
126126
isAuthenticatedSpy.mockReturnValue(true);
127127
isEnvTokenActiveSpy.mockReturnValue(true);
128+
// Clear SENTRY_AUTH_TOKEN so SENTRY_TOKEN takes priority
129+
const savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
130+
delete process.env.SENTRY_AUTH_TOKEN;
128131
// Set env var directly — getActiveEnvVarName() reads env vars via getEnvToken()
129132
process.env.SENTRY_TOKEN = "sntrys_token_456";
130133
const { context } = createContext();
@@ -138,6 +141,9 @@ describe("logoutCommand.func", () => {
138141
expect(msg).toContain("SENTRY_TOKEN");
139142
} finally {
140143
delete process.env.SENTRY_TOKEN;
144+
if (savedAuthToken !== undefined) {
145+
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
146+
}
141147
}
142148
expect(clearAuthSpy).not.toHaveBeenCalled();
143149
});

test/commands/auth/refresh.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ describe("refreshCommand.func", () => {
8484

8585
test("env token (SENTRY_TOKEN): throws AuthError with SENTRY_TOKEN in message", async () => {
8686
isEnvTokenActiveSpy.mockReturnValue(true);
87+
// Clear SENTRY_AUTH_TOKEN so SENTRY_TOKEN takes priority
88+
const savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
89+
delete process.env.SENTRY_AUTH_TOKEN;
8790
// Set env var directly — getActiveEnvVarName() reads env vars via getEnvToken()
8891
process.env.SENTRY_TOKEN = "sntrys_token_456";
8992

@@ -99,6 +102,9 @@ describe("refreshCommand.func", () => {
99102
expect((err as AuthError).message).not.toContain("SENTRY_AUTH_TOKEN");
100103
} finally {
101104
delete process.env.SENTRY_TOKEN;
105+
if (savedAuthToken !== undefined) {
106+
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
107+
}
102108
}
103109

104110
expect(refreshTokenSpy).not.toHaveBeenCalled();

test/commands/auth/whoami.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,24 @@ describe("whoamiCommand.func", () => {
8484
});
8585

8686
describe("unauthenticated", () => {
87+
let getAuthConfigSpy: ReturnType<typeof spyOn>;
88+
let savedAuthToken: string | undefined;
89+
90+
beforeEach(() => {
91+
savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
92+
delete process.env.SENTRY_AUTH_TOKEN;
93+
getAuthConfigSpy = spyOn(dbAuth, "getAuthConfig").mockReturnValue(
94+
undefined
95+
);
96+
});
97+
98+
afterEach(() => {
99+
getAuthConfigSpy.mockRestore();
100+
if (savedAuthToken !== undefined) {
101+
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
102+
}
103+
});
104+
87105
test("throws AuthError(not_authenticated) when no token stored", async () => {
88106
isAuthenticatedSpy.mockReturnValue(false);
89107

test/commands/log/list.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ import {
3030
import { listCommand } from "../../../src/commands/log/list.js";
3131
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
3232
import * as apiClient from "../../../src/lib/api-client.js";
33+
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
34+
import * as dbAuth from "../../../src/lib/db/auth.js";
3335
import {
3436
AuthError,
3537
ContextError,
@@ -237,6 +239,23 @@ const newerLogs: SentryLog[] = [
237239
},
238240
];
239241

242+
// ============================================================================
243+
// Auth setup — mock getAuthConfig for all tests (auth guard added in #611)
244+
// ============================================================================
245+
246+
let getAuthConfigSpy: ReturnType<typeof spyOn>;
247+
248+
beforeEach(() => {
249+
getAuthConfigSpy = spyOn(dbAuth, "getAuthConfig").mockReturnValue({
250+
token: "sntrys_test",
251+
source: "oauth" as const,
252+
});
253+
});
254+
255+
afterEach(() => {
256+
getAuthConfigSpy.mockRestore();
257+
});
258+
240259
// ============================================================================
241260
// Standard mode (project-scoped, no trace-id positional)
242261
// ============================================================================
@@ -837,7 +856,14 @@ describe("listCommand.func — flag validation", () => {
837856

838857
test("allows --sort newest with --follow", async () => {
839858
// Should not throw ValidationError — the error (if any) comes from
840-
// downstream resolution, not flag validation.
859+
// downstream resolution, not flag validation. Mock resolution to reject
860+
// with a non-ValidationError so we can verify flag validation passed.
861+
const resolveOrgProjectSpy = spyOn(
862+
resolveTarget,
863+
"resolveOrgProjectFromArg"
864+
).mockRejectedValueOnce(
865+
new ContextError("Organization", "sentry log list")
866+
);
841867
const { context } = createMockContext();
842868
const func = await listCommand.loader();
843869
await expect(
@@ -847,6 +873,7 @@ describe("listCommand.func — flag validation", () => {
847873
"my-org/my-project"
848874
)
849875
).rejects.not.toThrow(ValidationError);
876+
resolveOrgProjectSpy.mockRestore();
850877
});
851878
});
852879

test/commands/project/list.test.ts

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -970,7 +970,16 @@ describe("fetchOrgProjectsSafe", () => {
970970
// Clear auth token so the API client throws AuthError before making any request
971971
await clearAuth();
972972

973-
await expect(fetchOrgProjectsSafe("myorg")).rejects.toThrow(AuthError);
973+
const savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
974+
delete process.env.SENTRY_AUTH_TOKEN;
975+
976+
try {
977+
await expect(fetchOrgProjectsSafe("myorg")).rejects.toThrow(AuthError);
978+
} finally {
979+
if (savedAuthToken !== undefined) {
980+
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
981+
}
982+
}
974983
});
975984
});
976985

@@ -1234,13 +1243,22 @@ describe("handleAutoDetect", () => {
12341243
// Clear auth so getAuthToken() throws AuthError before any fetch
12351244
await clearAuth();
12361245

1237-
await expect(
1238-
handleAutoDetect("/tmp/test-project", {
1239-
limit: 30,
1240-
json: true,
1241-
fresh: false,
1242-
})
1243-
).rejects.toThrow(AuthError);
1246+
const savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
1247+
delete process.env.SENTRY_AUTH_TOKEN;
1248+
1249+
try {
1250+
await expect(
1251+
handleAutoDetect("/tmp/test-project", {
1252+
limit: 30,
1253+
json: true,
1254+
fresh: false,
1255+
})
1256+
).rejects.toThrow(AuthError);
1257+
} finally {
1258+
if (savedAuthToken !== undefined) {
1259+
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
1260+
}
1261+
}
12441262
});
12451263

12461264
test("slow path: uses full fetch when platform filter is active", async () => {

test/fixture.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ export function createE2EContext(
114114
run: (args: string[]) =>
115115
runCli(args, {
116116
env: {
117+
SENTRY_AUTH_TOKEN: "",
118+
SENTRY_TOKEN: "",
117119
[CONFIG_DIR_ENV_VAR]: configDir,
118120
SENTRY_URL: serverUrl,
119121
SENTRY_CLI_NO_TELEMETRY: "1",

test/lib/api-client.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,17 @@ describe("401 retry behavior", () => {
108108
// Note: These tests use rawApiRequest which goes to control silo (sentry.io)
109109
// and supports 401 retry with token refresh.
110110

111+
let savedAuthToken: string | undefined;
112+
beforeEach(() => {
113+
savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
114+
delete process.env.SENTRY_AUTH_TOKEN;
115+
});
116+
afterEach(() => {
117+
if (savedAuthToken !== undefined) {
118+
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
119+
}
120+
});
121+
111122
test("retries request with new token on 401 response", async () => {
112123
const requests: RequestLog[] = [];
113124

0 commit comments

Comments
 (0)