Skip to content

test: add browser interaction tests, formatter/pipeline coverage, and CI browser support #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
michnicki wants to merge 10 commits into from
Closed

test: add browser interaction tests, formatter/pipeline coverage, and CI browser support #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bf9f4b7
Merge pull request #18 from devarshishimpi/dev
devarshishimpi Jun 8, 2026
3fb6122
Merge pull request #29 from devarshishimpi/dev
devarshishimpi Jul 11, 2026
936109f
test: wire vitest browser mode with Chromium
michnicki Jul 12, 2026
cfa3e12
test: add jobs list and theme toggle browser interaction tests
michnicki Jul 12, 2026
0965c2f
test: add settings, repos, and job-detail browser interaction tests
michnicki Jul 12, 2026
fd2193d
test: add unit tests for FormatterService
michnicki Jul 12, 2026
c21ca1f
test: add end-to-end PR review pipeline test against real GitHubClient
michnicki Jul 12, 2026
289a250
ci: install Chromium and run browser tests
michnicki Jul 12, 2026
6c552dd
fix: remove unused Cloudflare vitest pool and update vitest to patch …
michnicki Jul 12, 2026
914046a
test: fix suite against upstream's Workflow-based review rewrite
michnicki Jul 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,20 @@ jobs:
- name: Install dependencies
run: npm ci

- name: Cache Playwright browsers

@codra-app-personal codra-app-personal Bot Jul 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Potential Playwright Cache Inconsistency

The cache key for Playwright relies solely on package-lock.json. If the Playwright version is pinned in package.json but not updated in lockfile changes, or if environmental factors change, the cache might become stale or inconsistent. Furthermore, running 'npx playwright install' after restoring a cache might lead to partial installation states or redundant downloads if the cache is corrupted.

Suggested change
- name: Cache Playwright browsers
Include the Playwright version or a specific dependency version in the cache key to ensure consistency across upgrades.

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
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,7 @@ vite.config.ts.timestamp-*
.wrangler

.agent

# Vitest browser mode failure screenshots and attachments
test/browser/__screenshots__/
.vitest-attachments/
103 changes: 66 additions & 37 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
"migrate": "node scripts/migrate.mjs",
"test": "node scripts/test.mjs",
"test:watch": "vitest",
"test:browser": "vitest run --project browser",

@codra-app-personal codra-app-personal Bot Jul 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Missing package dependency for new test script

The added script 'test:browser' relies on the 'vitest' runner being configured to handle a 'browser' project. The diff removes '@cloudflare/vitest-pool-workers', which is often associated with browser/worker testing environments. If this script is intended to run browser tests, ensure that the necessary browser providers (like Playwright or WebdriverIO) are explicitly defined in the devDependencies, otherwise the new CI step will fail upon execution.

"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",
Expand Down
2 changes: 1 addition & 1 deletion scripts/test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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']);

@codra-app-personal codra-app-personal Bot Jul 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Use of absolute node_modules paths

Hardcoding the path to 'node_modules/vitest/vitest.mjs' is fragile. It assumes the script is always executed from the root of the project and that Vitest is hoisted to the top-level node_modules, which might not be true in monorepo structures or different package manager configurations. It is safer to use 'require.resolve' or 'npm bin'/'npx' patterns to locate the executable.

Suggested change
run(process.execPath, ['node_modules/vitest/vitest.mjs', 'run', '--project', 'node']);
run(process.execPath, [require.resolve('vitest/vitest.mjs'), 'run', '--project', 'node']);

37 changes: 11 additions & 26 deletions test/e2e/dashboard.spec.tsx → test/browser/dashboard.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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', () => ({
Expand All @@ -20,7 +16,7 @@ vi.mock('@client/lib/api', () => ({
}
}));

describe('Frontend UI Flows (JSDOM)', () => {
describe('Frontend UI Flows', () => {

beforeEach(() => {
vi.clearAllMocks();
Expand All @@ -32,13 +28,7 @@ describe('Frontend UI Flows (JSDOM)', () => {
});

it('renders the GitHub sign-in flow', async () => {
render(
<ThemeProvider>
<MemoryRouter>
<LoginPage />
</MemoryRouter>
</ThemeProvider>
);
renderPage(<LoginPage />);

const signInLink = screen.getByRole('link', { name: 'Sign in with GitHub' });
expect(signInLink.getAttribute('href')).toBe('/auth/github');
Expand Down Expand Up @@ -77,22 +67,17 @@ describe('Frontend UI Flows (JSDOM)', () => {
total: 1
});

render(
<MemoryRouter>
<DashboardPage />
</MemoryRouter>
);
renderPage(<DashboardPage />);

// 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();
});
});
Loading
Loading