diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 72c7166..0d8d254 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -58,3 +58,28 @@ jobs: - name: Run type check run: pnpm check-types + + test: + name: Test Suite + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + version: 10.15.0 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: 22 + cache: pnpm + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Run tests + run: pnpm test diff --git a/apps/dashboard/src/lib/github-cache.test.ts b/apps/dashboard/src/lib/github-cache.test.ts index b1148b7..61f553c 100644 --- a/apps/dashboard/src/lib/github-cache.test.ts +++ b/apps/dashboard/src/lib/github-cache.test.ts @@ -8,6 +8,12 @@ import { getOrRevalidateGitHubResource, } from "./github-cache"; +vi.mock("@tanstack/react-start/server", () => ({ + getRequest: () => { + throw new Error("Not in request context"); + }, +})); + function createMemoryStore( initialEntries: GitHubCacheStoreEntry[] = [], ): GitHubCacheStore { diff --git a/apps/dashboard/src/lib/github.server.test.ts b/apps/dashboard/src/lib/github.server.test.ts index ded0f50..eedff96 100644 --- a/apps/dashboard/src/lib/github.server.test.ts +++ b/apps/dashboard/src/lib/github.server.test.ts @@ -30,10 +30,13 @@ const octokitConstructor = vi.fn((options: Record) => { log: instance.log, }; }); +const octokitDefaults = vi.fn(() => octokitConstructor); +Object.assign(octokitConstructor, { defaults: octokitDefaults }); const appConstructor = vi.fn(); const getGitHubAccessTokenByUserId = vi.fn(async () => "github-token"); const getGitHubAppId = vi.fn(() => "12345"); const getGitHubAppPrivateKey = vi.fn(() => "private-key"); +const configureGitHubRequestPolicies = vi.fn(); vi.mock("octokit", () => ({ App: appConstructor, @@ -46,6 +49,10 @@ vi.mock("./github-app.server", () => ({ getGitHubAppPrivateKey, })); +vi.mock("./github-request-policy", () => ({ + configureGitHubRequestPolicies, +})); + beforeEach(() => { vi.resetModules(); octokitInstances.length = 0; @@ -54,10 +61,12 @@ beforeEach(() => { getGitHubAccessTokenByUserId.mockClear(); getGitHubAppId.mockClear(); getGitHubAppPrivateKey.mockClear(); + octokitDefaults.mockClear(); + configureGitHubRequestPolicies.mockClear(); }); describe("getGitHubClient", () => { - it("configures Octokit throttling, bounded retries, and request timeouts", async () => { + it("creates an Octokit client with user token and request policies", async () => { const { getGitHubClient } = await import("./github.server"); await getGitHubClient("user-123"); @@ -70,100 +79,18 @@ describe("getGitHubClient", () => { auth: string; userAgent: string; retry: { enabled: boolean }; - throttle: { - enabled: boolean; - id: string; - fallbackSecondaryRateRetryAfter: number; - onRateLimit: ( - retryAfter: number, - options: { method?: string; url: string }, - octokit: { - log: { - warn: (message: string) => void; - info: (message: string) => void; - }; - }, - retryCount: number, - ) => boolean; - onSecondaryRateLimit: ( - retryAfter: number, - options: { method?: string; url: string }, - octokit: { - log: { - warn: (message: string) => void; - info: (message: string) => void; - }; - }, - retryCount: number, - ) => boolean; - }; + throttle: { enabled: boolean }; }; expect(options.auth).toBe("github-token"); expect(options.userAgent).toBe("diffkit-dashboard"); - expect(options.retry).toEqual({ enabled: true }); - expect(options.throttle.enabled).toBe(true); - expect(options.throttle.id).toBe("github-user:user-123"); - expect(options.throttle.fallbackSecondaryRateRetryAfter).toBe(60); - - expect(instance.hookBefore).toHaveBeenCalledTimes(1); - expect(instance.hookAfter).toHaveBeenCalledTimes(1); - expect(instance.hookError).toHaveBeenCalledTimes(1); - const [hookEvent, hookHandler] = instance.hookBefore.mock.calls[0] as [ - string, - (options: { - method?: string; - request?: { retries?: number; signal?: AbortSignal }; - }) => void, - ]; - expect(hookEvent).toBe("request"); - - const getOptions = { method: "GET" } as { - method?: string; - request?: { retries?: number; signal?: AbortSignal }; - }; - hookHandler(getOptions); - expect(getOptions.request?.retries).toBe(1); - expect(getOptions.request?.signal).toBeInstanceOf(AbortSignal); - - const postOptions = { method: "POST" } as { - method?: string; - request?: { retries?: number; signal?: AbortSignal }; - }; - hookHandler(postOptions); - expect(postOptions.request?.retries).toBe(0); - expect(postOptions.request?.signal).toBeInstanceOf(AbortSignal); + expect(options.retry).toEqual({ enabled: false }); + expect(options.throttle).toEqual({ enabled: false }); - expect( - options.throttle.onRateLimit( - 30, - { method: "GET", url: "/repos" }, - { - log: instance.log, - }, - 0, - ), - ).toBe(false); - expect( - options.throttle.onRateLimit( - 30, - { method: "POST", url: "/repos" }, - { - log: instance.log, - }, - 0, - ), - ).toBe(false); - expect( - options.throttle.onSecondaryRateLimit( - 30, - { method: "GET", url: "/search/issues" }, - { log: instance.log }, - 1, - ), - ).toBe(false); - expect(instance.log.warn).toHaveBeenCalled(); - expect(instance.log.info).not.toHaveBeenCalled(); + expect(configureGitHubRequestPolicies).toHaveBeenCalledTimes(1); + expect(configureGitHubRequestPolicies.mock.calls[0][1]).toEqual({ + tokenLabel: "oauth:user:user-123", + }); }); it("creates GitHub App installation clients from app credentials", async () => { @@ -206,24 +133,23 @@ describe("getGitHubClient", () => { auth: string; userAgent: string; retry: { enabled: boolean }; - throttle: { - enabled: boolean; - id: string; - fallbackSecondaryRateRetryAfter: number; - }; + throttle: { enabled: boolean }; }; expect(options.auth).toBe("installation-token"); expect(options.userAgent).toBe("diffkit-dashboard"); - expect(options.retry).toEqual({ enabled: true }); - expect(options.throttle.enabled).toBe(true); - expect(options.throttle.id).toBe("github-installation:987"); - expect(options.throttle.fallbackSecondaryRateRetryAfter).toBe(60); - expect(appOctokit.hook.before).toHaveBeenCalledTimes(1); - expect(appOctokit.hook.after).toHaveBeenCalledTimes(1); - expect(appOctokit.hook.error).toHaveBeenCalledTimes(1); - expect(installationInstance.hookBefore).toHaveBeenCalledTimes(1); - expect(installationInstance.hookAfter).toHaveBeenCalledTimes(1); - expect(installationInstance.hookError).toHaveBeenCalledTimes(1); + expect(options.retry).toEqual({ enabled: false }); + expect(options.throttle).toEqual({ enabled: false }); + expect(octokitDefaults).toHaveBeenCalledWith({ + retry: { enabled: false }, + throttle: { enabled: false }, + }); + expect(configureGitHubRequestPolicies).toHaveBeenCalledTimes(2); + expect(configureGitHubRequestPolicies.mock.calls[0][1]).toEqual({ + tokenLabel: "app-auth:installation:987", + }); + expect(configureGitHubRequestPolicies.mock.calls[1][1]).toEqual({ + tokenLabel: "installation:987", + }); }); it("reuses fresh installation tokens and remints after invalidation", async () => { diff --git a/package.json b/package.json index 0c4419a..606f70e 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "lint": "turbo run lint", "check": "turbo run check", "check-types": "turbo run check-types", + "test": "turbo run test", "format": "turbo run format", "prepare": "husky" }, diff --git a/turbo.json b/turbo.json index 43fa1af..905e080 100644 --- a/turbo.json +++ b/turbo.json @@ -19,6 +19,9 @@ "check-types": { "dependsOn": ["^check-types"] }, + "test": { + "dependsOn": ["^build"] + }, "format": {} } }