diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 56df09a..df7e7b4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,8 +60,20 @@ jobs: - name: Install dependencies run: npm ci + - name: Cache Playwright browsers + uses: actions/cache@v4 + with: + path: ~/.cache/ms-playwright + key: playwright-${{ runner.os }}-${{ hashFiles('package-lock.json') }} + + - name: Install Playwright Chromium + run: npx playwright install --with-deps chromium + - name: Static Analysis (Typecheck) run: npm run typecheck - name: Automated Tests run: npm test + + - name: Browser Tests + run: npm run test:browser diff --git a/.gitignore b/.gitignore index e6c2e3a..b172741 100644 --- a/.gitignore +++ b/.gitignore @@ -143,3 +143,7 @@ vite.config.ts.timestamp-* .wrangler .agent + +# Vitest browser mode failure screenshots and attachments +test/browser/__screenshots__/ +.vitest-attachments/ diff --git a/package-lock.json b/package-lock.json index 2cffbcc..fdf8e23 100644 --- a/package-lock.json +++ b/package-lock.json @@ -36,7 +36,6 @@ "zod": "^4.3.6" }, "devDependencies": { - "@cloudflare/vitest-pool-workers": "^0.16.20", "@tailwindcss/vite": "^4.2.2", "@testing-library/dom": "^10.4.1", "@testing-library/jest-dom": "^6.9.1", @@ -201,35 +200,6 @@ } } }, - "node_modules/@cloudflare/vitest-pool-workers": { - "version": "0.16.20", - "resolved": "https://registry.npmjs.org/@cloudflare/vitest-pool-workers/-/vitest-pool-workers-0.16.20.tgz", - "integrity": "sha512-buw0YgsAMT7s60wcmyxbtciEJjMJzKcWzayDMPhWaqMqfQzW+0WPLV67Lobn4C80nkNQhYocEJPnrEhLWnOf+A==", - "dev": true, - "license": "MIT", - "dependencies": { - "cjs-module-lexer": "1.2.3", - "esbuild": "0.28.1", - "miniflare": "4.20260625.0", - "wrangler": "4.105.0", - "zod": "3.25.76" - }, - "peerDependencies": { - "@vitest/runner": "^4.1.0", - "@vitest/snapshot": "^4.1.0", - "vitest": "^4.1.0" - } - }, - "node_modules/@cloudflare/vitest-pool-workers/node_modules/zod": { - "version": "3.25.76", - "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", - "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", - "dev": true, - "license": "MIT", - "funding": { - "url": "https://github.com/sponsors/colinhacks" - } - }, "node_modules/@cloudflare/workerd-darwin-64": { "version": "1.20260625.1", "resolved": "https://registry.npmjs.org/@cloudflare/workerd-darwin-64/-/workerd-darwin-64-1.20260625.1.tgz", @@ -2920,6 +2890,72 @@ "node": ">=14.0.0" } }, + "node_modules/@tailwindcss/oxide-wasm32-wasi/node_modules/@emnapi/core": { + "version": "1.11.1", + "dev": true, + "inBundle": true, + "license": "MIT", + "optional": true, + "dependencies": { + "@emnapi/wasi-threads": "1.2.2", + "tslib": "^2.4.0" + } + }, + "node_modules/@tailwindcss/oxide-wasm32-wasi/node_modules/@emnapi/runtime": { + "version": "1.11.1", + "dev": true, + "inBundle": true, + "license": "MIT", + "optional": true, + "dependencies": { + "tslib": "^2.4.0" + } + }, + "node_modules/@tailwindcss/oxide-wasm32-wasi/node_modules/@emnapi/wasi-threads": { + "version": "1.2.2", + "dev": true, + "inBundle": true, + "license": "MIT", + "optional": true, + "dependencies": { + "tslib": "^2.4.0" + } + }, + "node_modules/@tailwindcss/oxide-wasm32-wasi/node_modules/@napi-rs/wasm-runtime": { + "version": "1.1.4", + "dev": true, + "inBundle": true, + "license": "MIT", + "optional": true, + "dependencies": { + "@tybys/wasm-util": "^0.10.1" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + }, + "peerDependencies": { + "@emnapi/core": "^1.7.1", + "@emnapi/runtime": "^1.7.1" + } + }, + "node_modules/@tailwindcss/oxide-wasm32-wasi/node_modules/@tybys/wasm-util": { + "version": "0.10.2", + "dev": true, + "inBundle": true, + "license": "MIT", + "optional": true, + "dependencies": { + "tslib": "^2.4.0" + } + }, + "node_modules/@tailwindcss/oxide-wasm32-wasi/node_modules/tslib": { + "version": "2.8.1", + "dev": true, + "inBundle": true, + "license": "0BSD", + "optional": true + }, "node_modules/@tailwindcss/oxide-win32-arm64-msvc": { "version": "4.3.2", "resolved": "https://registry.npmjs.org/@tailwindcss/oxide-win32-arm64-msvc/-/oxide-win32-arm64-msvc-4.3.2.tgz", @@ -3600,13 +3636,6 @@ "url": "https://github.com/sponsors/wooorm" } }, - "node_modules/cjs-module-lexer": { - "version": "1.2.3", - "resolved": "https://registry.npmjs.org/cjs-module-lexer/-/cjs-module-lexer-1.2.3.tgz", - "integrity": "sha512-0TNiGstbQmCFwt4akjjBg5pLRTSyj/PkWQ1ZoO2zntmg9yLqSRxwEa4iCfQLGjqhiqBfOJa7W/E8wfGrTDmlZQ==", - "dev": true, - "license": "MIT" - }, "node_modules/class-variance-authority": { "version": "0.7.1", "resolved": "https://registry.npmjs.org/class-variance-authority/-/class-variance-authority-0.7.1.tgz", diff --git a/package.json b/package.json index 5801706..52a08ba 100644 --- a/package.json +++ b/package.json @@ -25,10 +25,10 @@ "migrate": "node scripts/migrate.mjs", "test": "node scripts/test.mjs", "test:watch": "vitest", + "test:browser": "vitest run --project browser", "typecheck": "tsc --noEmit" }, "devDependencies": { - "@cloudflare/vitest-pool-workers": "^0.16.20", "@tailwindcss/vite": "^4.2.2", "@testing-library/dom": "^10.4.1", "@testing-library/jest-dom": "^6.9.1", diff --git a/scripts/test.mjs b/scripts/test.mjs index 804fba1..39b5cf3 100644 --- a/scripts/test.mjs +++ b/scripts/test.mjs @@ -74,4 +74,4 @@ if (!usableEnvValue(process.env.TEST_DATABASE_URL)) { process.env.DATABASE_URL = process.env.TEST_DATABASE_URL; run(process.execPath, ['scripts/migrate.mjs']); -run(process.execPath, ['node_modules/vitest/vitest.mjs', 'run']); +run(process.execPath, ['node_modules/vitest/vitest.mjs', 'run', '--project', 'node']); diff --git a/test/e2e/dashboard.spec.tsx b/test/browser/dashboard.spec.tsx similarity index 71% rename from test/e2e/dashboard.spec.tsx rename to test/browser/dashboard.spec.tsx index 1245255..542718e 100644 --- a/test/e2e/dashboard.spec.tsx +++ b/test/browser/dashboard.spec.tsx @@ -1,13 +1,9 @@ -/** - * @vitest-environment jsdom - */ import { expect, it, describe, vi, beforeEach } from 'vitest'; -import { render, screen } from '@testing-library/react'; +import { screen } from '@testing-library/react'; import { LoginPage } from '@client/pages/login'; import { DashboardPage } from '@client/pages/dashboard'; -import { MemoryRouter } from 'react-router-dom'; import { api } from '@client/lib/api'; -import { ThemeProvider } from '@client/lib/theme'; +import { renderPage } from './render'; // Mock the API client vi.mock('@client/lib/api', () => ({ @@ -20,7 +16,7 @@ vi.mock('@client/lib/api', () => ({ } })); -describe('Frontend UI Flows (JSDOM)', () => { +describe('Frontend UI Flows', () => { beforeEach(() => { vi.clearAllMocks(); @@ -32,13 +28,7 @@ describe('Frontend UI Flows (JSDOM)', () => { }); it('renders the GitHub sign-in flow', async () => { - render( - - - - - - ); + renderPage(); const signInLink = screen.getByRole('link', { name: 'Sign in with GitHub' }); expect(signInLink.getAttribute('href')).toBe('/auth/github'); @@ -77,22 +67,17 @@ describe('Frontend UI Flows (JSDOM)', () => { total: 1 }); - render( - - - - ); + renderPage(); // Check for dashboard title (from PageHeader) - expect(await screen.findByText('Dashboard')).toBeDefined(); - + expect(await screen.findByText('Dashboard')).toBeInTheDocument(); + // Check for stats totals (using data from getStats mock) - // Note: fmtNumber might format 500 as "500" or similar - expect(screen.getByText('10')).toBeDefined(); - expect(screen.getByText('500')).toBeDefined(); + expect(await screen.findByText('10')).toBeInTheDocument(); + expect(screen.getByText('500')).toBeInTheDocument(); // Check for activity stream item - expect(screen.getByText('test-owner/test-repo')).toBeDefined(); - expect(screen.getByRole('link', { name: 'Fixing bug' })).toBeDefined(); + expect(await screen.findByText('test-owner/test-repo')).toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Fixing bug' })).toBeInTheDocument(); }); }); diff --git a/test/browser/job-detail.spec.tsx b/test/browser/job-detail.spec.tsx new file mode 100644 index 0000000..21081d3 --- /dev/null +++ b/test/browser/job-detail.spec.tsx @@ -0,0 +1,158 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; +import { JobDetailPage } from '@client/pages/job-detail'; +import { api } from '@client/lib/api'; +import { ThemeProvider } from '@client/lib/theme'; +import type { JobDetail } from '@shared/schema'; + +vi.mock('@client/lib/api', () => ({ + api: { + getJob: vi.fn(), + rerunJob: vi.fn(), + getUpdatesEmailStatus: vi.fn(), + subscribeUpdates: vi.fn(), + }, +})); + +const JOB_ID = '22222222-2222-2222-2222-222222222222'; + +const JOB: JobDetail = { + id: JOB_ID, + owner: 'acme', + repo: 'widgets', + installationId: '1', + prNumber: 42, + prTitle: 'Add retry handling', + prAuthor: 'octocat', + commitSha: 'abc123def456', + trigger: 'auto', + status: 'done', + verdict: 'comment', + fileCount: 1, + commentCount: 2, + totalInputTokens: 1200, + totalOutputTokens: 400, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + nextRetryAt: null, + startedAt: new Date().toISOString(), + finishedAt: new Date().toISOString(), + errorMessage: null, + steps: [], + checkRunId: null, + configSnapshot: null, + retryOfJobId: null, + baseSha: 'base123', + headRef: 'feature-branch', + baseRef: 'main', + summaryMarkdown: null, + reviewId: null, + summaryModel: null, + files: [ + { + id: '33333333-3333-3333-3333-333333333333', + jobId: JOB_ID, + filePath: 'src/index.ts', + fileStatus: 'done', + modelUsed: 'gpt-4o-mini', + diffLineCount: 12, + diffInput: null, + rawAiOutput: null, + parsedComments: [ + { + path: 'src/index.ts', + line: 10, + position: 3, + severity: 'P0', + category: 'security', + title: 'SQL injection risk', + body: 'User input is concatenated directly into the query string.', + codeSuggestion: null, + }, + { + path: 'src/index.ts', + line: 20, + position: 8, + severity: 'nit', + category: 'quality', + title: 'Prefer const over let', + body: 'This binding is never reassigned.', + codeSuggestion: null, + }, + ], + inputTokens: 1200, + outputTokens: 400, + durationMs: 2500, + verdict: 'comment', + fileSummary: 'Found one security issue and one style nit.', + errorMessage: null, + createdAt: new Date().toISOString(), + }, + ], +}; + +function renderJobDetail() { + return render( + + + + } /> + + + , + ); +} + +describe('JobDetailPage findings and retry', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(api.getJob).mockResolvedValue({ + status: 200, + etag: null, + lastModified: null, + notModified: false, + data: { job: JOB }, + }); + vi.mocked(api.getUpdatesEmailStatus).mockResolvedValue({ + status: 'subscribed', + email: 'user@example.com', + updatedAt: new Date().toISOString(), + }); + }); + + it('renders the job verdict and file findings', async () => { + renderJobDetail(); + + expect(await screen.findByText('Add retry handling')).toBeInTheDocument(); + expect(screen.getByText('src/index.ts', { selector: 'summary span' })).toBeInTheDocument(); + }); + + it('expands a file to reveal its inline findings', async () => { + const user = userEvent.setup(); + renderJobDetail(); + + const fileSummary = await screen.findByText('src/index.ts', { selector: 'summary span' }); + await user.click(fileSummary); + + expect(await screen.findByText('SQL injection risk')).toBeInTheDocument(); + expect(screen.getByText('Prefer const over let')).toBeInTheDocument(); + }); + + it('triggers a rerun when the re-run button is clicked', async () => { + vi.mocked(api.rerunJob).mockResolvedValue({ + job: { ...JOB, id: 'new-job-id', status: 'queued' } as any, + }); + + const user = userEvent.setup(); + renderJobDetail(); + + await screen.findByText('Add retry handling'); + await user.click(screen.getByRole('button', { name: 'Re-run job' })); + + await waitFor(() => { + expect(api.rerunJob).toHaveBeenCalledWith(JOB_ID); + }); + }); +}); diff --git a/test/browser/jobs.spec.tsx b/test/browser/jobs.spec.tsx new file mode 100644 index 0000000..0cddead --- /dev/null +++ b/test/browser/jobs.spec.tsx @@ -0,0 +1,117 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { JobsPage } from '@client/pages/jobs'; +import { api } from '@client/lib/api'; +import { renderPage } from './render'; + +vi.mock('@client/lib/api', () => ({ + api: { + getJobs: vi.fn(), + getUpdatesEmailStatus: vi.fn(), + subscribeUpdates: vi.fn(), + }, +})); + +function makeJob(overrides: Partial> = {}) { + return { + id: '1', + owner: 'test-owner', + repo: 'test-repo', + prNumber: 101, + prTitle: 'Fixing bug', + status: 'done', + trigger: 'auto', + createdAt: new Date().toISOString(), + commentCount: 2, + ...overrides, + } as any; +} + +describe('JobsPage filters and pagination', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(api.getJobs).mockResolvedValue({ jobs: [makeJob()], total: 1 }); + vi.mocked(api.getUpdatesEmailStatus).mockResolvedValue({ + status: 'subscribed', + email: 'user@example.com', + updatedAt: new Date().toISOString(), + }); + }); + + it('renders jobs from the initial load', async () => { + renderPage(); + + expect(await screen.findByText('test-owner/test-repo')).toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Fixing bug' })).toBeInTheDocument(); + }); + + it('sends the search term and resets to page 1', async () => { + const user = userEvent.setup(); + renderPage(); + + await screen.findByText('test-owner/test-repo'); + + const searchInput = screen.getByPlaceholderText('Title or #number...'); + await user.type(searchInput, 'flaky'); + + await waitFor(() => { + const lastCall = vi.mocked(api.getJobs).mock.calls.at(-1)?.[0]; + expect(lastCall).toMatchObject({ search: 'flaky', offset: 0 }); + }); + }); + + it('filters by status through the dropdown', async () => { + const user = userEvent.setup(); + renderPage(); + + await screen.findByText('test-owner/test-repo'); + + await user.click(screen.getByRole('button', { name: /All statuses/i })); + await user.click(await screen.findByRole('option', { name: 'Done' })); + + await waitFor(() => { + const lastCall = vi.mocked(api.getJobs).mock.calls.at(-1)?.[0]; + expect(lastCall).toMatchObject({ status: 'done', offset: 0 }); + }); + }); + + it('shows the empty state when no jobs are returned', async () => { + vi.mocked(api.getJobs).mockResolvedValue({ jobs: [], total: 0 }); + const user = userEvent.setup(); + renderPage(); + + const searchInput = await screen.findByPlaceholderText('Title or #number...'); + await user.type(searchInput, 'nonexistent'); + + expect(await screen.findByText('No jobs yet')).toBeInTheDocument(); + }); + + it('paginates using the Next and Previous page controls', async () => { + vi.mocked(api.getJobs).mockResolvedValue({ + jobs: [makeJob()], + total: 25, // 3 pages at the default itemsPerPage of 10 + }); + + const user = userEvent.setup(); + renderPage(); + + expect(await screen.findByText(/Page 1 of 3/)).toBeInTheDocument(); + const prevButton = screen.getByRole('button', { name: 'Previous page' }); + expect(prevButton).toBeDisabled(); + + await user.click(screen.getByRole('button', { name: 'Next page' })); + + await waitFor(() => { + const lastCall = vi.mocked(api.getJobs).mock.calls.at(-1)?.[0]; + expect(lastCall).toMatchObject({ offset: 10 }); + }); + expect(await screen.findByText(/Page 2 of 3/)).toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: 'Previous page' })); + await waitFor(() => { + const lastCall = vi.mocked(api.getJobs).mock.calls.at(-1)?.[0]; + expect(lastCall).toMatchObject({ offset: 0 }); + }); + }); +}); diff --git a/test/browser/render.tsx b/test/browser/render.tsx new file mode 100644 index 0000000..e48ddaf --- /dev/null +++ b/test/browser/render.tsx @@ -0,0 +1,13 @@ +import type { ReactNode } from 'react'; +import { render } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { ThemeProvider } from '@client/lib/theme'; + +export function renderPage(ui: ReactNode, options: { route?: string } = {}) { + const { route = '/' } = options; + return render( + + {ui} + , + ); +} diff --git a/test/browser/repos.spec.tsx b/test/browser/repos.spec.tsx new file mode 100644 index 0000000..5356aa7 --- /dev/null +++ b/test/browser/repos.spec.tsx @@ -0,0 +1,84 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { ReposPage } from '@client/pages/repos'; +import { api } from '@client/lib/api'; +import { renderPage } from './render'; +import type { RepoConfigRecord } from '@shared/schema'; + +vi.mock('@client/lib/api', () => ({ + api: { + getRepos: vi.fn(), + getGlobalConfig: vi.fn(), + getModelConfigs: vi.fn(), + updateRepoConfig: vi.fn(), + syncRepos: vi.fn(), + getUpdatesEmailStatus: vi.fn(), + subscribeUpdates: vi.fn(), + }, +})); + +const REPO: RepoConfigRecord = { + installationId: '1', + owner: 'acme', + repo: 'widgets', + parsedJson: {} as any, + updatedAt: new Date().toISOString(), + lastJobCreatedAt: null, + lastJobVerdict: null, + mainModel: null, + fallbackModels: null, + sizeOverrides: null, + enabled: true, +}; + +describe('ReposPage repository management', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(api.getRepos).mockResolvedValue({ repos: [REPO] }); + vi.mocked(api.getGlobalConfig).mockResolvedValue({ config: { main: null, fallbacks: [], size_overrides: [] } }); + vi.mocked(api.getModelConfigs).mockResolvedValue({ providers: [], configs: [], syncErrors: [] }); + vi.mocked(api.getUpdatesEmailStatus).mockResolvedValue({ + status: 'subscribed', + email: 'user@example.com', + updatedAt: new Date().toISOString(), + }); + }); + + it('renders the repo list with its enabled state', async () => { + renderPage(); + + expect(await screen.findByText('acme/widgets')).toBeInTheDocument(); + expect(screen.getByText('Enabled')).toBeInTheDocument(); + }); + + it('toggling the enabled switch patches the repo config', async () => { + vi.mocked(api.updateRepoConfig).mockResolvedValue({ ok: true }); + const user = userEvent.setup(); + renderPage(); + + await screen.findByText('acme/widgets'); + + await user.click(screen.getByRole('checkbox', { name: 'Pause reviews for acme/widgets' })); + + await waitFor(() => { + expect(api.updateRepoConfig).toHaveBeenCalledWith('acme', 'widgets', { enabled: false }); + }); + }); + + it('syncing repositories calls the sync endpoint and reloads the list', async () => { + vi.mocked(api.syncRepos).mockResolvedValue({ ok: true, synced: ['acme/widgets'] }); + const user = userEvent.setup(); + renderPage(); + + await screen.findByText('acme/widgets'); + vi.mocked(api.getRepos).mockClear(); + + await user.click(screen.getByRole('button', { name: /Sync/i })); + + await waitFor(() => { + expect(api.syncRepos).toHaveBeenCalled(); + expect(api.getRepos).toHaveBeenCalled(); + }); + }); +}); diff --git a/test/browser/settings.spec.tsx b/test/browser/settings.spec.tsx new file mode 100644 index 0000000..c3b403e --- /dev/null +++ b/test/browser/settings.spec.tsx @@ -0,0 +1,119 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { SettingsPage } from '@client/pages/settings'; +import { api } from '@client/lib/api'; +import { renderPage } from './render'; +import type { LlmProvider, ModelConfig } from '@shared/schema'; +import type { ModelConfigsResponse } from '@shared/api'; + +vi.mock('@client/lib/api', () => ({ + api: { + getModelConfigs: vi.fn(), + getGlobalConfig: vi.fn(), + getReviewSettings: vi.fn(), + refreshModelCatalog: vi.fn(), + updateProvider: vi.fn(), + createProvider: vi.fn(), + deleteProvider: vi.fn(), + updateGlobalConfig: vi.fn(), + getUpdatesEmailStatus: vi.fn(), + subscribeUpdates: vi.fn(), + }, +})); + +const PROVIDER: LlmProvider = { + id: '11111111-1111-1111-1111-111111111111', + name: 'Custom OpenAI', + apiFormat: 'openai', + baseUrl: 'https://api.example.com/v1', + enabled: false, + hasApiKey: false, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), +}; + +const MODEL_CONFIG: ModelConfig = { + modelId: 'gpt-4o-mini', + providerId: PROVIDER.id, + providerName: PROVIDER.name, + apiFormat: 'openai', + modelName: 'gpt-4o-mini', + updatedAt: new Date().toISOString(), +}; + +function modelConfigsResponse(overrides: Partial = {}): ModelConfigsResponse { + return { + providers: [PROVIDER], + configs: [MODEL_CONFIG], + syncErrors: [], + ...overrides, + }; +} + +describe('SettingsPage provider management', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(api.getModelConfigs).mockResolvedValue(modelConfigsResponse()); + vi.mocked(api.refreshModelCatalog).mockResolvedValue(modelConfigsResponse()); + vi.mocked(api.getGlobalConfig).mockResolvedValue({ config: { main: null, fallbacks: [], size_overrides: [] } }); + vi.mocked(api.getReviewSettings).mockResolvedValue({ settings: { concurrencyLevel: 'medium', maxComments: 10 } }); + vi.mocked(api.getUpdatesEmailStatus).mockResolvedValue({ + status: 'subscribed', + email: 'user@example.com', + updatedAt: new Date().toISOString(), + }); + }); + + it('renders providers and models loaded from the API', async () => { + renderPage(); + + expect((await screen.findAllByText('Custom OpenAI')).length).toBeGreaterThan(0); + // Proves the model catalog loaded: the provider row shows its model count. + expect(screen.getByText('ยท 1 model')).toBeInTheDocument(); + expect(screen.getByText('Default models')).toBeInTheDocument(); + }); + + it('edits a provider API key and saves the expected payload', async () => { + vi.mocked(api.updateProvider).mockResolvedValue({ + provider: { ...PROVIDER, hasApiKey: true }, + }); + + const user = userEvent.setup(); + renderPage(); + + await screen.findAllByText('Custom OpenAI'); + + await user.click(screen.getByRole('button', { name: 'Configure' })); + const apiKeyInput = await screen.findByPlaceholderText('sk-โ€ฆ'); + await user.type(apiKeyInput, 'sk-test-123'); + + await user.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect(api.updateProvider).toHaveBeenCalledWith(PROVIDER.id, { + name: 'Custom OpenAI', + apiFormat: 'openai', + baseUrl: 'https://api.example.com/v1', + enabled: false, + apiKey: 'sk-test-123', + }); + }); + }); + + it('shows an error alert when saving a provider fails', async () => { + vi.mocked(api.updateProvider).mockRejectedValue(new Error('Provider update failed: bad credentials')); + + const user = userEvent.setup(); + renderPage(); + + await screen.findAllByText('Custom OpenAI'); + + await user.click(screen.getByRole('button', { name: 'Configure' })); + const apiKeyInput = await screen.findByPlaceholderText('sk-โ€ฆ'); + await user.type(apiKeyInput, 'sk-test-123'); + await user.click(screen.getByRole('button', { name: 'Save' })); + + expect(await screen.findByText('Provider update failed: bad credentials')).toBeInTheDocument(); + }); +}); diff --git a/test/browser/setup.ts b/test/browser/setup.ts new file mode 100644 index 0000000..01d18c2 --- /dev/null +++ b/test/browser/setup.ts @@ -0,0 +1,7 @@ +import '@testing-library/jest-dom/vitest'; +import { afterEach } from 'vitest'; +import { cleanup } from '@testing-library/react'; + +afterEach(() => { + cleanup(); +}); diff --git a/test/browser/theme-login.spec.tsx b/test/browser/theme-login.spec.tsx new file mode 100644 index 0000000..e390c26 --- /dev/null +++ b/test/browser/theme-login.spec.tsx @@ -0,0 +1,42 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { LoginPage } from '@client/pages/login'; +import { renderPage } from './render'; + +describe('Theme toggle', () => { + beforeEach(() => { + localStorage.setItem('codra-theme', 'light'); + document.documentElement.classList.remove('dark'); + document.documentElement.setAttribute('data-theme', 'light'); + }); + + it('starts in the seeded light theme', async () => { + renderPage(); + + expect(document.documentElement.classList.contains('dark')).toBe(false); + }); + + it('toggles to dark and persists the choice in localStorage', async () => { + const user = userEvent.setup(); + renderPage(); + + await user.click(screen.getByRole('button', { name: 'Toggle theme' })); + + expect(document.documentElement.classList.contains('dark')).toBe(true); + expect(document.documentElement.getAttribute('data-theme')).toBe('dark'); + expect(localStorage.getItem('codra-theme')).toBe('dark'); + }); + + it('toggles back to light on a second click', async () => { + const user = userEvent.setup(); + renderPage(); + + const toggleButton = screen.getByRole('button', { name: 'Toggle theme' }); + await user.click(toggleButton); + await user.click(toggleButton); + + expect(document.documentElement.classList.contains('dark')).toBe(false); + expect(localStorage.getItem('codra-theme')).toBe('light'); + }); +}); diff --git a/test/formatter.spec.ts b/test/formatter.spec.ts new file mode 100644 index 0000000..4585ee5 --- /dev/null +++ b/test/formatter.spec.ts @@ -0,0 +1,148 @@ +import { describe, it, expect } from 'vitest'; +import { FormatterService } from '@server/services/formatter'; +import type { ParsedReviewComment } from '@shared/schema'; + +const BASE_URL = 'https://app.codra.example.com'; +const formatter = new FormatterService(BASE_URL); + +function comment(overrides: Partial = {}): ParsedReviewComment { + return { + path: 'src/index.ts', + line: 10, + position: 3, + severity: 'P1', + category: 'quality', + title: 'Example finding', + body: 'Something worth looking at.', + codeSuggestion: null, + ...overrides, + }; +} + +describe('FormatterService.toReviewEvent', () => { + it('maps approve to APPROVE', () => { + expect(formatter.toReviewEvent('approve')).toBe('APPROVE'); + }); + + it('maps comment to COMMENT', () => { + expect(formatter.toReviewEvent('comment')).toBe('COMMENT'); + }); +}); + +describe('FormatterService.severityIcon', () => { + it.each(['P0', 'P1', 'P2', 'P3', 'nit'] as const)('renders an icon for %s', (severity) => { + const icon = formatter.severityIcon(severity); + expect(icon).toContain(` { + expect(formatter.severityIcon('unknown' as ParsedReviewComment['severity'])).toBe('โšช'); + }); +}); + +describe('FormatterService.stripLeadingTags', () => { + it('strips a leading emoji', () => { + expect(formatter.stripLeadingTags('๐Ÿ”’ Possible SQL injection')).toBe('Possible SQL injection'); + }); + + it('strips legacy bracketed severity/category tags', () => { + expect(formatter.stripLeadingTags('[P0] [SECURITY] Possible SQL injection')).toBe('Possible SQL injection'); + }); + + it('strips a leading bracketed BUG tag', () => { + expect(formatter.stripLeadingTags('[BUG] Off-by-one error')).toBe('Off-by-one error'); + }); + + it('strips bare (unbracketed) legacy tag words', () => { + expect(formatter.stripLeadingTags('SECURITY: Possible SQL injection')).toBe('Possible SQL injection'); + }); + + it('strips mixed emoji and bracketed tags together', () => { + expect(formatter.stripLeadingTags('๐Ÿ› [P2] [BUG] Null pointer dereference')).toBe('Null pointer dereference'); + }); + + it('leaves already-clean text untouched', () => { + expect(formatter.stripLeadingTags('Possible SQL injection')).toBe('Possible SQL injection'); + }); +}); + +describe('FormatterService.formatInlineComment', () => { + it('includes the severity icon and title', () => { + const output = formatter.formatInlineComment(comment({ severity: 'P0', title: 'SQL injection risk', body: 'Details here.' })); + expect(output).toContain(`SQL injection risk'); + expect(output).toContain('Details here.'); + }); + + it('dedupes a body whose first line repeats the title', () => { + const output = formatter.formatInlineComment( + comment({ title: 'SQL injection risk', body: 'SQL injection risk\n\nUser input is concatenated into the query.' }), + ); + expect(output).toBe( + `${formatter.severityIcon('P1')} SQL injection risk\n\nUser input is concatenated into the query.`, + ); + expect(output.match(/SQL injection risk/g)).toHaveLength(1); + }); + + it('dedupes when the title itself is a prefix of the first body line', () => { + const output = formatter.formatInlineComment( + comment({ title: 'SQL injection', body: 'SQL injection risk in query builder\n\nUser input is concatenated into the query.' }), + ); + expect(output.match(/SQL injection/g)).toHaveLength(1); + expect(output).toContain('User input is concatenated into the query.'); + }); + + it('keeps the body intact when it does not duplicate the title', () => { + const output = formatter.formatInlineComment( + comment({ title: 'Null check missing', body: 'This value can be undefined at runtime.' }), + ); + expect(output).toContain('Null check missing'); + expect(output).toContain('This value can be undefined at runtime.'); + }); +}); + +describe('FormatterService.summarizeVerdict', () => { + it('approves when there are no P0/P1/P2 comments and no failures', () => { + expect(formatter.summarizeVerdict([comment({ severity: 'P3' }), comment({ severity: 'nit' })], false)).toEqual({ + verdict: 'approve', + errors: 0, + warnings: 0, + }); + }); + + it('requires comment verdict with P0 findings, counted as errors', () => { + const result = formatter.summarizeVerdict([comment({ severity: 'P0' }), comment({ severity: 'P0' })], false); + expect(result).toEqual({ verdict: 'comment', errors: 2, warnings: 0 }); + }); + + it('requires comment verdict with P1 findings, counted as errors', () => { + const result = formatter.summarizeVerdict([comment({ severity: 'P1' })], false); + expect(result).toEqual({ verdict: 'comment', errors: 1, warnings: 0 }); + }); + + it('requires comment verdict with P2 findings, counted as warnings only', () => { + const result = formatter.summarizeVerdict([comment({ severity: 'P2' })], false); + expect(result).toEqual({ verdict: 'comment', errors: 0, warnings: 1 }); + }); + + it('requires comment verdict when hasFailures is true even with no comments', () => { + expect(formatter.summarizeVerdict([], true)).toEqual({ verdict: 'comment', errors: 0, warnings: 0 }); + }); +}); + +describe('FormatterService.formatReviewOverview', () => { + it('truncates the commit sha to 10 characters and interpolates the bot username', () => { + const output = formatter.formatReviewOverview('abcdef1234567890', 'codra-app'); + + expect(output).toContain('**Reviewed commit:** `abcdef1234`'); + expect(output).toContain('@codra-app review'); + expect(output).toContain('@codra-app address that feedback'); + expect(output).toContain(`${BASE_URL}/repos`); + }); + + it('handles a short commit sha without throwing', () => { + const output = formatter.formatReviewOverview('abc', 'codra-app'); + expect(output).toContain('**Reviewed commit:** `abc`'); + }); +}); diff --git a/test/github-fetch-mock.ts b/test/github-fetch-mock.ts new file mode 100644 index 0000000..116e7e3 --- /dev/null +++ b/test/github-fetch-mock.ts @@ -0,0 +1,140 @@ +import { vi } from 'vitest'; + +export type RecordedGitHubCall = { + method: string; + path: string; + accept: string | null; + body: any; +}; + +export type ReviewResponseScript = Array<{ status: number; id?: number }>; + +export type GitHubFetchMockFixtures = { + owner: string; + repo: string; + prNumber: number; + pull: { + number: number; + title: string | null; + body: string | null; + draft: boolean; + head: { sha: string; ref: string }; + base: { sha: string; ref: string }; + user: { login: string }; + }; + diff: string; + /** Scripted status sequence for successive POST .../reviews calls. Defaults to a single 200. */ + reviewResponses?: ReviewResponseScript; +}; + +/** + * Stubs global fetch so the real GitHubClient (core/github.ts) can run end-to-end + * against a fake api.github.com. Every response is terminal (2xx/404/422) so + * GitHubClient's retry/backoff logic never triggers a real-time sleep. + */ +export function installGitHubFetchMock(fixtures: GitHubFetchMockFixtures) { + const calls: RecordedGitHubCall[] = []; + const originalFetch = globalThis.fetch; + const repoPrefix = `/repos/${fixtures.owner}/${fixtures.repo}`; + const reviewsListPath = `${repoPrefix}/pulls/${fixtures.prNumber}/reviews`; + const reviewResponses = fixtures.reviewResponses ?? [{ status: 200, id: 5150 }]; + let reviewCallIndex = 0; + + const existingLabels = new Map(); + const issueLabels = new Set(); + + async function handler(input: RequestInfo | URL, init?: RequestInit): Promise { + const rawUrl = typeof input === 'string' ? input : input instanceof Request ? input.url : String(input); + const url = new URL(rawUrl); + + if (url.hostname !== 'api.github.com') { + // core/telemetry.ts fires a real POST to codra.run on every finalize; return a fast synthetic + // response instead of letting it reach the real network from a test run. + return new Response('{}', { status: 200, headers: { 'content-type': 'application/json' } }); + } + + const method = (init?.method ?? 'GET').toUpperCase(); + const headers = (init?.headers ?? {}) as Record; + const accept = headers.Accept ?? null; + let body: any = null; + if (typeof init?.body === 'string') { + try { + body = JSON.parse(init.body); + } catch { + body = init.body; + } + } + + calls.push({ method, path: url.pathname, accept, body }); + + const json = (data: unknown, status = 200) => + new Response(JSON.stringify(data), { status, headers: { 'content-type': 'application/json' } }); + + if (method === 'GET' && url.pathname === `${repoPrefix}/pulls/${fixtures.prNumber}`) { + if (accept === 'application/vnd.github.v3.diff') { + return new Response(fixtures.diff, { status: 200 }); + } + return json(fixtures.pull); + } + + if (method === 'POST' && url.pathname === `${repoPrefix}/check-runs`) { + return json({ id: 9001 }, 201); + } + + if (method === 'PATCH' && /\/check-runs\/\d+$/.test(url.pathname)) { + return json({}); + } + + if (method === 'GET' && url.pathname === reviewsListPath) { + // findBotReviewForCommit's existing-review lookup (only hit when a finalize retries past + // the posting step). No prior review exists in these fixtures. + return json([]); + } + + if (method === 'POST' && url.pathname === reviewsListPath) { + const script = reviewResponses[Math.min(reviewCallIndex, reviewResponses.length - 1)]; + reviewCallIndex += 1; + if (script.status >= 400) { + return json({ message: 'Unprocessable Entity' }, script.status); + } + return json({ id: script.id ?? 5150 }, script.status); + } + + const labelLookup = new RegExp(`^${repoPrefix}/labels/([^/]+)$`).exec(url.pathname); + if (method === 'GET' && labelLookup) { + const name = decodeURIComponent(labelLookup[1]); + return existingLabels.has(name) ? json({ name }) : json({ message: 'Not Found' }, 404); + } + + if (method === 'POST' && url.pathname === `${repoPrefix}/labels`) { + existingLabels.set(body.name, body.color); + return json({ name: body.name, color: body.color }, 201); + } + + if (method === 'GET' && url.pathname === `${repoPrefix}/issues/${fixtures.prNumber}/labels`) { + return json(Array.from(issueLabels, (name) => ({ name }))); + } + + if (method === 'POST' && url.pathname === `${repoPrefix}/issues/${fixtures.prNumber}/labels`) { + for (const name of body?.labels ?? []) issueLabels.add(name); + return json([]); + } + + const labelRemoval = new RegExp(`^${repoPrefix}/issues/${fixtures.prNumber}/labels/([^/]+)$`).exec(url.pathname); + if (method === 'DELETE' && labelRemoval) { + issueLabels.delete(decodeURIComponent(labelRemoval[1])); + return json([]); + } + + return json({ message: `Unhandled mock GitHub route: ${method} ${url.pathname}` }, 404); + } + + vi.stubGlobal('fetch', handler); + + return { + calls, + restore() { + vi.stubGlobal('fetch', originalFetch); + }, + }; +} diff --git a/test/helpers.ts b/test/helpers.ts index 1f5cabb..1ea8d64 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -1,6 +1,8 @@ import type { AppBindings } from '@server/env'; import { encryptLlmApiKey } from '@server/core/llm-crypto'; import { queryRows } from '@server/db/client'; +import { updateModelConfig } from '@server/db/model-configs'; +import { updateGlobalConfig } from '@server/core/config'; export class MemoryKV { private readonly store = new Map(); @@ -186,6 +188,63 @@ ${lines.map((l) => `+${l}`).join('\n')}`; .join('\n'); } +/** + * Seeds a cached installation token so the real GitHubClient (core/github.ts) + * can run without touching APP_PRIVATE_KEY/GITHUB_APP_ID (which throw in + * createTestEnv). GitHubClient.getInstallationToken() checks this cache key + * before doing any JWT signing. + */ +export async function seedInstallationToken(env: AppBindings, installationId: string, token = 'test-installation-token') { + await env.APP_KV.put( + `install:${installationId}`, + JSON.stringify({ + token, + expiresAt: new Date(Date.now() + 60 * 60 * 1000).toISOString(), + }), + ); +} + +/** + * Registers a model as the global review strategy so the real ModelService + * (services/model.ts) can resolve a model instead of throwing "No review + * model strategy is configured". + */ +export async function seedDefaultModelStrategy(env: AppBindings, modelId: string, providerName = 'Cloudflare') { + const [provider] = await queryRows<{ id: string }>( + env, + `SELECT id FROM llm_providers WHERE name = $1`, + [providerName], + ); + if (!provider) { + throw new Error(`Test provider "${providerName}" not found; check that migrations seeded it.`); + } + + await updateModelConfig(env, { + modelId, + providerId: provider.id, + modelName: modelId, + }); + await updateGlobalConfig(env, { main: modelId, fallbacks: [], size_overrides: [] }); +} + +/** + * Signs a raw webhook body the same way GitHub does, for driving /webhook directly. + */ +export async function signWebhookPayload(secret: string, payload: string) { + const encoder = new TextEncoder(); + const key = await crypto.subtle.importKey( + 'raw', + encoder.encode(secret), + { name: 'HMAC', hash: 'SHA-256' }, + false, + ['sign'], + ); + const signature = await crypto.subtle.sign('HMAC', key, encoder.encode(payload)); + return `sha256=${Array.from(new Uint8Array(signature)) + .map((b) => b.toString(16).padStart(2, '0')) + .join('')}`; +} + /** * Creates a mock GitHub Webhook payload for a PR opened event. */ diff --git a/test/pr-review-pipeline.spec.ts b/test/pr-review-pipeline.spec.ts new file mode 100644 index 0000000..d926696 --- /dev/null +++ b/test/pr-review-pipeline.spec.ts @@ -0,0 +1,292 @@ +import { createApp } from '@server/app'; +import { runReviewJob } from '@server/core/review'; +import { queryRows, runWithDb } from '@server/db/client'; +import { findExistingJobForHead } from '@server/db/jobs'; +import { + createMockPRWebhook, + createTestEnv, + generateMockDiff, + hasConfiguredTestDatabaseUrl, + seedDefaultModelStrategy, + seedInstallationToken, + signWebhookPayload, +} from './helpers'; +import { installGitHubFetchMock } from './github-fetch-mock'; + +const dbDescribe = hasConfiguredTestDatabaseUrl() ? describe : describe.skip; +const PIPELINE_TEST_TIMEOUT_MS = 60_000; +const MODEL_ID = 'test-review-model'; +const INSTALLATION_ID = 987654; + +const AI_WITH_ISSUE = JSON.stringify({ + findings: [ + { + title: 'Possible null dereference', + body: 'This value could be null at runtime and is not checked before use.', + priority: 0, + code_location: { line: 3 }, + }, + ], + overall_explanation: 'Found a null-safety issue.', + overall_correctness: 'patch is incorrect', + overall_confidence_score: 0.9, +}); + +const AI_CLEAN = JSON.stringify({ + findings: [], + overall_explanation: 'No issues found.', + overall_correctness: 'patch is correct', + overall_confidence_score: 0.95, +}); + +function fakeAiBinding(responseText: string) { + return { + async run() { + return { response: responseText, usage: { prompt_tokens: 120, completion_tokens: 40 } }; + }, + }; +} + +function buildFixture(repo: string, prNumber: number) { + const headSha = 'a'.repeat(40); + const baseSha = 'b'.repeat(40); + const diff = generateMockDiff([{ path: 'src/app.ts', content: 'line1\nline2\nline3\nline4\nline5' }]); + + const pull = { + number: prNumber, + title: 'Add null check', + body: 'This PR adds a null check.', + draft: false, + head: { sha: headSha, ref: 'feature' }, + base: { sha: baseSha, ref: 'main' }, + user: { login: 'octocat' }, + }; + + const webhookPayload = createMockPRWebhook({ + action: 'opened', + installation: { id: INSTALLATION_ID }, + repository: { name: repo, owner: { login: 'test-owner' } }, + pull_request: pull, + }); + + return { headSha, baseSha, diff, pull, webhookPayload }; +} + +async function postWebhook(app: ReturnType, env: ReturnType, payload: unknown) { + const body = JSON.stringify(payload); + const signature = await signWebhookPayload(env.GITHUB_APP_WEBHOOK_SECRET, body); + + return app.request( + 'http://codra.test/webhook', + { + method: 'POST', + headers: { + 'x-github-event': 'pull_request', + 'x-github-delivery': `delivery-${Date.now()}-${Math.random()}`, + 'x-hub-signature-256': signature, + 'content-type': 'application/json', + }, + body, + }, + env, + ); +} + +// The review pipeline now runs through Cloudflare Workflows in production: runReviewJob no longer +// re-enqueues the next phase onto REVIEW_QUEUE itself, it returns `{action:'next_phase', phase, +// delaySeconds}` and the caller (normally ReviewWorkflow's step loop) decides whether to continue. +// This mirrors the `runAndDrain` helper in test/review-flow.spec.ts: drive runReviewJob directly, +// chasing the returned phase, and backdate last_queue_message_at so the next claim doesn't read as +// 'busy' (claimJobLease treats a future last_queue_message_at as a fresh lease held elsewhere). +async function runAndDrain(env: ReturnType, message: Parameters[1]) { + await runWithDb(env, async () => { + let currentMessage: typeof message | null = message; + let retries = 0; + const MAX_RETRIES = 5; + + while (currentMessage) { + const result = await runReviewJob(env, currentMessage); + if (result.action === 'next_phase') { + currentMessage = { ...currentMessage, phase: result.phase }; + retries = 0; + const jobId = (currentMessage as any).jobId; + if (jobId) { + await queryRows(env, `UPDATE jobs SET last_queue_message_at = now() - interval '5 seconds' WHERE id = $1`, [jobId]); + } + } else if (result.action === 'retry') { + if (++retries > MAX_RETRIES) throw new Error('Max retries exceeded'); + break; + } else { + currentMessage = null; + } + } + }); +} + +// runReviewJob throttles admission of a fresh 'queued' job once REVIEW_CONCURRENCY_LIMITS[level] +// other jobs are 'running' (globally, with no per-installation/repo scoping). Other spec files in +// this shared-DB suite intentionally leave jobs 'running' to exercise that same admission-control +// path, which would otherwise make this spec's outcome depend on file execution order. Since +// fileParallelism is false, nothing else touches the DB concurrently with this file, so it's safe +// to clear stray 'running' rows before each test. +async function clearStrayRunningJobs(env: ReturnType) { + await queryRows(env, `UPDATE jobs SET status = 'failed', error_msg = 'stray running job cleared before pipeline test' WHERE status = 'running'`); +} + +dbDescribe('PR review pipeline (real GitHubClient + real ModelService)', () => { + const app = createApp(); + + it('posts a COMMENT review with correctly positioned inline comments and manages labels', async () => { + const repo = `pipeline-repo-${Date.now()}-comment`; + const env = createTestEnv({ AI: fakeAiBinding(AI_WITH_ISSUE) as any }); + await seedInstallationToken(env, String(INSTALLATION_ID)); + await seedDefaultModelStrategy(env, MODEL_ID); + await clearStrayRunningJobs(env); + + const { headSha, diff, pull, webhookPayload } = buildFixture(repo, 11); + const { calls, restore } = installGitHubFetchMock({ + owner: 'test-owner', + repo, + prNumber: 11, + pull, + diff, + }); + + try { + const response = await postWebhook(app, env, webhookPayload); + expect(response.status).toBe(202); + const json = await response.json() as any; + expect(json.message).toBe('queued'); + expect((env.REVIEW_QUEUE as any).sent).toHaveLength(1); + + const initialMessage = (env.REVIEW_QUEUE as any).sent[0]; + await runAndDrain(env, initialMessage); + + const finalJob = await findExistingJobForHead(env, { + owner: 'test-owner', + repo, + prNumber: 11, + commitSha: headSha, + trigger: 'auto', + }); + expect(finalJob?.status).toBe('done'); + + const checkRunCreate = calls.find((c) => c.method === 'POST' && c.path.endsWith('/check-runs')); + expect(checkRunCreate?.body).toMatchObject({ name: 'Codra', head_sha: headSha }); + + const checkRunUpdates = calls.filter((c) => c.method === 'PATCH' && /\/check-runs\/\d+$/.test(c.path)); + expect(checkRunUpdates.length).toBeGreaterThan(0); + expect(checkRunUpdates.at(-1)?.body).toMatchObject({ status: 'completed' }); + + const reviewCall = calls.find((c) => c.method === 'POST' && c.path.endsWith('/reviews')); + expect(reviewCall?.body).toMatchObject({ + commit_id: headSha, + event: 'COMMENT', + }); + expect(reviewCall?.body.comments).toEqual([ + expect.objectContaining({ + path: 'src/app.ts', + position: 3, + }), + ]); + expect(reviewCall?.body.comments[0].body).toContain('Possible null dereference'); + + const labelLookup = calls.find((c) => c.method === 'GET' && c.path.includes('/labels/')); + expect(labelLookup).toBeDefined(); + const labelCreate = calls.find((c) => c.method === 'POST' && c.path.endsWith('/labels') && !c.path.includes('/issues/')); + expect(labelCreate?.body).toMatchObject({ name: 'review: needs-attention' }); + const issueLabelAdd = calls.find((c) => c.method === 'POST' && /\/issues\/\d+\/labels$/.test(c.path)); + expect(issueLabelAdd?.body).toMatchObject({ labels: ['review: needs-attention'] }); + } finally { + restore(); + } + }, PIPELINE_TEST_TIMEOUT_MS); + + it('approves a clean PR with no inline comments', async () => { + const repo = `pipeline-repo-${Date.now()}-approve`; + const env = createTestEnv({ AI: fakeAiBinding(AI_CLEAN) as any }); + await seedInstallationToken(env, String(INSTALLATION_ID)); + await seedDefaultModelStrategy(env, MODEL_ID); + await clearStrayRunningJobs(env); + + const { headSha, diff, pull, webhookPayload } = buildFixture(repo, 22); + const { calls, restore } = installGitHubFetchMock({ + owner: 'test-owner', + repo, + prNumber: 22, + pull, + diff, + }); + + try { + const response = await postWebhook(app, env, webhookPayload); + expect(response.status).toBe(202); + + const initialMessage = (env.REVIEW_QUEUE as any).sent[0]; + await runAndDrain(env, initialMessage); + + const finalJob = await findExistingJobForHead(env, { + owner: 'test-owner', + repo, + prNumber: 22, + commitSha: headSha, + trigger: 'auto', + }); + expect(finalJob?.status).toBe('done'); + expect(finalJob?.verdict).toBe('approve'); + + const reviewCall = calls.find((c) => c.method === 'POST' && c.path.endsWith('/reviews')); + expect(reviewCall?.body).toMatchObject({ event: 'APPROVE', comments: [] }); + + const checkRunUpdates = calls.filter((c) => c.method === 'PATCH' && /\/check-runs\/\d+$/.test(c.path)); + expect(checkRunUpdates.at(-1)?.body).toMatchObject({ status: 'completed', conclusion: 'success' }); + + const labelCreate = calls.find((c) => c.method === 'POST' && c.path.endsWith('/labels') && !c.path.includes('/issues/')); + expect(labelCreate?.body).toMatchObject({ name: 'review: approved' }); + } finally { + restore(); + } + }, PIPELINE_TEST_TIMEOUT_MS); + + it('retries review creation without inline comments when GitHub returns 422', async () => { + const repo = `pipeline-repo-${Date.now()}-422`; + const env = createTestEnv({ AI: fakeAiBinding(AI_WITH_ISSUE) as any }); + await seedInstallationToken(env, String(INSTALLATION_ID)); + await seedDefaultModelStrategy(env, MODEL_ID); + await clearStrayRunningJobs(env); + + const { headSha, diff, pull, webhookPayload } = buildFixture(repo, 33); + const { calls, restore } = installGitHubFetchMock({ + owner: 'test-owner', + repo, + prNumber: 33, + pull, + diff, + reviewResponses: [{ status: 422 }, { status: 200, id: 7001 }], + }); + + try { + const response = await postWebhook(app, env, webhookPayload); + expect(response.status).toBe(202); + + const initialMessage = (env.REVIEW_QUEUE as any).sent[0]; + await runAndDrain(env, initialMessage); + + const finalJob = await findExistingJobForHead(env, { + owner: 'test-owner', + repo, + prNumber: 33, + commitSha: headSha, + trigger: 'auto', + }); + expect(finalJob?.status).toBe('done'); + + const reviewCalls = calls.filter((c) => c.method === 'POST' && c.path.endsWith('/reviews')); + expect(reviewCalls).toHaveLength(2); + expect(reviewCalls[0].body.comments.length).toBeGreaterThan(0); + expect(reviewCalls[1].body.comments).toEqual([]); + } finally { + restore(); + } + }, PIPELINE_TEST_TIMEOUT_MS); +}); diff --git a/test/webhook-handling.spec.ts b/test/webhook-handling.spec.ts index 965e603..9114211 100644 --- a/test/webhook-handling.spec.ts +++ b/test/webhook-handling.spec.ts @@ -1,5 +1,5 @@ import { createApp } from '@server/app'; -import { createMockPRWebhook, createTestEnv } from './helpers'; +import { createMockPRWebhook, createTestEnv, signWebhookPayload as signPayload } from './helpers'; import { vi } from 'vitest'; // Mock GitHubClient to avoid real JWT signing and network calls @@ -14,21 +14,6 @@ vi.mock('@server/core/github', async (importOriginal) => { }; }); -async function signPayload(secret: string, payload: string) { - const encoder = new TextEncoder(); - const key = await crypto.subtle.importKey( - 'raw', - encoder.encode(secret), - { name: 'HMAC', hash: 'SHA-256' }, - false, - ['sign'], - ); - const signature = await crypto.subtle.sign('HMAC', key, encoder.encode(payload)); - return `sha256=${Array.from(new Uint8Array(signature)) - .map((b) => b.toString(16).padStart(2, '0')) - .join('')}`; -} - describe('Webhook Handling Suite', () => { const env = createTestEnv(); const app = createApp(); diff --git a/vitest.config.ts b/vitest.config.ts index 6431215..d1b4e65 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,5 +1,6 @@ import { defineConfig } from 'vitest/config'; import react from '@vitejs/plugin-react'; +import { playwright } from '@vitest/browser-playwright'; import { resolve } from 'path'; export default defineConfig({ @@ -15,9 +16,32 @@ export default defineConfig({ }, test: { globals: true, - environment: 'node', - include: ['test/**/*.spec.ts', 'test/**/*.spec.tsx'], - setupFiles: ['./test/setup.ts'], - fileParallelism: false, + projects: [ + { + extends: true, + test: { + name: 'node', + environment: 'node', + include: ['test/**/*.spec.ts'], + exclude: ['test/browser/**'], + setupFiles: ['./test/setup.ts'], + fileParallelism: false, + }, + }, + { + extends: true, + test: { + name: 'browser', + include: ['test/browser/**/*.spec.tsx'], + setupFiles: ['./test/browser/setup.ts'], + browser: { + enabled: true, + headless: true, + provider: playwright(), + instances: [{ browser: 'chromium' }], + }, + }, + }, + ], }, });