Test Review

Review tests for a feature to catch coverage gaps and quality issues before PR.

Instructions

• Identify the feature being tested (from user or recent git changes)

• Find related source files: validations, tRPC routers, components, pages

• Find related test files: vitest and playwright

• Check coverage gaps - what's missing?

• Check test quality - are assertions meaningful?

• Check for common mistakes

• Report findings with file:line references

Coverage Gaps

Validation Schemas

• Happy path with valid input

• Each required field missing

• Invalid formats (email, date, etc.)

• Edge cases (empty string, zero, negative numbers)

• Type coercion if applicable

tRPC Procedures

• Success case returns expected shape

• Error case (not found, unauthorized)

• Input validation rejects bad data

• Side effects happen (database updated, email sent)

React Components

• Renders without crashing

• Displays data correctly

• Loading state

• Error state

• User interactions (click, submit, type)

• Accessibility (keyboard nav, screen reader)

E2E (Playwright)

• Happy path flow start to finish

• Form validation errors shown to user

• Error recovery (retry, go back)

• Empty states

Test Quality

Meaningful Assertions

// Bad - just checks truthy expect(result).toBeTruthy()

// Good - checks specific value expect(result.id).toBe('123') expect(result.items).toHaveLength(3)

Independent Tests

• No shared mutable state between tests

• Each test sets up its own data

• Order of tests doesn't matter

Realistic Mocks

// Bad - mock returns nothing useful const mockFetch = jest.fn()

// Good - mock returns realistic data const mockFetch = jest.fn().mockResolvedValue({ id: '123', name: 'Test User', createdAt: new Date(), })

Error Testing

// Bad - just checks it throws expect(() => fn()).toThrow()

// Good - checks specific error expect(() => fn()).toThrow('User not found') expect(() => fn()).toThrow(NotFoundError)

Common Mistakes

General

• No .only or .skip left in code

• No console.log in tests

• No hardcoded IDs that won't exist in CI

• No time-dependent tests without mocking Date.now()

Async Issues

// Bad - no await, test passes before async completes it('creates user', () => { createUser({ name: 'Test' }) expect(db.users).toHaveLength(1) })

// Good - awaits async operation it('creates user', async () => { await createUser({ name: 'Test' }) expect(db.users).toHaveLength(1) })

Playwright-Specific

• Uses semantic locators (getByRole , getByLabel ) not CSS selectors

• No page.waitForTimeout()

• use waitForResponse , waitForSelector , or expect().toBeVisible()

• Tests don't depend on previous test's state

• Assertions check visible outcomes, not implementation

// Bad - fragile selector, arbitrary timeout await page.click('.btn-submit') await page.waitForTimeout(1000)

// Good - semantic locator, waits for outcome await page.getByRole('button', { name: 'Submit' }).click() await expect(page.getByText('Saved successfully')).toBeVisible()

Vitest-Specific

• Mocks are reset between tests (vi.clearAllMocks() in beforeEach )

• No network calls without MSW or manual mocking

• describe blocks group related tests logically

Output Format

Test Review: [feature]

Coverage Gaps

• validations/booking.ts has no tests for updateBookingSchema
• No E2E test for delete flow
• Missing error state test for BookingForm component

Quality Issues

• tests/vitest/booking.test.ts:23 - assertion just checks truthy, verify specific value
• tests/vitest/booking.test.ts:45 - mock returns empty object, use realistic data

Common Mistakes

• tests/playwright/booking.spec.ts:12 - uses waitForTimeout(2000), use waitForResponse or assertion
• tests/vitest/booking.test.ts:67 - .only left in code

Passed

• Validation schema has happy path + required field tests
• tRPC procedures test success and not-found cases
• No hardcoded IDs
• Semantic locators used throughout

Notes

• Focus on changed files first, but check related test files too

• Not every gap needs fixing - use judgment on risk vs effort

• Flag .only and .skip as blockers - these break CI