playwright-code-review

playwright-code-review

Reviews Playwright test code against a comprehensive checklist of best practices, security concerns, and maintainability standards. Runs on-demand or after test generation. Emits findings with severity levels: BLOCKER, SUGGESTION, NIT.

Review Checklist

Category	Finding	Severity	Description	Example
Waits	Hardcoded page.waitForTimeout()	BLOCKER	Use web-first assertions instead	Replace await page.waitForTimeout(1000) with await expect(locator).toBeVisible()
Waits	page.waitForNavigation() outside context	BLOCKER	Can race with page loads; use page.waitForNavigation({ waitUntil: 'networkidle' })	Check usage context
Locators	page.$() or page.$$()	BLOCKER	Deprecated; use page.locator()	Replace page.$('.button') with page.locator('.button')
Locators	page.$eval() or page.$$eval()	BLOCKER	Deprecated; breaks with async operations	Use page.locator() + assertions instead
Selectors	XPath selectors (e.g., //div[@id='submit'])	BLOCKER	Brittle; use semantic or role-based locators	Replace //button[@type='submit'] with getByRole('button', { name: 'Submit' })
Test Isolation	Test depends on another test (implicit state)	BLOCKER	Tests must be independent; use fixtures for setup	Avoid test.only() or interdependent test order
Security	Hardcoded credentials (password, API key)	BLOCKER	Externalize via env vars or fixtures	Replace password: 'SecretPass123' with process.env.TEST_PASSWORD
Selectors	Non-user-facing CSS class selectors (e.g., .css-12345)	SUGGESTION	Brittle; prefer getByRole(), getByLabel(), or data-testid	Replace .css-12345 with getByRole('button') or getByTestId('submit-btn')
Waits	Manual page.waitForSelector() or page.waitForFunction()	SUGGESTION	Use web-first assertions or page.locator().waitFor()	Replace page.waitForSelector() with expect(page.locator()).toBeVisible()
Assertions	Missing assertion message (e.g., expect().toBe())	SUGGESTION	Add descriptive message for debugging	Add .toBe(expected, 'descriptive message')
Setup	beforeAll() hook with side effects (DB mutations, API calls)	SUGGESTION	Side effects belong in setup project; avoid in hooks	Move DB setup to dedicated setup test
Fixtures	Not using Playwright fixtures for reusable logic	SUGGESTION	Define custom fixtures for common patterns	Create fixture: test.extend({ loginAs: ... })
Cleanup	No cleanup in afterEach() or fixture teardown	SUGGESTION	Clean up state to prevent test pollution	Add afterEach() with logout, clear storage, etc.
POM	Page Object Method returning another POM inline	NIT	Separate method calls for clarity	OK: await page.loginAs() then await page.checkout() (not chained)
Files	Test file > 200 lines	NIT	Break into smaller, focused test suites	Split into auth.e2e.ts, checkout.e2e.ts
Naming	Test name not descriptive (e.g., test('test'))	NIT	Use clear, behavior-driven names	Change test('test') to test('should display error when credentials invalid')
Imports	Unused imports or fixtures	NIT	Clean up imports for readability	Remove unused { expect } if not used

Framework-Specific Checks

Apply additional checks based on detected framework:

React/Next.js

Check	Severity	Details
Using getByRole() for interactive elements	SUGGESTION	React Testing Library best practice
Using getByText() for display elements	SUGGESTION	User-facing text is reliable
Avoid testing implementation (e.g., state, hooks)	BLOCKER	Test behavior, not internals
Using data-testid only as last resort	SUGGESTION	Prefer role/label/text

Angular

Check	Severity	Details
Using getByRole() with proper ARIA labels	SUGGESTION	Ensure components have semantic HTML
Using data-testid for dynamic content	SUGGESTION	Acceptable for generated or complex components
Handling Angular change detection delays	SUGGESTION	Consider waitForLoadState('networkidle')

Vue/Nuxt

Check	Severity	Details
Using data-test attributes for selectors	SUGGESTION	Vue convention over data-testid
Avoiding direct component state testing	BLOCKER	Test UI behavior, not Vue internals

Plain HTML

Check	Severity	Details
Using semantic selectors (button, input, a)	SUGGESTION	Stable and accessible
Using ARIA labels where needed	SUGGESTION	Improves selector reliability

Review Algorithm

1. Parse test file — extract all test blocks and fixture usage
2. Apply checklist — scan for each finding
3. Collect results — group by severity (BLOCKER > SUGGESTION > NIT)
4. Emit report — structured findings with locations and fixes

Output

Emit structured code review results:

{
  "fileReviewed": "tests/checkout.e2e.ts",
  "totalTests": 5,
  "status": "NEEDS_REVISION",
  "findings": [
    {
      "severity": "BLOCKER",
      "category": "Selectors",
      "finding": "XPath selector detected",
      "description": "XPath selectors are brittle and hard to maintain.",
      "location": {
        "line": 25,
        "file": "tests/checkout.e2e.ts"
      },
      "code": "const button = await page.$(\"//button[@type='submit']\");",
      "suggestion": "Replace with getByRole('button', { name: 'Submit' })"
    },
    {
      "severity": "BLOCKER",
      "category": "Security",
      "finding": "Hardcoded credentials",
      "description": "API key should be externalized to environment variable.",
      "location": {
        "line": 42,
        "file": "tests/checkout.e2e.ts"
      },
      "code": "apiKey: 'sk-12345abcde',",
      "suggestion": "Replace with process.env.API_KEY"
    },
    {
      "severity": "SUGGESTION",
      "category": "Assertions",
      "finding": "Missing assertion message",
      "description": "Add descriptive message for debugging.",
      "location": {
        "line": 68,
        "file": "tests/checkout.e2e.ts"
      },
      "code": "expect(total).toBe(49.99);",
      "suggestion": "expect(total).toBe(49.99, 'Order total should be $49.99 after discount')"
    },
    {
      "severity": "NIT",
      "category": "Naming",
      "finding": "Test name not descriptive",
      "description": "Use clear, behavior-driven test names.",
      "location": {
        "line": 15,
        "file": "tests/checkout.e2e.ts"
      },
      "code": "test('test');",
      "suggestion": "test('should apply discount code and reduce total');"
    }
  ],
  "summary": {
    "blockers": 2,
    "suggestions": 1,
    "nits": 1,
    "totalFindings": 4
  },
  "recommendation": "Fix BLOCKER issues before running tests in CI. SUGGESTION items improve maintainability."
}

Also emit human-readable report:

Code Review: tests/checkout.e2e.ts
==================================

🚫 BLOCKERS (2)
─────────────
1. Line 25: XPath selector detected
   Code: const button = await page.$(\"//button[@type='submit']\");
   Fix: Replace with getByRole('button', { name: 'Submit' })

2. Line 42: Hardcoded credentials
   Code: apiKey: 'sk-12345abcde',
   Fix: Replace with process.env.API_KEY

⚠️ SUGGESTIONS (1)
──────────────
3. Line 68: Missing assertion message
   Code: expect(total).toBe(49.99);
   Fix: expect(total).toBe(49.99, 'Order total should be $49.99 after discount')

💡 NITS (1)
──────────
4. Line 15: Test name not descriptive
   Code: test('test');
   Fix: test('should apply discount code and reduce total');

Recommendation: Fix BLOCKER issues before running tests. Review SUGGESTION items for maintainability.

When no issues found:

{
  "fileReviewed": "tests/login.e2e.ts",
  "totalTests": 3,
  "status": "PASSED",
  "findings": [],
  "summary": {
    "blockers": 0,
    "suggestions": 0,
    "nits": 0,
    "totalFindings": 0
  },
  "recommendation": "Code meets review standards. Ready for commit."
}