From 5b51f9d6e49ed8c0e854a8669f50d63f004504b2 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 24 Apr 2026 16:17:19 +0000 Subject: [PATCH] test: collapse test/isolated/ into test/lib and test/commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #839. With `bun test --isolate` giving each file a fresh module graph, the test/isolated/ split that existed to work around `mock.module()` pollution (oven-sh/cli#258) is no longer needed. ## Merged into existing files Per-feature tests moved into their natural counterparts: - isolated/brew-upgrade.test.ts → test/lib/upgrade.test.ts - isolated/log-view-prompt.test.ts → test/commands/log/view.test.ts - isolated/login-reauth.test.ts → test/commands/auth/login.test.ts - isolated/project-delete-confirm.test.ts → test/commands/project/delete.test.ts - isolated/set-commits-auto.test.ts → test/lib/api/releases.test.ts - isolated/set-commits-auto-no-remote.test.ts → test/lib/api/releases.test.ts (merged as a single setCommitsAuto describe; getRepositoryName is now a controllable `mock()`) - isolated/dsn/project-root.test.ts → test/lib/dsn/project-root.test.ts Pattern: `mock.module()` at the top of the merged file, followed by a dynamic `await import()` for the code under test so the mocked bindings win. The rest of the file's static imports stay as-is. Also unblocks a previously-deferred stub in `test/lib/api/releases.test.ts` that said "setCommitsAuto tests live in test/isolated/ because they require mock.module() for git helpers". ## Moved as sibling files Two isolated files tested the same subjects as existing test files but with incompatible mocking strategies. Under `--isolate` they're safe to keep as siblings since module state doesn't leak: - isolated/resolve-target.test.ts → test/lib/resolve-target.mocked.test.ts - isolated/delta-upgrade.test.ts → test/lib/delta-upgrade.mocked.test.ts Both existing `resolve-target.test.ts` and `delta-upgrade.test.ts` use real DB + mocked fetch; the `.mocked.` siblings stub out every collaborator via `mock.module()`. The two styles can coexist in the same directory now that module graphs are per-file. ## New files (no prior counterpart) Three isolated DSN helpers had no target file in test/lib/dsn/: - isolated/dsn/errors.test.ts → test/lib/dsn/errors.test.ts - isolated/dsn/fs-utils.test.ts → test/lib/dsn/fs-utils.test.ts - isolated/dsn/resolver.test.ts → test/lib/dsn/resolver.test.ts ## Removed `scopedFetchMock` helper `test/lib/sentry-client.test.ts` previously wrapped all its fetch mocks with `scopedFetchMock(marker, handler)` to filter out foreign fetch calls that leaked from other test files (CI run 24835339085). Under `bun test --isolate` each file gets a fresh `globalThis.fetch`, so cross-file leakage is eliminated. The 6 call sites now use `mockFetch()` directly; the helper and its `urlOf()` dependency are gone (~30 lines). ## Other cleanups - Removed `test/isolated/` directory - Updated `package.json` `test:unit` to drop `test/isolated` from the argument list (files now live under `test/lib/` and `test/commands/`) - `test/lib/dsn/project-root.test.ts`: `noopSpan` stub now includes `setAttributes` (production code calls both `setAttribute` and `setAttributes`) - Replaced several `() => {}` test stubs with named `noop` functions to satisfy `lint/suspicious/noEmptyBlockStatements` without per-line biome-ignore comments ## Verification - `bun run test:unit` — 5920 pass / 0 fail across 253 files (was 260 — merges consolidated 12 files into 5 new + 7 existing) - 3 consecutive stress runs clean - `bun run typecheck`, `bun run lint` - `bun run lint:fix` applied one formatting tweak --- package.json | 2 +- test/commands/auth/login.test.ts | 256 ++++++++++++- test/commands/log/view.test.ts | 162 +++++++- test/commands/project/delete.test.ts | 205 +++++++++- test/isolated/brew-upgrade.test.ts | 360 ------------------ test/isolated/dsn/project-root.test.ts | 79 ---- test/isolated/log-view-prompt.test.ts | 177 --------- test/isolated/login-reauth.test.ts | 272 ------------- test/isolated/project-delete-confirm.test.ts | 219 ----------- .../set-commits-auto-no-remote.test.ts | 31 -- test/isolated/set-commits-auto.test.ts | 212 ----------- test/lib/api/releases.test.ts | 189 ++++++++- .../delta-upgrade.mocked.test.ts} | 14 +- test/{isolated => lib}/dsn/errors.test.ts | 0 test/{isolated => lib}/dsn/fs-utils.test.ts | 0 test/lib/dsn/project-root.test.ts | 76 +++- test/{isolated => lib}/dsn/resolver.test.ts | 0 .../resolve-target.mocked.test.ts} | 23 +- test/lib/sentry-client.test.ts | 49 +-- test/lib/upgrade.test.ts | 357 ++++++++++++++++- 20 files changed, 1240 insertions(+), 1443 deletions(-) delete mode 100644 test/isolated/brew-upgrade.test.ts delete mode 100644 test/isolated/dsn/project-root.test.ts delete mode 100644 test/isolated/log-view-prompt.test.ts delete mode 100644 test/isolated/login-reauth.test.ts delete mode 100644 test/isolated/project-delete-confirm.test.ts delete mode 100644 test/isolated/set-commits-auto-no-remote.test.ts delete mode 100644 test/isolated/set-commits-auto.test.ts rename test/{isolated/delta-upgrade.test.ts => lib/delta-upgrade.mocked.test.ts} (96%) rename test/{isolated => lib}/dsn/errors.test.ts (100%) rename test/{isolated => lib}/dsn/fs-utils.test.ts (100%) rename test/{isolated => lib}/dsn/resolver.test.ts (100%) rename test/{isolated/resolve-target.test.ts => lib/resolve-target.mocked.test.ts} (97%) diff --git a/package.json b/package.json index 660b0e37b..8dcf0a878 100644 --- a/package.json +++ b/package.json @@ -80,7 +80,7 @@ "lint": "bunx ultracite check", "lint:fix": "bunx ultracite fix", "test": "bun run test:unit", - "test:unit": "bun run generate:docs && bun run generate:sdk && bun test --timeout 15000 --isolate --parallel test/lib test/commands test/types test/isolated --coverage --coverage-reporter=lcov", + "test:unit": "bun run generate:docs && bun run generate:sdk && bun test --timeout 15000 --isolate --parallel test/lib test/commands test/types --coverage --coverage-reporter=lcov", "test:changed": "bun run generate:docs && bun run generate:sdk && bun test --timeout 15000 --isolate --changed", "test:e2e": "bun run generate:docs && bun run generate:sdk && bun test --timeout 15000 test/e2e", "test:init-eval": "bun test test/init-eval --timeout 600000 --concurrency 6", diff --git a/test/commands/auth/login.test.ts b/test/commands/auth/login.test.ts index 8541ec334..ae9208c66 100644 --- a/test/commands/auth/login.test.ts +++ b/test/commands/auth/login.test.ts @@ -1,16 +1,14 @@ /** * Login Command Tests * - * Unit tests for the --token and --force authentication paths in src/commands/auth/login.ts. - * Uses spyOn to mock api-client, db/auth, db/user, and interactive-login - * to cover all branches without real HTTP calls or database access. + * Unit tests for the --token, --force, and interactive TTY re-authentication + * paths in src/commands/auth/login.ts. Uses spyOn to mock api-client, db/auth, + * db/user, and interactive-login to cover all branches without real HTTP + * calls or database access. * - * Status messages go through consola (→ stderr). Logger message content is NOT - * asserted here because mock.module in login-reauth.test.ts can replace the - * logger module globally. Tests verify behavior via spy assertions instead. - * - * Tests that require isatty(0) to return true (interactive TTY prompt tests) - * live in test/isolated/login-reauth.test.ts to avoid mock.module pollution. + * The interactive TTY prompt tests use mock.module() at the top of this file + * to stub node:tty (so isatty(0) returns true) and the logger module (so + * `.prompt()` is controllable). */ import { @@ -22,7 +20,65 @@ import { spyOn, test, } from "bun:test"; -import { loginCommand } from "../../../src/commands/auth/login.js"; + +// Mock isatty to simulate interactive terminal for the re-auth prompt path. +// Bun's ESM wrapper for CJS built-ins exposes `default` + `ReadStream` + +// `WriteStream` — all must be present. +const mockIsatty = mock(() => false); +class FakeReadStream {} +class FakeWriteStream {} +const ttyExports = { + isatty: mockIsatty, + ReadStream: FakeReadStream, + WriteStream: FakeWriteStream, +}; +mock.module("node:tty", () => ({ + ...ttyExports, + default: ttyExports, +})); + +/** No-op placeholder for unused logger methods. */ +function noop() { + // intentional no-op +} + +// Mock the logger module to intercept the .prompt() call made by the +// module-scoped `log = logger.withTag("auth.login")` in login.ts. +const mockPrompt = mock((): Promise => Promise.resolve(true)); +const fakeLog: { + prompt: typeof mockPrompt; + info: ReturnType; + warn: ReturnType; + error: ReturnType; + debug: ReturnType; + success: ReturnType; + withTag: () => typeof fakeLog; +} = { + prompt: mockPrompt, + info: mock(noop), + warn: mock(noop), + error: mock(noop), + debug: mock(noop), + success: mock(noop), + withTag: () => fakeLog, +}; +mock.module("../../../src/lib/logger.js", () => ({ + logger: fakeLog, + setLogLevel: mock(noop), + attachSentryReporter: mock(noop), + LOG_LEVEL_NAMES: ["error", "warn", "log", "info", "debug", "trace"], + LOG_LEVEL_ENV_VAR: "SENTRY_LOG_LEVEL", + parseLogLevel: (name: string) => { + const levels = ["error", "warn", "log", "info", "debug", "trace"]; + const idx = levels.indexOf(name.toLowerCase().trim()); + return idx === -1 ? 3 : idx; + }, + getEnvLogLevel: () => null, +})); + +// Dynamic import: must run AFTER mock.module() so login.ts picks up fakeLog. +const { loginCommand } = await import("../../../src/commands/auth/login.js"); + // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as apiClient from "../../../src/lib/api-client.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking @@ -54,9 +110,9 @@ const SAMPLE_USER = { * * `getStdout()` returns rendered command output (human formatter → context.stdout). * - * Logger messages (early-exit diagnostics) are NOT captured here because - * mock.module in login-reauth.test.ts can replace the logger module globally. - * Tests for logger message content live in test/isolated/login-reauth.test.ts. + * Logger messages (early-exit diagnostics) go through the fakeLog mocked at + * the top of this file. Tests that care about specific prompt content inspect + * `mockPrompt.mock.calls` directly. */ function createContext() { const stdoutChunks: string[] = []; @@ -349,3 +405,177 @@ describe("loginCommand.func --token path", () => { expect(runInteractiveLoginSpy).toHaveBeenCalled(); }); }); + +/** + * Tests for the interactive TTY re-authentication prompt. + * + * Uses the module-level `mock.module()` on node:tty (so `isatty(0)` returns + * true) and the logger (so `.prompt()` is controllable). + */ +describe("login re-authentication interactive prompt", () => { + let isAuthenticatedSpy: ReturnType; + let isEnvTokenActiveSpy: ReturnType; + let clearAuthSpy: ReturnType; + let runInteractiveLoginSpy: ReturnType; + let getUserInfoSpy: ReturnType; + let listOrgsUncachedSpy: ReturnType; + let func: LoginFunc; + + function createPromptContext() { + return { + stdout: { write: mock(() => true) }, + stderr: { write: mock(() => true) }, + cwd: "/tmp", + }; + } + + beforeEach(async () => { + isAuthenticatedSpy = spyOn(dbAuth, "isAuthenticated"); + isEnvTokenActiveSpy = spyOn(dbAuth, "isEnvTokenActive"); + clearAuthSpy = spyOn(dbAuth, "clearAuth"); + runInteractiveLoginSpy = spyOn(interactiveLogin, "runInteractiveLogin"); + getUserInfoSpy = spyOn(dbUser, "getUserInfo"); + // Prevent warmOrgCache() fire-and-forget from hitting real fetch. + listOrgsUncachedSpy = spyOn(apiClient, "listOrganizationsUncached"); + listOrgsUncachedSpy.mockResolvedValue([]); + + // Defaults + isEnvTokenActiveSpy.mockReturnValue(false); + clearAuthSpy.mockResolvedValue(undefined); + runInteractiveLoginSpy.mockResolvedValue(true); + mockIsatty.mockReturnValue(true); + mockPrompt.mockClear(); + + func = (await loginCommand.loader()) as unknown as LoginFunc; + }); + + afterEach(() => { + isAuthenticatedSpy.mockRestore(); + isEnvTokenActiveSpy.mockRestore(); + clearAuthSpy.mockRestore(); + runInteractiveLoginSpy.mockRestore(); + getUserInfoSpy.mockRestore(); + listOrgsUncachedSpy.mockRestore(); + mockIsatty.mockReturnValue(false); + }); + + test("shows prompt with user identity when authenticated on TTY", async () => { + isAuthenticatedSpy.mockReturnValue(true); + getUserInfoSpy.mockReturnValue({ + userId: "42", + name: "Jane Doe", + email: "jane@example.com", + }); + mockPrompt.mockResolvedValue(true); + + const context = createPromptContext(); + await func.call(context, { force: false, timeout: 900 }); + + expect(mockPrompt).toHaveBeenCalledTimes(1); + const promptMessage = (mockPrompt.mock.calls[0] as unknown as string[])[0]; + expect(promptMessage).toContain("Jane Doe"); + expect(promptMessage).toContain("jane@example.com"); + expect(promptMessage).toContain("Re-authenticate?"); + }); + + test("shows 'current user' fallback when no cached user info", async () => { + isAuthenticatedSpy.mockReturnValue(true); + getUserInfoSpy.mockReturnValue(undefined); + mockPrompt.mockResolvedValue(true); + + const context = createPromptContext(); + await func.call(context, { force: false, timeout: 900 }); + + expect(mockPrompt).toHaveBeenCalledTimes(1); + const promptMessage = (mockPrompt.mock.calls[0] as unknown as string[])[0]; + expect(promptMessage).toContain("current user"); + }); + + test("confirm: clears auth and proceeds to login", async () => { + isAuthenticatedSpy.mockReturnValue(true); + getUserInfoSpy.mockReturnValue(undefined); + mockPrompt.mockResolvedValue(true); + + const context = createPromptContext(); + await func.call(context, { force: false, timeout: 900 }); + + expect(clearAuthSpy).toHaveBeenCalled(); + expect(runInteractiveLoginSpy).toHaveBeenCalled(); + }); + + test("decline: returns without re-auth", async () => { + isAuthenticatedSpy.mockReturnValue(true); + getUserInfoSpy.mockReturnValue(undefined); + mockPrompt.mockResolvedValue(false); + + const context = createPromptContext(); + await func.call(context, { force: false, timeout: 900 }); + + expect(mockPrompt).toHaveBeenCalled(); + expect(clearAuthSpy).not.toHaveBeenCalled(); + expect(runInteractiveLoginSpy).not.toHaveBeenCalled(); + }); + + test("cancel (Ctrl+C): returns without re-auth", async () => { + isAuthenticatedSpy.mockReturnValue(true); + getUserInfoSpy.mockReturnValue(undefined); + // consola returns Symbol(clack:cancel) on Ctrl+C — truthy but not `true`. + mockPrompt.mockResolvedValue(Symbol("clack:cancel")); + + const context = createPromptContext(); + await func.call(context, { force: false, timeout: 900 }); + + expect(mockPrompt).toHaveBeenCalled(); + expect(clearAuthSpy).not.toHaveBeenCalled(); + expect(runInteractiveLoginSpy).not.toHaveBeenCalled(); + }); + + test("--force skips prompt even on TTY", async () => { + isAuthenticatedSpy.mockReturnValue(true); + getUserInfoSpy.mockReturnValue(undefined); + + const context = createPromptContext(); + await func.call(context, { force: true, timeout: 900 }); + + expect(mockPrompt).not.toHaveBeenCalled(); + expect(clearAuthSpy).toHaveBeenCalled(); + expect(runInteractiveLoginSpy).toHaveBeenCalled(); + }); + + test("confirm + --token: clears auth and re-authenticates with token", async () => { + isAuthenticatedSpy.mockReturnValue(true); + getUserInfoSpy.mockReturnValue(undefined); + mockPrompt.mockResolvedValue(true); + + const setAuthTokenSpy = spyOn(dbAuth, "setAuthToken"); + setAuthTokenSpy.mockImplementation(noop); + const getUserRegionsSpy = spyOn(apiClient, "getUserRegions"); + getUserRegionsSpy.mockResolvedValue([]); + const getCurrentUserSpy = spyOn(apiClient, "getCurrentUser"); + getCurrentUserSpy.mockResolvedValue({ + id: "42", + name: "Jane", + username: "jane", + email: "jane@example.com", + }); + const setUserInfoSpy = spyOn(dbUser, "setUserInfo"); + setUserInfoSpy.mockReturnValue(undefined); + + const context = createPromptContext(); + try { + await func.call(context, { + token: "new-token", + force: false, + timeout: 900, + }); + + expect(clearAuthSpy).toHaveBeenCalled(); + expect(setAuthTokenSpy).toHaveBeenCalledWith("new-token"); + } finally { + setAuthTokenSpy.mockRestore(); + getUserRegionsSpy.mockRestore(); + getCurrentUserSpy.mockRestore(); + setUserInfoSpy.mockRestore(); + } + }); +}); diff --git a/test/commands/log/view.test.ts b/test/commands/log/view.test.ts index 66273350d..37f85f955 100644 --- a/test/commands/log/view.test.ts +++ b/test/commands/log/view.test.ts @@ -14,10 +14,67 @@ import { spyOn, test, } from "bun:test"; -import { - parsePositionalArgs, - viewCommand, -} from "../../../src/commands/log/view.js"; + +// Mock isatty to simulate an interactive terminal for the --web prompt path. +// Bun's ESM wrapper for CJS built-ins exposes a `default` re-export plus +// `ReadStream` / `WriteStream` — all must be present or Bun throws +// "Missing 'default' export in module 'node:tty'". +const mockIsatty = mock(() => false); +class FakeReadStream {} +class FakeWriteStream {} +const ttyExports = { + isatty: mockIsatty, + ReadStream: FakeReadStream, + WriteStream: FakeWriteStream, +}; +mock.module("node:tty", () => ({ + ...ttyExports, + default: ttyExports, +})); + +/** No-op placeholder for unused logger methods. */ +function noop() { + // intentional no-op +} + +// Mock the logger module to intercept the .prompt() call made by the +// module-scoped `log = logger.withTag("log-view")` in view.ts. +const mockPrompt = mock(() => Promise.resolve(true)); +const fakeLog: { + prompt: typeof mockPrompt; + warn: ReturnType; + info: ReturnType; + error: ReturnType; + debug: ReturnType; + withTag: () => typeof fakeLog; +} = { + prompt: mockPrompt, + warn: mock(noop), + info: mock(noop), + error: mock(noop), + debug: mock(noop), + withTag: () => fakeLog, +}; +mock.module("../../../src/lib/logger.js", () => ({ + logger: fakeLog, + setLogLevel: mock(noop), + attachSentryReporter: mock(noop), + LOG_LEVEL_NAMES: ["error", "warn", "log", "info", "debug", "trace"], + LOG_LEVEL_ENV_VAR: "SENTRY_LOG_LEVEL", + parseLogLevel: (name: string) => { + const levels = ["error", "warn", "log", "info", "debug", "trace"]; + const idx = levels.indexOf(name.toLowerCase().trim()); + return idx === -1 ? 3 : idx; + }, + getEnvLogLevel: () => null, +})); + +// Dynamic import: must load AFTER mock.module() registrations above so the +// `log = logger.withTag(...)` binding inside view.ts picks up fakeLog. +const { parsePositionalArgs, viewCommand } = await import( + "../../../src/commands/log/view.js" +); + import type { ProjectWithOrg } from "../../../src/lib/api-client.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as apiClient from "../../../src/lib/api-client.js"; @@ -534,3 +591,100 @@ describe("viewCommand.func", () => { } }); }); + +/** + * Tests for the --web interactive prompt path. + * + * Uses the module-level `mock.module()` on `node:tty` and the logger (set at + * the top of this file) to simulate an interactive terminal and control the + * prompt response. + */ +describe("log view --web interactive prompt", () => { + const PROMPT_ID1 = "aaaa1111bbbb2222cccc3333dddd4444"; + const PROMPT_ID2 = "1111222233334444555566667777aaaa"; + let openInBrowserSpy: ReturnType; + + function createPromptMockContext() { + const stdoutWrite = mock(() => true); + return { + context: { + stdout: { write: stdoutWrite }, + stderr: { write: mock(() => true) }, + cwd: "/tmp", + }, + stdoutWrite, + }; + } + + beforeEach(() => { + openInBrowserSpy = spyOn(browser, "openInBrowser"); + mockIsatty.mockReturnValue(true); + mockPrompt.mockClear(); + }); + + afterEach(() => { + openInBrowserSpy.mockRestore(); + mockIsatty.mockReturnValue(false); + }); + + test("prompts and opens all tabs when user confirms", async () => { + mockPrompt.mockResolvedValue(true); + openInBrowserSpy.mockResolvedValue(undefined); + + const { context } = createPromptMockContext(); + const func = await viewCommand.loader(); + await func.call( + context, + { json: false, web: true }, + "my-org/proj", + PROMPT_ID1, + PROMPT_ID2 + ); + + expect(mockPrompt).toHaveBeenCalled(); + expect(openInBrowserSpy).toHaveBeenCalledTimes(2); + const url1 = openInBrowserSpy.mock.calls[0][0] as string; + const url2 = openInBrowserSpy.mock.calls[1][0] as string; + expect(url1).toContain(PROMPT_ID1); + expect(url2).toContain(PROMPT_ID2); + }); + + test("prompts and aborts when user declines", async () => { + mockPrompt.mockResolvedValue(false); + openInBrowserSpy.mockResolvedValue(undefined); + + const { context } = createPromptMockContext(); + const func = await viewCommand.loader(); + await func.call( + context, + { json: false, web: true }, + "my-org/proj", + PROMPT_ID1, + PROMPT_ID2 + ); + + expect(mockPrompt).toHaveBeenCalled(); + expect(openInBrowserSpy).not.toHaveBeenCalled(); + }); + + test("aborts when user cancels prompt with Ctrl+C (truthy Symbol)", async () => { + // consola returns Symbol(clack:cancel) on Ctrl+C — truthy but not `true`. + // Cast needed because the mock is typed as boolean but consola actually + // returns a Symbol on cancel. + mockPrompt.mockResolvedValue(Symbol("clack:cancel") as unknown as boolean); + openInBrowserSpy.mockResolvedValue(undefined); + + const { context } = createPromptMockContext(); + const func = await viewCommand.loader(); + await func.call( + context, + { json: false, web: true }, + "my-org/proj", + PROMPT_ID1, + PROMPT_ID2 + ); + + expect(mockPrompt).toHaveBeenCalled(); + expect(openInBrowserSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/test/commands/project/delete.test.ts b/test/commands/project/delete.test.ts index 878886b8c..acff12d65 100644 --- a/test/commands/project/delete.test.ts +++ b/test/commands/project/delete.test.ts @@ -2,12 +2,10 @@ * Project Delete Command Tests * * Tests for the project delete command in src/commands/project/delete.ts. - * Uses spyOn to mock api-client and resolve-target to test - * the func() body without real HTTP calls or database access. - * - * Note: Interactive prompt (type-out confirmation) tests live in - * test/isolated/project-delete-confirm.test.ts because they require - * mock.module() to override node:tty and the logger module. + * Uses spyOn to mock api-client and resolve-target for the func() body + * without real HTTP calls or database access. The interactive type-out + * confirmation tests use mock.module() on node:tty and the logger module + * to control the prompt. */ import { @@ -19,7 +17,70 @@ import { spyOn, test, } from "bun:test"; -import { deleteCommand } from "../../../src/commands/project/delete.js"; + +// Mock isatty so deleteCommand's non-interactive guard passes. +// Bun's ESM wrapper for CJS built-ins exposes `default` + `ReadStream` + +// `WriteStream` — all must be present. +const mockIsatty = mock(() => false); +class FakeReadStream {} +class FakeWriteStream {} +const ttyExports = { + isatty: mockIsatty, + ReadStream: FakeReadStream, + WriteStream: FakeWriteStream, +}; +mock.module("node:tty", () => ({ + ...ttyExports, + default: ttyExports, +})); + +/** No-op placeholder for unused logger methods. */ +function noop() { + // intentional no-op +} + +// Mock the logger to intercept the .prompt() call made by the module-scoped +// `log = logger.withTag("project.delete")` inside delete.ts. +const mockPrompt = mock( + (): Promise => Promise.resolve("acme-corp/my-app") +); +const fakeLog: { + prompt: typeof mockPrompt; + info: ReturnType; + warn: ReturnType; + error: ReturnType; + debug: ReturnType; + success: ReturnType; + withTag: () => typeof fakeLog; +} = { + prompt: mockPrompt, + info: mock(noop), + warn: mock(noop), + error: mock(noop), + debug: mock(noop), + success: mock(noop), + withTag: () => fakeLog, +}; +mock.module("../../../src/lib/logger.js", () => ({ + logger: fakeLog, + setLogLevel: mock(noop), + attachSentryReporter: mock(noop), + LOG_LEVEL_NAMES: ["error", "warn", "log", "info", "debug", "trace"], + LOG_LEVEL_ENV_VAR: "SENTRY_LOG_LEVEL", + parseLogLevel: (name: string) => { + const levels = ["error", "warn", "log", "info", "debug", "trace"]; + const idx = levels.indexOf(name.toLowerCase().trim()); + return idx === -1 ? 3 : idx; + }, + getEnvLogLevel: () => null, +})); + +// Dynamic import: must run AFTER mock.module() so the module-scoped logger +// binding inside delete.ts picks up fakeLog. +const { deleteCommand } = await import( + "../../../src/commands/project/delete.js" +); + // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as apiClient from "../../../src/lib/api-client.js"; import { ApiError, ContextError } from "../../../src/lib/errors.js"; @@ -318,3 +379,133 @@ describe("project delete", () => { expect(deleteProjectSpy).not.toHaveBeenCalled(); }); }); + +/** + * Tests for the interactive type-out confirmation prompt. + * + * These rely on `mock.module()` at the top of this file to stub `node:tty` + * (so `isatty(0)` returns true) and the logger (so `.prompt()` returns a + * controllable value). + */ +describe("project delete — interactive confirmation", () => { + let getProjectSpy: ReturnType; + let deleteProjectSpy: ReturnType; + let resolveOrgProjectTargetSpy: ReturnType; + + function createPromptMockContext() { + const stdoutWrite = mock(() => true); + return { + context: { + stdout: { write: stdoutWrite }, + stderr: { write: mock(() => true) }, + cwd: "/tmp", + }, + stdoutWrite, + }; + } + + beforeEach(() => { + mockIsatty.mockReturnValue(true); + getProjectSpy = spyOn(apiClient, "getProject"); + deleteProjectSpy = spyOn(apiClient, "deleteProject"); + resolveOrgProjectTargetSpy = spyOn( + resolveTarget, + "resolveOrgProjectTarget" + ); + + getProjectSpy.mockResolvedValue(sampleProject); + deleteProjectSpy.mockResolvedValue(undefined); + resolveOrgProjectTargetSpy.mockResolvedValue({ + org: "acme-corp", + project: "my-app", + }); + + mockPrompt.mockClear(); + fakeLog.info.mockClear(); + }); + + afterEach(() => { + getProjectSpy.mockRestore(); + deleteProjectSpy.mockRestore(); + resolveOrgProjectTargetSpy.mockRestore(); + mockIsatty.mockReturnValue(false); + }); + + test("proceeds when user types exact org/project", async () => { + mockPrompt.mockResolvedValue("acme-corp/my-app"); + + const { context, stdoutWrite } = createPromptMockContext(); + const func = await deleteCommand.loader(); + await func.call( + context, + { yes: false, "dry-run": false }, + "acme-corp/my-app" + ); + + expect(deleteProjectSpy).toHaveBeenCalledWith("acme-corp", "my-app"); + const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); + expect(output).toContain("Deleted project"); + }); + + test("cancels when user types wrong value", async () => { + mockPrompt.mockResolvedValue("wrong-org/wrong-project"); + + const { context } = createPromptMockContext(); + const func = await deleteCommand.loader(); + await func.call( + context, + { yes: false, "dry-run": false }, + "acme-corp/my-app" + ); + + expect(deleteProjectSpy).not.toHaveBeenCalled(); + expect(fakeLog.info).toHaveBeenCalledWith("Cancelled."); + }); + + test("cancels when user presses Ctrl+C (Symbol)", async () => { + // consola returns Symbol(clack:cancel) on Ctrl+C — truthy but not a string. + mockPrompt.mockResolvedValue(Symbol("clack:cancel")); + + const { context } = createPromptMockContext(); + const func = await deleteCommand.loader(); + await func.call( + context, + { yes: false, "dry-run": false }, + "acme-corp/my-app" + ); + + expect(deleteProjectSpy).not.toHaveBeenCalled(); + expect(fakeLog.info).toHaveBeenCalledWith("Cancelled."); + }); + + test("cancels when user submits empty string", async () => { + mockPrompt.mockResolvedValue(""); + + const { context } = createPromptMockContext(); + const func = await deleteCommand.loader(); + await func.call( + context, + { yes: false, "dry-run": false }, + "acme-corp/my-app" + ); + + expect(deleteProjectSpy).not.toHaveBeenCalled(); + }); + + test("prompt message includes project name and expected input", async () => { + mockPrompt.mockResolvedValue("acme-corp/my-app"); + + const { context } = createPromptMockContext(); + const func = await deleteCommand.loader(); + await func.call( + context, + { yes: false, "dry-run": false }, + "acme-corp/my-app" + ); + + expect(mockPrompt).toHaveBeenCalledWith( + expect.stringContaining("acme-corp/my-app"), + expect.objectContaining({ type: "text" }) + ); + }); +}); diff --git a/test/isolated/brew-upgrade.test.ts b/test/isolated/brew-upgrade.test.ts deleted file mode 100644 index 811c18ecf..000000000 --- a/test/isolated/brew-upgrade.test.ts +++ /dev/null @@ -1,360 +0,0 @@ -/** - * Isolated tests for subprocess-based upgrade execution. - * - * Uses mock.module() to stub node:child_process/spawn, which leaks module - * state — kept isolated so it doesn't affect other test files. - * - * Covers: executeUpgrade (brew + package managers), runCommand, isInstalledWith, - * detectLegacyInstallationMethod, and detectInstallationMethod legacy path. - */ - -import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; -import { - execFile, - execFileSync, - execSync, - fork, - spawnSync, -} from "node:child_process"; -import { EventEmitter } from "node:events"; -import { UpgradeError } from "../../src/lib/errors.js"; -import { useTestConfigDir } from "../helpers.js"; - -// --------------------------------------------------------------------------- -// Fake ChildProcess helpers -// --------------------------------------------------------------------------- - -type FakeStdio = { - on: (event: string, cb: (chunk: Buffer) => void) => FakeStdio; - resume: () => void; -}; - -type FakeProc = EventEmitter & { - stdout: FakeStdio; - stderr: FakeStdio; -}; - -/** - * Build a minimal fake ChildProcess EventEmitter that emits 'close' - * with the given exit code after a microtask tick. - * - * @param exitCode - Exit code to emit on 'close' - * @param stdoutData - Optional data to emit on stdout before close - */ -function fakeProcess(exitCode: number, stdoutData = ""): FakeProc { - const emitter = new EventEmitter() as FakeProc; - - const listeners: Array<(chunk: Buffer) => void> = []; - emitter.stdout = { - on: (_event: string, cb: (chunk: Buffer) => void) => { - listeners.push(cb); - return emitter.stdout; - }, - // biome-ignore lint/suspicious/noEmptyBlockStatements: stub - resume: () => {}, - }; - emitter.stderr = { - on: (_event: string, _cb: (chunk: Buffer) => void) => emitter.stderr, - // biome-ignore lint/suspicious/noEmptyBlockStatements: stub - resume: () => {}, - }; - - queueMicrotask(() => { - if (stdoutData) { - for (const cb of listeners) { - cb(Buffer.from(stdoutData)); - } - } - emitter.emit("close", exitCode); - }); - - return emitter; -} - -/** - * Build a fake ChildProcess that emits an 'error' event instead of closing. - */ -function fakeErrorProcess(message: string): FakeProc { - const emitter = new EventEmitter() as FakeProc; - emitter.stdout = { - on: (_e: string, _cb: (chunk: Buffer) => void) => emitter.stdout, - // biome-ignore lint/suspicious/noEmptyBlockStatements: stub - resume: () => {}, - }; - emitter.stderr = { - on: (_e: string, _cb: (chunk: Buffer) => void) => emitter.stderr, - // biome-ignore lint/suspicious/noEmptyBlockStatements: stub - resume: () => {}, - }; - queueMicrotask(() => emitter.emit("error", new Error(message))); - return emitter; -} - -// --------------------------------------------------------------------------- -// mock.module — must be declared before any imports of the module under test. -// Bun hoists mock.module() calls so they run before top-level awaits. -// Pass through real non-spawn exports so transitive deps are unaffected. -// --------------------------------------------------------------------------- - -let spawnImpl: (cmd: string, args: string[], opts: object) => FakeProc = () => - fakeProcess(0); - -mock.module("node:child_process", () => ({ - execFile, - execFileSync, - execSync, - fork, - spawnSync, - spawn: (cmd: string, args: string[], opts: object) => - spawnImpl(cmd, args, opts), -})); - -// Import after mock is registered -const { detectInstallationMethod, executeUpgrade } = await import( - "../../src/lib/upgrade.js" -); - -const { clearInstallInfo } = await import("../../src/lib/db/install-info.js"); - -// --------------------------------------------------------------------------- -// executeUpgrade — brew -// --------------------------------------------------------------------------- - -describe("executeUpgrade (brew)", () => { - test("returns null on successful brew upgrade", async () => { - spawnImpl = () => fakeProcess(0); - expect(await executeUpgrade("brew", "1.0.0")).toBeNull(); - }); - - test("throws UpgradeError on non-zero brew exit", async () => { - spawnImpl = () => fakeProcess(1); - try { - await executeUpgrade("brew", "1.0.0"); - expect.unreachable("should have thrown"); - } catch (err) { - expect(err).toBeInstanceOf(UpgradeError); - expect((err as UpgradeError).reason).toBe("execution_failed"); - expect((err as UpgradeError).message).toContain("exit code 1"); - } - }); - - test("throws UpgradeError on brew spawn error", async () => { - spawnImpl = () => fakeErrorProcess("brew not found"); - try { - await executeUpgrade("brew", "1.0.0"); - expect.unreachable("should have thrown"); - } catch (err) { - expect(err).toBeInstanceOf(UpgradeError); - expect((err as UpgradeError).reason).toBe("execution_failed"); - expect((err as UpgradeError).message).toContain("brew not found"); - } - }); - - test("invokes brew with correct arguments", async () => { - let capturedCmd = ""; - let capturedArgs: string[] = []; - let capturedOpts: object = {}; - spawnImpl = (cmd, args, opts) => { - capturedCmd = cmd; - capturedArgs = args; - capturedOpts = opts; - return fakeProcess(0); - }; - await executeUpgrade("brew", "1.0.0"); - expect(capturedCmd).toBe("brew"); - expect(capturedArgs).toEqual(["upgrade", "getsentry/tools/sentry"]); - expect(capturedOpts).toHaveProperty("shell", process.platform === "win32"); - }); -}); - -// --------------------------------------------------------------------------- -// executeUpgrade — package managers (npm, pnpm, bun, yarn) -// --------------------------------------------------------------------------- - -describe("executeUpgrade (package managers)", () => { - test("npm: returns null on success", async () => { - spawnImpl = () => fakeProcess(0); - expect(await executeUpgrade("npm", "1.0.0")).toBeNull(); - }); - - test("npm: uses correct install arguments", async () => { - let capturedCmd = ""; - let capturedArgs: string[] = []; - let capturedOpts: object = {}; - spawnImpl = (cmd, args, opts) => { - capturedCmd = cmd; - capturedArgs = args; - capturedOpts = opts; - return fakeProcess(0); - }; - await executeUpgrade("npm", "1.2.3"); - expect(capturedCmd).toBe("npm"); - expect(capturedArgs).toEqual(["install", "-g", "sentry@1.2.3"]); - expect(capturedOpts).toHaveProperty("shell", process.platform === "win32"); - }); - - test("pnpm: uses correct install arguments", async () => { - let capturedArgs: string[] = []; - spawnImpl = (_cmd, args) => { - capturedArgs = args; - return fakeProcess(0); - }; - await executeUpgrade("pnpm", "1.2.3"); - expect(capturedArgs).toEqual(["install", "-g", "sentry@1.2.3"]); - }); - - test("bun: uses correct install arguments", async () => { - let capturedArgs: string[] = []; - spawnImpl = (_cmd, args) => { - capturedArgs = args; - return fakeProcess(0); - }; - await executeUpgrade("bun", "1.2.3"); - expect(capturedArgs).toEqual(["install", "-g", "sentry@1.2.3"]); - }); - - test("yarn: uses 'global add' arguments", async () => { - let capturedCmd = ""; - let capturedArgs: string[] = []; - let capturedOpts: object = {}; - spawnImpl = (cmd, args, opts) => { - capturedCmd = cmd; - capturedArgs = args; - capturedOpts = opts; - return fakeProcess(0); - }; - await executeUpgrade("yarn", "1.2.3"); - expect(capturedCmd).toBe("yarn"); - expect(capturedArgs).toEqual(["global", "add", "sentry@1.2.3"]); - expect(capturedOpts).toHaveProperty("shell", process.platform === "win32"); - }); - - test("npm: throws UpgradeError on non-zero exit", async () => { - spawnImpl = () => fakeProcess(1); - try { - await executeUpgrade("npm", "1.0.0"); - expect.unreachable("should have thrown"); - } catch (err) { - expect(err).toBeInstanceOf(UpgradeError); - expect((err as UpgradeError).reason).toBe("execution_failed"); - expect((err as UpgradeError).message).toContain("npm install failed"); - } - }); - - test("npm: throws UpgradeError on spawn error", async () => { - spawnImpl = () => fakeErrorProcess("npm not found"); - try { - await executeUpgrade("npm", "1.0.0"); - expect.unreachable("should have thrown"); - } catch (err) { - expect(err).toBeInstanceOf(UpgradeError); - expect((err as UpgradeError).reason).toBe("execution_failed"); - expect((err as UpgradeError).message).toContain("npm not found"); - } - }); -}); - -// --------------------------------------------------------------------------- -// executeUpgrade — unknown method (default switch case) -// --------------------------------------------------------------------------- - -describe("executeUpgrade (unknown method)", () => { - test("throws UpgradeError with unknown_method reason", async () => { - try { - await executeUpgrade("unknown" as never, "1.0.0"); - expect.unreachable("should have thrown"); - } catch (err) { - expect(err).toBeInstanceOf(UpgradeError); - expect((err as UpgradeError).reason).toBe("unknown_method"); - } - }); -}); - -// --------------------------------------------------------------------------- -// runCommand via isInstalledWith (indirect coverage of runCommand) -// --------------------------------------------------------------------------- - -describe("detectInstallationMethod — legacy pm detection via isInstalledWith", () => { - useTestConfigDir("test-detect-legacy-"); - - let originalExecPath: string; - - beforeEach(() => { - originalExecPath = process.execPath; - // Non-Homebrew, non-known-curl execPath so detection falls through to pm checks - Object.defineProperty(process, "execPath", { - value: "/usr/bin/sentry", - configurable: true, - }); - clearInstallInfo(); - }); - - afterEach(() => { - Object.defineProperty(process, "execPath", { - value: originalExecPath, - configurable: true, - }); - clearInstallInfo(); - }); - - test("detects npm when 'npm list -g sentry' output includes 'sentry@'", async () => { - let capturedOpts: object = {}; - spawnImpl = (_cmd, args, opts) => { - capturedOpts = opts; - return fakeProcess(0, args.includes("sentry") ? "sentry@1.0.0" : ""); - }; - const method = await detectInstallationMethod(); - expect(method).toBe("npm"); - // runCommand passes shell: true on Windows for .cmd compatibility - expect(capturedOpts).toHaveProperty("shell", process.platform === "win32"); - }); - - test("detects yarn when 'yarn global list' output includes 'sentry@'", async () => { - // npm is checked first — make npm/pnpm/bun return empty; only yarn matches - spawnImpl = (cmd) => { - if (cmd === "yarn") return fakeProcess(0, "sentry@1.0.0"); - return fakeProcess(0, ""); - }; - const method = await detectInstallationMethod(); - expect(method).toBe("yarn"); - }); - - test("returns 'unknown' when no package manager lists sentry", async () => { - spawnImpl = () => fakeProcess(0, ""); // all return empty stdout - const method = await detectInstallationMethod(); - expect(method).toBe("unknown"); - }); - - test("returns 'unknown' when all package manager spawns error", async () => { - spawnImpl = () => fakeErrorProcess("command not found"); - const method = await detectInstallationMethod(); - expect(method).toBe("unknown"); - }); - - test("auto-saves detected method when non-unknown", async () => { - spawnImpl = (_cmd, args) => - fakeProcess(0, args.includes("sentry") ? "sentry@2.0.0" : ""); - await detectInstallationMethod(); - // After detection, install info should be auto-saved with method=npm - const { getInstallInfo } = await import("../../src/lib/db/install-info.js"); - const stored = getInstallInfo(); - expect(stored?.method).toBe("npm"); - }); - - test("returns stored method on second call (auto-save fast path)", async () => { - // First call: npm detected and auto-saved - spawnImpl = (_cmd, args) => - fakeProcess(0, args.includes("sentry") ? "sentry@1.0.0" : ""); - await detectInstallationMethod(); - - // Second call: spawn should not be called again (stored info takes precedence) - let spawnCalled = false; - spawnImpl = () => { - spawnCalled = true; - return fakeProcess(0, "sentry@1.0.0"); - }; - const method = await detectInstallationMethod(); - expect(method).toBe("npm"); - expect(spawnCalled).toBe(false); - }); -}); diff --git a/test/isolated/dsn/project-root.test.ts b/test/isolated/dsn/project-root.test.ts deleted file mode 100644 index 0f12c7705..000000000 --- a/test/isolated/dsn/project-root.test.ts +++ /dev/null @@ -1,79 +0,0 @@ -/** - * Isolated tests for project-root stat() concurrency limiting. - * - * Uses mock.module() at file top-level (required pattern for module mocking - * in Bun — see test/isolated/dsn/fs-utils.test.ts for the established pattern). - * - * Note: pathExists() in project-root.ts uses a statically-bound import of - * node:fs/promises stat, so mock.module() cannot intercept it post-hoc. These - * tests instead verify the exported STAT_CONCURRENCY constant (which configures - * the pLimit instance) and confirm marker detection works end-to-end. The - * concurrency enforcement itself is delegated to pLimit, whose correctness is - * covered by its own test suite. - */ - -import { describe, expect, mock, test } from "bun:test"; -import { mkdirSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; - -// Mock Sentry to avoid telemetry side effects during isolated tests. -// startSpan must pass a no-op span object to the callback — withTracingSpan -// calls span.setStatus() on the result. -const noopSpan = { setStatus: mock(), setAttribute: mock() }; -mock.module("@sentry/node-core/light", () => ({ - startSpan: (_opts: unknown, fn: (span: unknown) => unknown) => fn(noopSpan), - captureException: mock(), -})); - -const { hasBuildSystemMarker, hasLanguageMarker, STAT_CONCURRENCY } = - await import("../../../src/lib/dsn/project-root.js"); - -function makeTempDir(prefix: string): string { - const dir = join( - tmpdir(), - `${prefix}-${Date.now()}-${Math.random().toString(36).slice(2)}` - ); - mkdirSync(dir, { recursive: true }); - return dir; -} - -describe("stat() concurrency limiting", () => { - test("STAT_CONCURRENCY is 32 — matches the pLimit budget in project-root.ts", () => { - // Guards against accidental changes to the budget value. The actual - // enforcement is delegated to pLimit; what we own is this constant. - expect(STAT_CONCURRENCY).toBe(32); - }); - - test("marker detection works correctly through the limiter (positive case)", async () => { - const testDir = makeTempDir("sentry-cli-marker"); - writeFileSync(join(testDir, "Makefile"), ""); - - const result = await hasBuildSystemMarker(testDir); - - expect(result).toBe(true); - }); - - test("marker detection returns false when no match (negative case)", async () => { - const testDir = makeTempDir("sentry-cli-no-marker"); - - const result = await hasBuildSystemMarker(testDir); - - expect(result).toBe(false); - }); - - test("multiple marker groups run correctly in parallel through shared limiter", async () => { - const testDir = makeTempDir("sentry-cli-parallel"); - writeFileSync(join(testDir, "package.json"), "{}"); - - // Fire both checks concurrently — both share statLimit. The language marker - // should be found; the build system marker should not. - const [hasBuild, hasLang] = await Promise.all([ - hasBuildSystemMarker(testDir), - hasLanguageMarker(testDir), - ]); - - expect(hasBuild).toBe(false); - expect(hasLang).toBe(true); - }); -}); diff --git a/test/isolated/log-view-prompt.test.ts b/test/isolated/log-view-prompt.test.ts deleted file mode 100644 index b92eaf0dd..000000000 --- a/test/isolated/log-view-prompt.test.ts +++ /dev/null @@ -1,177 +0,0 @@ -/** - * Isolated test for log view --web interactive prompt path. - * - * Uses mock.module() to override node:tty so isatty(0) returns true, - * and mocks the logger module to control the prompt response. - * - * Run with: bun test test/isolated - */ - -import { - afterEach, - beforeEach, - describe, - expect, - mock, - spyOn, - test, -} from "bun:test"; - -// Mock isatty to simulate interactive terminal. -// Bun's ESM wrapper for CJS built-ins exposes a `default` re-export plus -// `ReadStream` / `WriteStream` — all must be present or Bun throws -// "Missing 'default' export in module 'node:tty'". -const mockIsatty = mock(() => true); - -class FakeReadStream {} -class FakeWriteStream {} - -const ttyExports = { - isatty: mockIsatty, - ReadStream: FakeReadStream, - WriteStream: FakeWriteStream, -}; -mock.module("node:tty", () => ({ - ...ttyExports, - default: ttyExports, -})); - -// Mock prompt on the logger module — we need to intercept the .prompt() -// call made by the module-scoped `log = logger.withTag("log-view")` in view.ts. -// The approach: mock the entire logger so .withTag() returns a consola-like -// object whose .prompt() we control. -const mockPrompt = mock(() => Promise.resolve(true)); -const mockWarn = mock(() => { - // no-op -}); - -/** Fake scoped logger returned by withTag() */ -const fakeLog = { - prompt: mockPrompt, - warn: mockWarn, - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - info: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - error: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - debug: mock(() => {}), - withTag: () => fakeLog, -}; - -/** Fake root logger */ -const fakeLogger = { - ...fakeLog, - withTag: () => fakeLog, -}; - -mock.module("../../src/lib/logger.js", () => ({ - logger: fakeLogger, - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - setLogLevel: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - attachSentryReporter: mock(() => {}), - // These exports are required by command.ts (in the view.ts import chain) - LOG_LEVEL_NAMES: ["error", "warn", "log", "info", "debug", "trace"], - LOG_LEVEL_ENV_VAR: "SENTRY_LOG_LEVEL", - parseLogLevel: (name: string) => { - const levels = ["error", "warn", "log", "info", "debug", "trace"]; - const idx = levels.indexOf(name.toLowerCase().trim()); - return idx === -1 ? 3 : idx; - }, - getEnvLogLevel: () => null, -})); - -// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking -import * as browser from "../../src/lib/browser.js"; - -const { viewCommand } = await import("../../src/commands/log/view.js"); - -const ID1 = "aaaa1111bbbb2222cccc3333dddd4444"; -const ID2 = "1111222233334444555566667777aaaa"; - -function createMockContext() { - const stdoutWrite = mock(() => true); - return { - context: { - stdout: { write: stdoutWrite }, - stderr: { write: mock(() => true) }, - cwd: "/tmp", - }, - stdoutWrite, - }; -} - -describe("log view --web interactive prompt", () => { - let openInBrowserSpy: ReturnType; - - beforeEach(() => { - openInBrowserSpy = spyOn(browser, "openInBrowser"); - mockIsatty.mockReturnValue(true); - mockPrompt.mockClear(); - }); - - afterEach(() => { - openInBrowserSpy.mockRestore(); - }); - - test("prompts and opens all tabs when user confirms", async () => { - mockPrompt.mockResolvedValue(true); - openInBrowserSpy.mockResolvedValue(undefined); - - const { context } = createMockContext(); - const func = await viewCommand.loader(); - await func.call( - context, - { json: false, web: true }, - "my-org/proj", - ID1, - ID2 - ); - - expect(mockPrompt).toHaveBeenCalled(); - expect(openInBrowserSpy).toHaveBeenCalledTimes(2); - const url1 = openInBrowserSpy.mock.calls[0][0] as string; - const url2 = openInBrowserSpy.mock.calls[1][0] as string; - expect(url1).toContain(ID1); - expect(url2).toContain(ID2); - }); - - test("prompts and aborts when user declines", async () => { - mockPrompt.mockResolvedValue(false); - openInBrowserSpy.mockResolvedValue(undefined); - - const { context } = createMockContext(); - const func = await viewCommand.loader(); - await func.call( - context, - { json: false, web: true }, - "my-org/proj", - ID1, - ID2 - ); - - expect(mockPrompt).toHaveBeenCalled(); - expect(openInBrowserSpy).not.toHaveBeenCalled(); - }); - - test("aborts when user cancels prompt with Ctrl+C (truthy Symbol)", async () => { - // consola returns Symbol(clack:cancel) on Ctrl+C — truthy but not `true`. - // Cast needed because the mock is typed as boolean but consola actually - // returns a Symbol on cancel. - mockPrompt.mockResolvedValue(Symbol("clack:cancel") as unknown as boolean); - openInBrowserSpy.mockResolvedValue(undefined); - - const { context } = createMockContext(); - const func = await viewCommand.loader(); - await func.call( - context, - { json: false, web: true }, - "my-org/proj", - ID1, - ID2 - ); - - expect(mockPrompt).toHaveBeenCalled(); - expect(openInBrowserSpy).not.toHaveBeenCalled(); - }); -}); diff --git a/test/isolated/login-reauth.test.ts b/test/isolated/login-reauth.test.ts deleted file mode 100644 index d0ab026e3..000000000 --- a/test/isolated/login-reauth.test.ts +++ /dev/null @@ -1,272 +0,0 @@ -/** - * Isolated test for login re-authentication interactive prompt path. - * - * Uses mock.module() to override node:tty so isatty(0) returns true, - * and mocks the logger module to control the prompt response. - * - * Run with: bun test test/isolated/login-reauth.test.ts - */ - -import { - afterEach, - beforeEach, - describe, - expect, - mock, - spyOn, - test, -} from "bun:test"; - -// Mock isatty to simulate interactive terminal. -// Bun's ESM wrapper for CJS built-ins exposes a `default` re-export plus -// `ReadStream` / `WriteStream` — all must be present or Bun throws -// "Missing 'default' export in module 'node:tty'". -const mockIsatty = mock(() => true); - -class FakeReadStream {} -class FakeWriteStream {} - -const ttyExports = { - isatty: mockIsatty, - ReadStream: FakeReadStream, - WriteStream: FakeWriteStream, -}; -mock.module("node:tty", () => ({ - ...ttyExports, - default: ttyExports, -})); - -// Mock prompt on the logger module — we need to intercept the .prompt() -// call made by the module-scoped `log = logger.withTag("auth.login")` in login.ts. -// The approach: mock the entire logger so .withTag() returns a consola-like -// object whose .prompt() we control. -const mockPrompt = mock(() => Promise.resolve(true)); - -/** Fake scoped logger returned by withTag() */ -const fakeLog = { - prompt: mockPrompt, - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - info: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - warn: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - error: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - debug: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - success: mock(() => {}), - withTag: () => fakeLog, -}; - -/** Fake root logger */ -const fakeLogger = { - ...fakeLog, - withTag: () => fakeLog, -}; - -mock.module("../../src/lib/logger.js", () => ({ - logger: fakeLogger, - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - setLogLevel: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - attachSentryReporter: mock(() => {}), - // These exports are required by command.ts (in the login.ts import chain) - LOG_LEVEL_NAMES: ["error", "warn", "log", "info", "debug", "trace"], - LOG_LEVEL_ENV_VAR: "SENTRY_LOG_LEVEL", - parseLogLevel: (name: string) => { - const levels = ["error", "warn", "log", "info", "debug", "trace"]; - const idx = levels.indexOf(name.toLowerCase().trim()); - return idx === -1 ? 3 : idx; - }, - getEnvLogLevel: () => null, -})); - -// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking -import * as apiClient from "../../src/lib/api-client.js"; -// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking -import * as dbAuth from "../../src/lib/db/auth.js"; -// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking -import * as dbUser from "../../src/lib/db/user.js"; -// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking -import * as interactiveLogin from "../../src/lib/interactive-login.js"; - -const { loginCommand } = await import("../../src/commands/auth/login.js"); - -type LoginFlags = { - readonly token?: string; - readonly timeout: number; - readonly force: boolean; -}; - -type LoginFunc = (this: unknown, flags: LoginFlags) => Promise; - -function createMockContext() { - return { - stdout: { write: mock(() => true) }, - stderr: { write: mock(() => true) }, - cwd: "/tmp", - }; -} - -describe("login re-authentication interactive prompt", () => { - let isAuthenticatedSpy: ReturnType; - let isEnvTokenActiveSpy: ReturnType; - let clearAuthSpy: ReturnType; - let runInteractiveLoginSpy: ReturnType; - let getUserInfoSpy: ReturnType; - let listOrgsUncachedSpy: ReturnType; - let func: LoginFunc; - - beforeEach(async () => { - isAuthenticatedSpy = spyOn(dbAuth, "isAuthenticated"); - isEnvTokenActiveSpy = spyOn(dbAuth, "isEnvTokenActive"); - clearAuthSpy = spyOn(dbAuth, "clearAuth"); - runInteractiveLoginSpy = spyOn(interactiveLogin, "runInteractiveLogin"); - getUserInfoSpy = spyOn(dbUser, "getUserInfo"); - // Prevent warmOrgCache() fire-and-forget from hitting real fetch. - // After successful login, warmOrgCache() calls listOrganizationsUncached() - // which triggers API calls that leak as "unexpected fetch" warnings. - listOrgsUncachedSpy = spyOn(apiClient, "listOrganizationsUncached"); - listOrgsUncachedSpy.mockResolvedValue([]); - - // Defaults - isEnvTokenActiveSpy.mockReturnValue(false); - clearAuthSpy.mockResolvedValue(undefined); - runInteractiveLoginSpy.mockResolvedValue(true); - mockIsatty.mockReturnValue(true); - mockPrompt.mockClear(); - - func = (await loginCommand.loader()) as unknown as LoginFunc; - }); - - afterEach(() => { - isAuthenticatedSpy.mockRestore(); - isEnvTokenActiveSpy.mockRestore(); - clearAuthSpy.mockRestore(); - runInteractiveLoginSpy.mockRestore(); - getUserInfoSpy.mockRestore(); - listOrgsUncachedSpy.mockRestore(); - }); - - test("shows prompt with user identity when authenticated on TTY", async () => { - isAuthenticatedSpy.mockReturnValue(true); - getUserInfoSpy.mockReturnValue({ - userId: "42", - name: "Jane Doe", - email: "jane@example.com", - }); - mockPrompt.mockResolvedValue(true); - - const context = createMockContext(); - await func.call(context, { force: false, timeout: 900 }); - - expect(mockPrompt).toHaveBeenCalledTimes(1); - const promptMessage = (mockPrompt.mock.calls[0] as unknown as string[])[0]; - expect(promptMessage).toContain("Jane Doe"); - expect(promptMessage).toContain("jane@example.com"); - expect(promptMessage).toContain("Re-authenticate?"); - }); - - test("shows 'current user' fallback when no cached user info", async () => { - isAuthenticatedSpy.mockReturnValue(true); - getUserInfoSpy.mockReturnValue(undefined); - mockPrompt.mockResolvedValue(true); - - const context = createMockContext(); - await func.call(context, { force: false, timeout: 900 }); - - expect(mockPrompt).toHaveBeenCalledTimes(1); - const promptMessage = (mockPrompt.mock.calls[0] as unknown as string[])[0]; - expect(promptMessage).toContain("current user"); - }); - - test("confirm: clears auth and proceeds to login", async () => { - isAuthenticatedSpy.mockReturnValue(true); - getUserInfoSpy.mockReturnValue(undefined); - mockPrompt.mockResolvedValue(true); - - const context = createMockContext(); - await func.call(context, { force: false, timeout: 900 }); - - expect(clearAuthSpy).toHaveBeenCalled(); - expect(runInteractiveLoginSpy).toHaveBeenCalled(); - }); - - test("decline: returns without re-auth", async () => { - isAuthenticatedSpy.mockReturnValue(true); - getUserInfoSpy.mockReturnValue(undefined); - mockPrompt.mockResolvedValue(false); - - const context = createMockContext(); - await func.call(context, { force: false, timeout: 900 }); - - expect(mockPrompt).toHaveBeenCalled(); - expect(clearAuthSpy).not.toHaveBeenCalled(); - expect(runInteractiveLoginSpy).not.toHaveBeenCalled(); - }); - - test("cancel (Ctrl+C): returns without re-auth", async () => { - isAuthenticatedSpy.mockReturnValue(true); - getUserInfoSpy.mockReturnValue(undefined); - // consola returns Symbol(clack:cancel) on Ctrl+C — truthy but not `true`. - mockPrompt.mockResolvedValue(Symbol("clack:cancel") as unknown as boolean); - - const context = createMockContext(); - await func.call(context, { force: false, timeout: 900 }); - - expect(mockPrompt).toHaveBeenCalled(); - expect(clearAuthSpy).not.toHaveBeenCalled(); - expect(runInteractiveLoginSpy).not.toHaveBeenCalled(); - }); - - test("--force skips prompt even on TTY", async () => { - isAuthenticatedSpy.mockReturnValue(true); - getUserInfoSpy.mockReturnValue(undefined); - - const context = createMockContext(); - await func.call(context, { force: true, timeout: 900 }); - - expect(mockPrompt).not.toHaveBeenCalled(); - expect(clearAuthSpy).toHaveBeenCalled(); - expect(runInteractiveLoginSpy).toHaveBeenCalled(); - }); - - test("confirm + --token: clears auth and re-authenticates with token", async () => { - isAuthenticatedSpy.mockReturnValue(true); - getUserInfoSpy.mockReturnValue(undefined); - mockPrompt.mockResolvedValue(true); - - const setAuthTokenSpy = spyOn(dbAuth, "setAuthToken"); - setAuthTokenSpy.mockImplementation(() => { - // no-op — token storage mocked - }); - const getUserRegionsSpy = spyOn(apiClient, "getUserRegions"); - getUserRegionsSpy.mockResolvedValue([]); - const getCurrentUserSpy = spyOn(apiClient, "getCurrentUser"); - getCurrentUserSpy.mockResolvedValue({ - id: "42", - name: "Jane", - username: "jane", - email: "jane@example.com", - }); - const setUserInfoSpy = spyOn(dbUser, "setUserInfo"); - setUserInfoSpy.mockReturnValue(undefined); - - const context = createMockContext(); - try { - await func.call(context, { - token: "new-token", - force: false, - timeout: 900, - }); - - expect(clearAuthSpy).toHaveBeenCalled(); - expect(setAuthTokenSpy).toHaveBeenCalledWith("new-token"); - } finally { - setAuthTokenSpy.mockRestore(); - getUserRegionsSpy.mockRestore(); - getCurrentUserSpy.mockRestore(); - setUserInfoSpy.mockRestore(); - } - }); -}); diff --git a/test/isolated/project-delete-confirm.test.ts b/test/isolated/project-delete-confirm.test.ts deleted file mode 100644 index 4606245fb..000000000 --- a/test/isolated/project-delete-confirm.test.ts +++ /dev/null @@ -1,219 +0,0 @@ -/** - * Isolated test for project delete interactive confirmation path. - * - * Uses mock.module() to override node:tty so isatty(0) returns true, - * and mocks the logger module to control the prompt response. - * - * Run with: bun test test/isolated/project-delete-confirm.test.ts - */ - -import { - afterEach, - beforeEach, - describe, - expect, - mock, - spyOn, - test, -} from "bun:test"; - -// Mock isatty to simulate interactive terminal. -// Bun's ESM wrapper for CJS built-ins exposes a `default` re-export plus -// `ReadStream` / `WriteStream` — all must be present or Bun throws -// "Missing 'default' export in module 'node:tty'". -const mockIsatty = mock(() => true); - -class FakeReadStream {} -class FakeWriteStream {} - -const ttyExports = { - isatty: mockIsatty, - ReadStream: FakeReadStream, - WriteStream: FakeWriteStream, -}; -mock.module("node:tty", () => ({ - ...ttyExports, - default: ttyExports, -})); - -// Mock prompt on the logger module — we need to intercept the .prompt() -// call made by the module-scoped `log = logger.withTag("project.delete")`. -const mockPrompt = mock(() => Promise.resolve("acme-corp/my-app")); - -/** Fake scoped logger returned by withTag() */ -const fakeLog = { - prompt: mockPrompt, - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - info: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - warn: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - error: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - debug: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - success: mock(() => {}), - withTag: () => fakeLog, -}; - -/** Fake root logger */ -const fakeLogger = { - ...fakeLog, - withTag: () => fakeLog, -}; - -mock.module("../../src/lib/logger.js", () => ({ - logger: fakeLogger, - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - setLogLevel: mock(() => {}), - // biome-ignore lint/suspicious/noEmptyBlockStatements: no-op mock - attachSentryReporter: mock(() => {}), - // These exports are required by command.ts (in the delete.ts import chain) - LOG_LEVEL_NAMES: ["error", "warn", "log", "info", "debug", "trace"], - LOG_LEVEL_ENV_VAR: "SENTRY_LOG_LEVEL", - parseLogLevel: (name: string) => { - const levels = ["error", "warn", "log", "info", "debug", "trace"]; - const idx = levels.indexOf(name.toLowerCase().trim()); - return idx === -1 ? 3 : idx; - }, - getEnvLogLevel: () => null, -})); - -// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking -import * as apiClient from "../../src/lib/api-client.js"; -// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking -import * as resolveTarget from "../../src/lib/resolve-target.js"; -import type { SentryProject } from "../../src/types/index.js"; - -const { deleteCommand } = await import("../../src/commands/project/delete.js"); - -const sampleProject: SentryProject = { - id: "999", - slug: "my-app", - name: "My App", - platform: "python", - dateCreated: "2026-02-12T10:00:00Z", -}; - -function createMockContext() { - const stdoutWrite = mock(() => true); - return { - context: { - stdout: { write: stdoutWrite }, - stderr: { write: mock(() => true) }, - cwd: "/tmp", - }, - stdoutWrite, - }; -} - -describe("project delete — interactive confirmation", () => { - let getProjectSpy: ReturnType; - let deleteProjectSpy: ReturnType; - let resolveOrgProjectTargetSpy: ReturnType; - - beforeEach(() => { - getProjectSpy = spyOn(apiClient, "getProject"); - deleteProjectSpy = spyOn(apiClient, "deleteProject"); - resolveOrgProjectTargetSpy = spyOn( - resolveTarget, - "resolveOrgProjectTarget" - ); - - getProjectSpy.mockResolvedValue(sampleProject); - deleteProjectSpy.mockResolvedValue(undefined); - resolveOrgProjectTargetSpy.mockResolvedValue({ - org: "acme-corp", - project: "my-app", - }); - - mockPrompt.mockClear(); - fakeLog.info.mockClear(); - }); - - afterEach(() => { - getProjectSpy.mockRestore(); - deleteProjectSpy.mockRestore(); - resolveOrgProjectTargetSpy.mockRestore(); - }); - - test("proceeds when user types exact org/project", async () => { - mockPrompt.mockResolvedValue("acme-corp/my-app"); - - const { context, stdoutWrite } = createMockContext(); - const func = await deleteCommand.loader(); - await func.call( - context, - { yes: false, "dry-run": false }, - "acme-corp/my-app" - ); - - expect(deleteProjectSpy).toHaveBeenCalledWith("acme-corp", "my-app"); - const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); - expect(output).toContain("Deleted project"); - }); - - test("cancels when user types wrong value", async () => { - mockPrompt.mockResolvedValue("wrong-org/wrong-project"); - - const { context } = createMockContext(); - const func = await deleteCommand.loader(); - await func.call( - context, - { yes: false, "dry-run": false }, - "acme-corp/my-app" - ); - - expect(deleteProjectSpy).not.toHaveBeenCalled(); - expect(fakeLog.info).toHaveBeenCalledWith("Cancelled."); - }); - - test("cancels when user presses Ctrl+C (Symbol)", async () => { - // consola returns Symbol(clack:cancel) on Ctrl+C — truthy but not a string. - // Cast needed because the mock is typed as string but consola actually - // returns a Symbol on cancel. - mockPrompt.mockResolvedValue(Symbol("clack:cancel") as unknown as string); - - const { context } = createMockContext(); - const func = await deleteCommand.loader(); - await func.call( - context, - { yes: false, "dry-run": false }, - "acme-corp/my-app" - ); - - expect(deleteProjectSpy).not.toHaveBeenCalled(); - expect(fakeLog.info).toHaveBeenCalledWith("Cancelled."); - }); - - test("cancels when user submits empty string", async () => { - mockPrompt.mockResolvedValue(""); - - const { context } = createMockContext(); - const func = await deleteCommand.loader(); - await func.call( - context, - { yes: false, "dry-run": false }, - "acme-corp/my-app" - ); - - expect(deleteProjectSpy).not.toHaveBeenCalled(); - }); - - test("prompt message includes project name and expected input", async () => { - mockPrompt.mockResolvedValue("acme-corp/my-app"); - - const { context } = createMockContext(); - const func = await deleteCommand.loader(); - await func.call( - context, - { yes: false, "dry-run": false }, - "acme-corp/my-app" - ); - - expect(mockPrompt).toHaveBeenCalledWith( - expect.stringContaining("acme-corp/my-app"), - expect.objectContaining({ type: "text" }) - ); - }); -}); diff --git a/test/isolated/set-commits-auto-no-remote.test.ts b/test/isolated/set-commits-auto-no-remote.test.ts deleted file mode 100644 index 28f11fb2e..000000000 --- a/test/isolated/set-commits-auto-no-remote.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -/** - * Isolated test for setCommitsAuto when no git remote is available. - * - * Separate from set-commits-auto.test.ts because mock.module() - * sets getRepositoryName to return null (different mock). - */ - -import { describe, expect, mock, test } from "bun:test"; -import { useTestConfigDir } from "../helpers.js"; - -useTestConfigDir("set-commits-auto-no-remote-"); - -mock.module("../../src/lib/git.js", () => ({ - getRepositoryName: () => null, - getHeadCommit: () => "0000000000000000000000000000000000000000", - isInsideGitWorkTree: () => false, - isShallowRepository: () => false, - getCommitLog: () => [], - getUncommittedFiles: () => [], - parseRemoteUrl: (url: string) => url, -})); - -const { setCommitsAuto } = await import("../../src/lib/api/releases.js"); - -describe("setCommitsAuto (no git remote)", () => { - test("throws ValidationError when local git remote is not available", async () => { - await expect(setCommitsAuto("test-org", "1.0.0", "/tmp")).rejects.toThrow( - /Could not determine repository name/ - ); - }); -}); diff --git a/test/isolated/set-commits-auto.test.ts b/test/isolated/set-commits-auto.test.ts deleted file mode 100644 index 666572062..000000000 --- a/test/isolated/set-commits-auto.test.ts +++ /dev/null @@ -1,212 +0,0 @@ -/** - * Isolated tests for setCommitsAuto - * - * Uses mock.module() for git helpers, so must run in isolation - * to avoid polluting other test files' module state. - */ - -import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; -import { setAuthToken } from "../../src/lib/db/auth.js"; -import { setOrgRegion } from "../../src/lib/db/regions.js"; -import type { SentryRelease } from "../../src/types/index.js"; -import { mockFetch, useTestConfigDir } from "../helpers.js"; - -useTestConfigDir("set-commits-auto-"); - -mock.module("../../src/lib/git.js", () => ({ - getRepositoryName: () => "getsentry/cli", - getHeadCommit: () => "abc123def456789012345678901234567890abcd", - isInsideGitWorkTree: () => true, - isShallowRepository: () => false, - getCommitLog: () => [], - getUncommittedFiles: () => [], - parseRemoteUrl: (url: string) => url, -})); - -// Import after mock.module so the mocked git helpers are used -const { setCommitsAuto } = await import("../../src/lib/api/releases.js"); - -const SAMPLE_RELEASE: SentryRelease = { - id: 1, - version: "1.0.0", - shortVersion: "1.0.0", - status: "open", - dateCreated: "2025-01-01T00:00:00Z", - dateReleased: null, - firstEvent: null, - lastEvent: null, - ref: null, - url: null, - commitCount: 0, - deployCount: 0, - newGroups: 0, - authors: [], - projects: [ - { - id: 1, - slug: "test-project", - name: "Test Project", - platform: "javascript", - platforms: ["javascript"], - hasHealthData: false, - newGroups: 0, - }, - ], - data: {}, - versionInfo: null, -}; - -const SAMPLE_REPO = { - id: "1", - name: "getsentry/cli", - url: "https://github.com/getsentry/cli", - provider: { id: "integrations:github", name: "GitHub" }, - status: "active", -}; - -let originalFetch: typeof globalThis.fetch; - -beforeEach(async () => { - originalFetch = globalThis.fetch; - await setAuthToken("test-token"); - setOrgRegion("test-org", "https://us.sentry.io"); -}); - -afterEach(() => { - globalThis.fetch = originalFetch; -}); - -describe("setCommitsAuto", () => { - test("lists repos, discovers HEAD, fetches previous commit, and sends refs", async () => { - const withCommits = { ...SAMPLE_RELEASE, commitCount: 5 }; - const requests: { method: string; url: string }[] = []; - - globalThis.fetch = mockFetch(async (input, init) => { - const req = new Request(input!, init); - requests.push({ method: req.method, url: req.url }); - - // List org repositories (SDK uses /repos/ endpoint) - if (req.url.includes("/repos/")) { - expect(req.method).toBe("GET"); - return new Response(JSON.stringify([SAMPLE_REPO]), { - status: 200, - headers: { "Content-Type": "application/json" }, - }); - } - - // Previous release commit lookup - if (req.url.includes("/previous-with-commits/")) { - expect(req.method).toBe("GET"); - return new Response( - JSON.stringify({ - lastCommit: { id: "prev000000000000000000000000000000000000" }, - }), - { status: 200, headers: { "Content-Type": "application/json" } } - ); - } - - // PUT refs on the release - expect(req.method).toBe("PUT"); - expect(req.url).toContain("/releases/1.0.0/"); - const body = (await req.json()) as { - refs: Array<{ - repository: string; - commit: string; - previousCommit?: string; - }>; - }; - expect(body.refs).toEqual([ - { - repository: "getsentry/cli", - commit: "abc123def456789012345678901234567890abcd", - previousCommit: "prev000000000000000000000000000000000000", - }, - ]); - return new Response(JSON.stringify(withCommits), { - status: 200, - headers: { "Content-Type": "application/json" }, - }); - }); - - const release = await setCommitsAuto("test-org", "1.0.0", "/tmp"); - - expect(release.commitCount).toBe(5); - }); - - test("throws ApiError when org has no repositories", async () => { - globalThis.fetch = mockFetch( - async () => - new Response(JSON.stringify([]), { - status: 200, - headers: { "Content-Type": "application/json" }, - }) - ); - - await expect(setCommitsAuto("test-org", "1.0.0", "/tmp")).rejects.toThrow( - /No repository integrations/ - ); - }); - - test("throws ValidationError when no repo matches local remote", async () => { - const otherRepo = { ...SAMPLE_REPO, name: "getsentry/sentry" }; - globalThis.fetch = mockFetch( - async () => - new Response(JSON.stringify([otherRepo]), { - status: 200, - headers: { "Content-Type": "application/json" }, - }) - ); - - await expect(setCommitsAuto("test-org", "1.0.0", "/tmp")).rejects.toThrow( - /No Sentry repository matching/ - ); - }); - - test("paginates through multiple pages to find matching repo", async () => { - const withCommits = { ...SAMPLE_RELEASE, commitCount: 3 }; - const otherRepo = { ...SAMPLE_REPO, id: "2", name: "getsentry/sentry" }; - let repoRequestCount = 0; - - globalThis.fetch = mockFetch(async (input, init) => { - const req = new Request(input!, init); - - if (req.url.includes("/repos/")) { - repoRequestCount += 1; - if (repoRequestCount === 1) { - // First page: different repo, with a next cursor - return new Response(JSON.stringify([otherRepo]), { - status: 200, - headers: { - "Content-Type": "application/json", - Link: '; rel="next"; results="true"; cursor="page2:0:0"', - }, - }); - } - // Second page: the matching repo, no next cursor - return new Response(JSON.stringify([SAMPLE_REPO]), { - status: 200, - headers: { "Content-Type": "application/json" }, - }); - } - - // Previous release commit lookup (no previous release) - if (req.url.includes("/previous-with-commits/")) { - return new Response(JSON.stringify({}), { - status: 200, - headers: { "Content-Type": "application/json" }, - }); - } - - // PUT refs on the release - return new Response(JSON.stringify(withCommits), { - status: 200, - headers: { "Content-Type": "application/json" }, - }); - }); - - const release = await setCommitsAuto("test-org", "1.0.0", "/tmp"); - - expect(release.commitCount).toBe(3); - expect(repoRequestCount).toBe(2); - }); -}); diff --git a/test/lib/api/releases.test.ts b/test/lib/api/releases.test.ts index 4c199ced5..82903ee15 100644 --- a/test/lib/api/releases.test.ts +++ b/test/lib/api/releases.test.ts @@ -4,19 +4,43 @@ * Tests the real API function bodies by mocking globalThis.fetch. * This ensures the functions correctly call the SDK, pass parameters, * and transform responses. + * + * The `setCommitsAuto` tests additionally use `mock.module()` to stub the + * git helpers (`getRepositoryName`, etc.) because `setCommitsAuto` reads + * them at runtime. `getRepositoryName` is a controllable `mock()` so + * individual tests can change its return value (e.g. null for the + * "no git remote" path). */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; + +// Controllable git-helper mocks. `setCommitsAuto` calls these at runtime to +// build the `refs` array sent to Sentry. +const mockGetRepositoryName = mock((): string | null => "getsentry/cli"); +mock.module("../../../src/lib/git.js", () => ({ + getRepositoryName: mockGetRepositoryName, + getHeadCommit: () => "abc123def456789012345678901234567890abcd", + isInsideGitWorkTree: () => true, + isShallowRepository: () => false, + getCommitLog: () => [], + getUncommittedFiles: () => [], + parseRemoteUrl: (url: string) => url, +})); + +// Dynamic import: must run AFTER mock.module() so setCommitsAuto picks up +// the mocked git helpers. +const { createRelease, createReleaseDeploy, deleteRelease, getRelease, listReleaseDeploys, listReleasesPaginated, + setCommitsAuto, setCommitsLocal, updateRelease, -} from "../../../src/lib/api/releases.js"; +} = await import("../../../src/lib/api/releases.js"); + import { setAuthToken } from "../../../src/lib/db/auth.js"; import { setOrgRegion } from "../../../src/lib/db/regions.js"; import type { SentryDeploy, SentryRelease } from "../../../src/types/index.js"; @@ -233,8 +257,163 @@ describe("createReleaseDeploy", () => { // setCommitsAuto // ============================================================================= -// setCommitsAuto tests are in test/isolated/set-commits-auto.test.ts -// because they require mock.module() for git helpers. +describe("setCommitsAuto", () => { + const SAMPLE_REPO = { + id: "1", + name: "getsentry/cli", + url: "https://github.com/getsentry/cli", + provider: { id: "integrations:github", name: "GitHub" }, + status: "active", + }; + + beforeEach(() => { + // Reset the git-helper mock to the default (cli repo). Individual tests + // can override via mockGetRepositoryName.mockReturnValueOnce(null) or + // mockReturnValue("..."). + mockGetRepositoryName.mockReturnValue("getsentry/cli"); + }); + + test("lists repos, discovers HEAD, fetches previous commit, and sends refs", async () => { + const withCommits = { ...SAMPLE_RELEASE, commitCount: 5 }; + const requests: { method: string; url: string }[] = []; + + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input!, init); + requests.push({ method: req.method, url: req.url }); + + // List org repositories (SDK uses /repos/ endpoint) + if (req.url.includes("/repos/")) { + expect(req.method).toBe("GET"); + return new Response(JSON.stringify([SAMPLE_REPO]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + + // Previous release commit lookup + if (req.url.includes("/previous-with-commits/")) { + expect(req.method).toBe("GET"); + return new Response( + JSON.stringify({ + lastCommit: { id: "prev000000000000000000000000000000000000" }, + }), + { status: 200, headers: { "Content-Type": "application/json" } } + ); + } + + // PUT refs on the release + expect(req.method).toBe("PUT"); + expect(req.url).toContain("/releases/1.0.0/"); + const body = (await req.json()) as { + refs: Array<{ + repository: string; + commit: string; + previousCommit?: string; + }>; + }; + expect(body.refs).toEqual([ + { + repository: "getsentry/cli", + commit: "abc123def456789012345678901234567890abcd", + previousCommit: "prev000000000000000000000000000000000000", + }, + ]); + return new Response(JSON.stringify(withCommits), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + + const release = await setCommitsAuto("test-org", "1.0.0", "/tmp"); + + expect(release.commitCount).toBe(5); + }); + + test("throws ApiError when org has no repositories", async () => { + globalThis.fetch = mockFetch( + async () => + new Response(JSON.stringify([]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + ); + + await expect(setCommitsAuto("test-org", "1.0.0", "/tmp")).rejects.toThrow( + /No repository integrations/ + ); + }); + + test("throws ValidationError when no repo matches local remote", async () => { + const otherRepo = { ...SAMPLE_REPO, name: "getsentry/sentry" }; + globalThis.fetch = mockFetch( + async () => + new Response(JSON.stringify([otherRepo]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + ); + + await expect(setCommitsAuto("test-org", "1.0.0", "/tmp")).rejects.toThrow( + /No Sentry repository matching/ + ); + }); + + test("paginates through multiple pages to find matching repo", async () => { + const withCommits = { ...SAMPLE_RELEASE, commitCount: 3 }; + const otherRepo = { ...SAMPLE_REPO, id: "2", name: "getsentry/sentry" }; + let repoRequestCount = 0; + + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input!, init); + + if (req.url.includes("/repos/")) { + repoRequestCount += 1; + if (repoRequestCount === 1) { + // First page: different repo, with a next cursor + return new Response(JSON.stringify([otherRepo]), { + status: 200, + headers: { + "Content-Type": "application/json", + Link: '; rel="next"; results="true"; cursor="page2:0:0"', + }, + }); + } + // Second page: the matching repo, no next cursor + return new Response(JSON.stringify([SAMPLE_REPO]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + + // Previous release commit lookup (no previous release) + if (req.url.includes("/previous-with-commits/")) { + return new Response(JSON.stringify({}), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + + // PUT refs on the release + return new Response(JSON.stringify(withCommits), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + + const release = await setCommitsAuto("test-org", "1.0.0", "/tmp"); + + expect(release.commitCount).toBe(3); + expect(repoRequestCount).toBe(2); + }); + + test("throws ValidationError when local git remote is not available", async () => { + mockGetRepositoryName.mockReturnValue(null); + + await expect(setCommitsAuto("test-org", "1.0.0", "/tmp")).rejects.toThrow( + /Could not determine repository name/ + ); + }); +}); // ============================================================================= // setCommitsLocal diff --git a/test/isolated/delta-upgrade.test.ts b/test/lib/delta-upgrade.mocked.test.ts similarity index 96% rename from test/isolated/delta-upgrade.test.ts rename to test/lib/delta-upgrade.mocked.test.ts index eaabb2b43..610958ea1 100644 --- a/test/isolated/delta-upgrade.test.ts +++ b/test/lib/delta-upgrade.mocked.test.ts @@ -1,12 +1,14 @@ /** - * Integration tests for delta upgrade orchestration + * Integration tests for delta upgrade orchestration with a non-dev CLI_VERSION. * - * These tests use mock.module() to override CLI_VERSION from constants.js - * so that canAttemptDelta() passes its dev-build guard. They are isolated - * in a separate directory to run independently and avoid interfering with - * other test files. + * These tests use `mock.module()` to override `CLI_VERSION` from constants.js + * so that `canAttemptDelta()` passes its dev-build guard (the real `CLI_VERSION` + * is "0.0.0-dev" in test mode, which short-circuits the orchestrator). * - * Run with: bun test test/isolated/delta-upgrade.test.ts + * Kept as a sibling file to `delta-upgrade.test.ts` because its + * `mock.module()` would invert the assumptions of the dev-mode null-return + * tests in that file. Under `bun test --isolate` each file gets a fresh + * module graph, so the mocks here don't leak. */ import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; diff --git a/test/isolated/dsn/errors.test.ts b/test/lib/dsn/errors.test.ts similarity index 100% rename from test/isolated/dsn/errors.test.ts rename to test/lib/dsn/errors.test.ts diff --git a/test/isolated/dsn/fs-utils.test.ts b/test/lib/dsn/fs-utils.test.ts similarity index 100% rename from test/isolated/dsn/fs-utils.test.ts rename to test/lib/dsn/fs-utils.test.ts diff --git a/test/lib/dsn/project-root.test.ts b/test/lib/dsn/project-root.test.ts index c336a1437..13b0d178e 100644 --- a/test/lib/dsn/project-root.test.ts +++ b/test/lib/dsn/project-root.test.ts @@ -4,16 +4,31 @@ * Tests for finding project root by walking up from a starting directory. */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; import { mkdirSync, rmSync, writeFileSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; import { join } from "node:path"; + +// Mock Sentry to avoid telemetry side effects. startSpan must pass a no-op +// span object to the callback — withTracingSpan calls span.setStatus() on it, +// and production code may call span.setAttribute()/setAttributes() as well. +const noopSpan = { + setStatus: mock(), + setAttribute: mock(), + setAttributes: mock(), +}; +mock.module("@sentry/node-core/light", () => ({ + startSpan: (_opts: unknown, fn: (span: unknown) => unknown) => fn(noopSpan), + captureException: mock(), +})); + import { findProjectRoot, getStopBoundary, hasBuildSystemMarker, hasLanguageMarker, hasRepoRootMarker, + STAT_CONCURRENCY, } from "../../../src/lib/dsn/project-root.js"; // Test directory structure helper @@ -318,3 +333,62 @@ describe("project-root", () => { }); }); }); + +/** + * Tests for stat() concurrency limiting. + * + * Note: pathExists() in project-root.ts uses a statically-bound import of + * node:fs/promises stat, so mock.module() cannot intercept it post-hoc. These + * tests instead verify the exported STAT_CONCURRENCY constant (which configures + * the pLimit instance) and confirm marker detection works end-to-end. The + * concurrency enforcement itself is delegated to pLimit, whose correctness is + * covered by its own test suite. + */ +describe("stat() concurrency limiting", () => { + function makeTempConcurrencyDir(prefix: string): string { + const dir = join( + tmpdir(), + `${prefix}-${Date.now()}-${Math.random().toString(36).slice(2)}` + ); + mkdirSync(dir, { recursive: true }); + return dir; + } + + test("STAT_CONCURRENCY is 32 — matches the pLimit budget in project-root.ts", () => { + // Guards against accidental changes to the budget value. The actual + // enforcement is delegated to pLimit; what we own is this constant. + expect(STAT_CONCURRENCY).toBe(32); + }); + + test("marker detection works correctly through the limiter (positive case)", async () => { + const testDir = makeTempConcurrencyDir("sentry-cli-marker"); + writeFileSync(join(testDir, "Makefile"), ""); + + const result = await hasBuildSystemMarker(testDir); + + expect(result).toBe(true); + }); + + test("marker detection returns false when no match (negative case)", async () => { + const testDir = makeTempConcurrencyDir("sentry-cli-no-marker"); + + const result = await hasBuildSystemMarker(testDir); + + expect(result).toBe(false); + }); + + test("multiple marker groups run correctly in parallel through shared limiter", async () => { + const testDir = makeTempConcurrencyDir("sentry-cli-parallel"); + writeFileSync(join(testDir, "package.json"), "{}"); + + // Fire both checks concurrently — both share statLimit. The language marker + // should be found; the build system marker should not. + const [hasBuild, hasLang] = await Promise.all([ + hasBuildSystemMarker(testDir), + hasLanguageMarker(testDir), + ]); + + expect(hasBuild).toBe(false); + expect(hasLang).toBe(true); + }); +}); diff --git a/test/isolated/dsn/resolver.test.ts b/test/lib/dsn/resolver.test.ts similarity index 100% rename from test/isolated/dsn/resolver.test.ts rename to test/lib/dsn/resolver.test.ts diff --git a/test/isolated/resolve-target.test.ts b/test/lib/resolve-target.mocked.test.ts similarity index 97% rename from test/isolated/resolve-target.test.ts rename to test/lib/resolve-target.mocked.test.ts index 6cc2a1f2f..b0107d71f 100644 --- a/test/isolated/resolve-target.test.ts +++ b/test/lib/resolve-target.mocked.test.ts @@ -1,20 +1,23 @@ /** - * Integration tests for resolve-target utilities + * Unit tests for resolve-target utilities with mocked collaborators. * - * These tests use mock.module() which affects global module state. - * They are isolated in a separate directory to run independently - * and avoid interfering with other test files. + * These tests use `mock.module()` to stub out every dependency (DB caches, + * DSN detection, api-client). This gives tight control over return values + * at each decision point, at the cost of not exercising the real DB/HTTP + * paths — those are covered by `resolve-target.test.ts` (real DB + mocked + * fetch). * - * Run with: bun test test/isolated + * Keeping this as a separate file from `resolve-target.test.ts` so the two + * mocking strategies don't cross-contaminate: `bun test --isolate` gives + * each file a fresh module graph, so the mocks here don't leak into the + * real-DB integration tests. */ import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; -// IMPORTANT: Import the real formatMultipleProjectsFooter from its source file -// (not the barrel dsn/index.js). We pass this through the mock below so that -// if Bun leaks the mock.module() into other test files (which it does — see -// https://github.com/getsentry/cli/issues/258), the leaked version still has -// the real behavior instead of a simplified stub. +// Import the real formatMultipleProjectsFooter from its source file (not the +// barrel dsn/index.js), then pass it through the mock below so the mocked +// barrel still returns the real implementation for this export. import { formatMultipleProjectsFooter } from "../../src/lib/dsn/errors.js"; // ============================================================================ diff --git a/test/lib/sentry-client.test.ts b/test/lib/sentry-client.test.ts index b552e6205..bf109ecdc 100644 --- a/test/lib/sentry-client.test.ts +++ b/test/lib/sentry-client.test.ts @@ -35,50 +35,13 @@ function getAuthenticatedFetch(): typeof fetch { return getSdkConfig(REGION_URL).fetch as typeof fetch; } -/** - * Extract the URL string from any fetch input (Request, URL, or string). - */ -function urlOf(input: Parameters[0]): string { - if (typeof input === "string") { - return input; - } - if (input instanceof URL) { - return input.href; - } - return input.url; -} - -/** - * Wrap a mock handler so it only counts / reacts to requests whose URL - * contains `marker`. Any other `globalThis.fetch` call that happens to - * fire during the test (e.g. async work leaked from a previous file) - * is delegated to the caller-visible `originalFetch` (typically the - * preload.ts blocker) so foreign calls don't pollute this test's - * assertions. See CI run 24835339085 for the original flake. - */ -function scopedFetchMock( - marker: string, - handler: ( - input: Parameters[0], - init?: RequestInit - ) => Promise -): typeof fetch { - const captured = originalFetch; - return mockFetch(async (input, init) => { - if (!urlOf(input).includes(marker)) { - return await captured(input, init); - } - return await handler(input, init); - }); -} - describe("fetchWithRetry / buildAttemptFactory", () => { test("retries a POST with a string body without re-consuming the body", async () => { const marker = "__test_string_body__"; const seen: string[] = []; let callCount = 0; - globalThis.fetch = scopedFetchMock(marker, async (_input, init) => { + globalThis.fetch = mockFetch(async (_input, init) => { callCount += 1; const body = init?.body; if (typeof body === "string") { @@ -122,7 +85,7 @@ describe("fetchWithRetry / buildAttemptFactory", () => { let callCount = 0; const seen: string[] = []; - globalThis.fetch = scopedFetchMock(marker, async (input) => { + globalThis.fetch = mockFetch(async (input) => { callCount += 1; if (input instanceof Request) { seen.push(await input.clone().text()); @@ -160,7 +123,7 @@ describe("fetchWithRetry / buildAttemptFactory", () => { let callCount = 0; const seen: string[] = []; - globalThis.fetch = scopedFetchMock(marker, async (_input, init) => { + globalThis.fetch = mockFetch(async (_input, init) => { callCount += 1; const body = init?.body; if (body instanceof ArrayBuffer || ArrayBuffer.isView(body)) { @@ -214,7 +177,7 @@ describe("fetchWithRetry / buildAttemptFactory", () => { const contentTypes: (string | null)[] = []; const bodies: string[] = []; - globalThis.fetch = scopedFetchMock(marker, async (input, init) => { + globalThis.fetch = mockFetch(async (input, init) => { callCount += 1; const req = new Request(input as string, init); contentTypes.push(req.headers.get("content-type")); @@ -270,7 +233,7 @@ describe("fetchWithTimeout internal timeout classification", () => { try { let calls = 0; - globalThis.fetch = scopedFetchMock(marker, async (_input, init) => { + globalThis.fetch = mockFetch(async (_input, init) => { calls += 1; return await new Promise((_resolve, reject) => { const signal = init?.signal; @@ -314,7 +277,7 @@ describe("user abort passthrough", () => { test("external AbortSignal.abort() propagates the original AbortError", async () => { const marker = "__test_user_abort__"; let fetchCalled = false; - globalThis.fetch = scopedFetchMock(marker, async (_input, init) => { + globalThis.fetch = mockFetch(async (_input, init) => { fetchCalled = true; return await new Promise((_resolve, reject) => { const signal = init?.signal; diff --git a/test/lib/upgrade.test.ts b/test/lib/upgrade.test.ts index 00ca6ef00..ba8de0b78 100644 --- a/test/lib/upgrade.test.ts +++ b/test/lib/upgrade.test.ts @@ -2,13 +2,119 @@ * Upgrade Module Tests * * Tests for upgrade detection and logic. + * + * The `executeUpgrade` and `detectInstallationMethod` subprocess tests use + * `mock.module("node:child_process", ...)` at the top of this file to + * intercept `spawn()` calls via a swappable `spawnImpl`. Non-spawn exports + * pass through to the real `node:child_process`. */ -import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { + afterEach, + beforeEach, + describe, + expect, + mock, + spyOn, + test, +} from "bun:test"; +import { + execFile, + execFileSync, + execSync, + fork, + spawnSync, +} from "node:child_process"; +import { EventEmitter } from "node:events"; import { chmodSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { unlink } from "node:fs/promises"; import { homedir, platform } from "node:os"; import { join } from "node:path"; + +// --------------------------------------------------------------------------- +// Fake ChildProcess helpers used by the subprocess-based upgrade tests. +// --------------------------------------------------------------------------- + +type FakeStdio = { + on: (event: string, cb: (chunk: Buffer) => void) => FakeStdio; + resume: () => void; +}; + +type FakeProc = EventEmitter & { + stdout: FakeStdio; + stderr: FakeStdio; +}; + +/** No-op placeholder for stream methods we don't exercise. */ +function noopStream() { + // intentional no-op +} + +/** + * Build a minimal fake ChildProcess EventEmitter that emits 'close' + * with the given exit code after a microtask tick. + */ +function fakeProcess(exitCode: number, stdoutData = ""): FakeProc { + const emitter = new EventEmitter() as FakeProc; + + const listeners: Array<(chunk: Buffer) => void> = []; + emitter.stdout = { + on: (_event: string, cb: (chunk: Buffer) => void) => { + listeners.push(cb); + return emitter.stdout; + }, + resume: noopStream, + }; + emitter.stderr = { + on: (_event: string, _cb: (chunk: Buffer) => void) => emitter.stderr, + resume: noopStream, + }; + + queueMicrotask(() => { + if (stdoutData) { + for (const cb of listeners) { + cb(Buffer.from(stdoutData)); + } + } + emitter.emit("close", exitCode); + }); + + return emitter; +} + +/** Build a fake ChildProcess that emits an 'error' event instead of closing. */ +function fakeErrorProcess(message: string): FakeProc { + const emitter = new EventEmitter() as FakeProc; + emitter.stdout = { + on: (_e: string, _cb: (chunk: Buffer) => void) => emitter.stdout, + resume: noopStream, + }; + emitter.stderr = { + on: (_e: string, _cb: (chunk: Buffer) => void) => emitter.stderr, + resume: noopStream, + }; + queueMicrotask(() => emitter.emit("error", new Error(message))); + return emitter; +} + +// Swappable spawn implementation. Individual tests replace this before +// calling the code under test. Must be declared before mock.module() so the +// returned closure captures the live binding. +let spawnImpl: (cmd: string, args: string[], opts: object) => FakeProc = () => + fakeProcess(0); + +mock.module("node:child_process", () => ({ + execFile, + execFileSync, + execSync, + fork, + spawnSync, + spawn: (cmd: string, args: string[], opts: object) => + spawnImpl(cmd, args, opts), +})); + +// Dynamic imports: must run AFTER mock.module() so upgrade.ts picks up the +// mocked spawn. import { isEnoentSpawnError } from "../../src/commands/cli/upgrade.js"; import { acquireLock, @@ -22,7 +128,8 @@ import { setInstallInfo, } from "../../src/lib/db/install-info.js"; import { UpgradeError } from "../../src/lib/errors.js"; -import { + +const { detectInstallationMethod, detectPackageManagerFromPath, downloadBinaryToTemp, @@ -35,7 +142,8 @@ import { parseInstallationMethod, startCleanupOldBinary, versionExists, -} from "../../src/lib/upgrade.js"; +} = await import("../../src/lib/upgrade.js"); + import { TEST_TMP_DIR, useTestConfigDir } from "../helpers.js"; // Store original fetch for restoration @@ -1703,3 +1811,246 @@ describe("isEnoentSpawnError", () => { expect(isEnoentSpawnError(null)).toBe(false); }); }); + +// --------------------------------------------------------------------------- +// executeUpgrade — brew +// --------------------------------------------------------------------------- + +describe("executeUpgrade (brew)", () => { + test("returns null on successful brew upgrade", async () => { + spawnImpl = () => fakeProcess(0); + expect(await executeUpgrade("brew", "1.0.0")).toBeNull(); + }); + + test("throws UpgradeError on non-zero brew exit", async () => { + spawnImpl = () => fakeProcess(1); + try { + await executeUpgrade("brew", "1.0.0"); + expect.unreachable("should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(UpgradeError); + expect((err as UpgradeError).reason).toBe("execution_failed"); + expect((err as UpgradeError).message).toContain("exit code 1"); + } + }); + + test("throws UpgradeError on brew spawn error", async () => { + spawnImpl = () => fakeErrorProcess("brew not found"); + try { + await executeUpgrade("brew", "1.0.0"); + expect.unreachable("should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(UpgradeError); + expect((err as UpgradeError).reason).toBe("execution_failed"); + expect((err as UpgradeError).message).toContain("brew not found"); + } + }); + + test("invokes brew with correct arguments", async () => { + let capturedCmd = ""; + let capturedArgs: string[] = []; + let capturedOpts: object = {}; + spawnImpl = (cmd, args, opts) => { + capturedCmd = cmd; + capturedArgs = args; + capturedOpts = opts; + return fakeProcess(0); + }; + await executeUpgrade("brew", "1.0.0"); + expect(capturedCmd).toBe("brew"); + expect(capturedArgs).toEqual(["upgrade", "getsentry/tools/sentry"]); + expect(capturedOpts).toHaveProperty("shell", process.platform === "win32"); + }); +}); + +// --------------------------------------------------------------------------- +// executeUpgrade — package managers (npm, pnpm, bun, yarn) +// --------------------------------------------------------------------------- + +describe("executeUpgrade (package managers)", () => { + test("npm: returns null on success", async () => { + spawnImpl = () => fakeProcess(0); + expect(await executeUpgrade("npm", "1.0.0")).toBeNull(); + }); + + test("npm: uses correct install arguments", async () => { + let capturedCmd = ""; + let capturedArgs: string[] = []; + let capturedOpts: object = {}; + spawnImpl = (cmd, args, opts) => { + capturedCmd = cmd; + capturedArgs = args; + capturedOpts = opts; + return fakeProcess(0); + }; + await executeUpgrade("npm", "1.2.3"); + expect(capturedCmd).toBe("npm"); + expect(capturedArgs).toEqual(["install", "-g", "sentry@1.2.3"]); + expect(capturedOpts).toHaveProperty("shell", process.platform === "win32"); + }); + + test("pnpm: uses correct install arguments", async () => { + let capturedArgs: string[] = []; + spawnImpl = (_cmd, args) => { + capturedArgs = args; + return fakeProcess(0); + }; + await executeUpgrade("pnpm", "1.2.3"); + expect(capturedArgs).toEqual(["install", "-g", "sentry@1.2.3"]); + }); + + test("bun: uses correct install arguments", async () => { + let capturedArgs: string[] = []; + spawnImpl = (_cmd, args) => { + capturedArgs = args; + return fakeProcess(0); + }; + await executeUpgrade("bun", "1.2.3"); + expect(capturedArgs).toEqual(["install", "-g", "sentry@1.2.3"]); + }); + + test("yarn: uses 'global add' arguments", async () => { + let capturedCmd = ""; + let capturedArgs: string[] = []; + let capturedOpts: object = {}; + spawnImpl = (cmd, args, opts) => { + capturedCmd = cmd; + capturedArgs = args; + capturedOpts = opts; + return fakeProcess(0); + }; + await executeUpgrade("yarn", "1.2.3"); + expect(capturedCmd).toBe("yarn"); + expect(capturedArgs).toEqual(["global", "add", "sentry@1.2.3"]); + expect(capturedOpts).toHaveProperty("shell", process.platform === "win32"); + }); + + test("npm: throws UpgradeError on non-zero exit", async () => { + spawnImpl = () => fakeProcess(1); + try { + await executeUpgrade("npm", "1.0.0"); + expect.unreachable("should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(UpgradeError); + expect((err as UpgradeError).reason).toBe("execution_failed"); + expect((err as UpgradeError).message).toContain("npm install failed"); + } + }); + + test("npm: throws UpgradeError on spawn error", async () => { + spawnImpl = () => fakeErrorProcess("npm not found"); + try { + await executeUpgrade("npm", "1.0.0"); + expect.unreachable("should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(UpgradeError); + expect((err as UpgradeError).reason).toBe("execution_failed"); + expect((err as UpgradeError).message).toContain("npm not found"); + } + }); +}); + +// --------------------------------------------------------------------------- +// executeUpgrade — unknown method (default switch case) +// --------------------------------------------------------------------------- + +describe("executeUpgrade (unknown method)", () => { + test("throws UpgradeError with unknown_method reason", async () => { + try { + await executeUpgrade("unknown" as never, "1.0.0"); + expect.unreachable("should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(UpgradeError); + expect((err as UpgradeError).reason).toBe("unknown_method"); + } + }); +}); + +// --------------------------------------------------------------------------- +// runCommand via isInstalledWith (indirect coverage of runCommand) +// --------------------------------------------------------------------------- + +describe("detectInstallationMethod — legacy pm detection via isInstalledWith", () => { + useTestConfigDir("test-detect-legacy-"); + + let originalExecPath: string; + + beforeEach(() => { + originalExecPath = process.execPath; + // Non-Homebrew, non-known-curl execPath so detection falls through to pm checks + Object.defineProperty(process, "execPath", { + value: "/usr/bin/sentry", + configurable: true, + }); + clearInstallInfo(); + }); + + afterEach(() => { + Object.defineProperty(process, "execPath", { + value: originalExecPath, + configurable: true, + }); + clearInstallInfo(); + }); + + test("detects npm when 'npm list -g sentry' output includes 'sentry@'", async () => { + let capturedOpts: object = {}; + spawnImpl = (_cmd, args, opts) => { + capturedOpts = opts; + return fakeProcess(0, args.includes("sentry") ? "sentry@1.0.0" : ""); + }; + const method = await detectInstallationMethod(); + expect(method).toBe("npm"); + // runCommand passes shell: true on Windows for .cmd compatibility + expect(capturedOpts).toHaveProperty("shell", process.platform === "win32"); + }); + + test("detects yarn when 'yarn global list' output includes 'sentry@'", async () => { + // npm is checked first — make npm/pnpm/bun return empty; only yarn matches + spawnImpl = (cmd) => { + if (cmd === "yarn") return fakeProcess(0, "sentry@1.0.0"); + return fakeProcess(0, ""); + }; + const method = await detectInstallationMethod(); + expect(method).toBe("yarn"); + }); + + test("returns 'unknown' when no package manager lists sentry", async () => { + spawnImpl = () => fakeProcess(0, ""); // all return empty stdout + const method = await detectInstallationMethod(); + expect(method).toBe("unknown"); + }); + + test("returns 'unknown' when all package manager spawns error", async () => { + spawnImpl = () => fakeErrorProcess("command not found"); + const method = await detectInstallationMethod(); + expect(method).toBe("unknown"); + }); + + test("auto-saves detected method when non-unknown", async () => { + spawnImpl = (_cmd, args) => + fakeProcess(0, args.includes("sentry") ? "sentry@2.0.0" : ""); + await detectInstallationMethod(); + // After detection, install info should be auto-saved with method=npm + const { getInstallInfo } = await import("../../src/lib/db/install-info.js"); + const stored = getInstallInfo(); + expect(stored?.method).toBe("npm"); + }); + + test("returns stored method on second call (auto-save fast path)", async () => { + // First call: npm detected and auto-saved + spawnImpl = (_cmd, args) => + fakeProcess(0, args.includes("sentry") ? "sentry@1.0.0" : ""); + await detectInstallationMethod(); + + // Second call: spawn should not be called again (stored info takes precedence) + let spawnCalled = false; + spawnImpl = () => { + spawnCalled = true; + return fakeProcess(0, "sentry@1.0.0"); + }; + const method = await detectInstallationMethod(); + expect(method).toBe("npm"); + expect(spawnCalled).toBe(false); + }); +});