Mid-Level Automation · Practice 04

Code Review: Find 5 Issues in a Junior's Test

A junior SDET has written this Playwright test for a KiwiSaver enrolment flow. It passes locally. Your job: find at least 5 issues before it goes into the shared test suite.

1 The Scenario

Code Review Exercise

System under test: KiwiSaver enrolment flow — a NZ retirement savings product registration form

Context: A junior SDET on the team wrote this test last week. It passes on their laptop every time. They've asked you to review it before they merge it into the shared CI suite. The team runs tests on every pull request against a staging environment.

Your job: Read the code carefully. List every issue you can find. For each one, explain the problem, the risk it creates, and how you'd fix it. Aim for at least 5 issues — there are 7 in total.

Tip: Read it once end-to-end, then go line by line. Ask yourself: what happens when this runs in CI? What happens when the codebase changes? What happens when this test fails — will you be able to tell why?

2 The Test to Review

tests/kiwisaver-enrolment.spec.ts

import { test } from '@playwright/test';

test('kiwisaver enrolment', async ({ page }) => {
  // Navigate to the enrolment form
  await page.goto('https://kiwisaver.myapp.co.nz/enrol');

  // Fill in email address
  await page.click('div.form-wrapper > div:nth-child(2) > input');
  await page.fill('div.form-wrapper > div:nth-child(2) > input', 'tane@example.co.nz');

  // Wait for the form to settle
  await page.waitForTimeout(3000);

  // Agree to terms
  await page.click('text=I agree to the Terms and Conditions of KiwiSaver');

  // Submit the form
  await page.click('button:has-text("Enrol Now")');

  // Log the API response for debugging
  const response = await page.waitForResponse('**/api/enrolment**');
  console.log('API response:', await response.json());

  // Fill in IRD number
  await page.fill('[name="irdNumber"]', '123-456-789');
  await page.fill('[name="contributionRate"]', '3');
});

Your task

Before you reveal the model answer, write your own list. For each issue you spot:

Problem: what specifically is wrong
Risk: what goes wrong in practice (flakiness, false positives, security, maintainability)
Fix: the corrected code or approach

Did you find 5? 6? All 7? Reveal the model answer to compare.

Model Answer — 7 Issues Found

1 Hardcoded production URL

Problem: page.goto('https://kiwisaver.myapp.co.nz/enrol') points directly at production. When this runs in CI it will hit real infrastructure — or fail if staging has a different hostname.

Risk: Tests run against production data. Enrolment records could be created. Staging changes won't be tested.

// In playwright.config.ts:
use: { baseURL: process.env.BASE_URL ?? 'https://staging.kiwisaver.myapp.co.nz' }

// In the test:
await page.goto('/enrol');

2 Brittle CSS path selector

Problem: div.form-wrapper > div:nth-child(2) > input breaks the moment a developer reorders form fields or wraps an element in another div. It describes DOM structure, not user intent.

Risk: Test breaks on unrelated frontend refactors. False negatives flood the CI pipeline.

await page.getByLabel('Email address').fill('tane@example.co.nz');

3 Hard-coded sleep (waitForTimeout)

Problem: await page.waitForTimeout(3000) adds 3 seconds regardless of whether the page is ready. On a slow CI runner it may not be enough. On a fast local machine it wastes time.

Risk: Intermittent failures in CI; slow suite overall. The Playwright docs explicitly say not to use this in production test code.

// Wait for the element you actually need next:
await expect(page.getByRole('checkbox', { name: /Terms and Conditions/ })).toBeVisible();

4 Brittle full-text selector for the checkbox

Problem: text=I agree to the Terms and Conditions of KiwiSaver breaks if the copy team changes the label wording by even one word.

Risk: Copy changes are routine. This creates a maintenance burden that has nothing to do with whether the feature works.

await page.getByRole('checkbox', { name: /Terms and Conditions/i }).check();

5 No assertion — test always passes regardless of outcome

Problem: After clicking "Enrol Now", the test ends with no assertion. Whether the enrolment succeeded, failed, or returned an error page, the test reports green.

Risk: A completely broken enrolment flow will never be caught by this test. It gives false confidence.

await page.click('button:has-text("Enrol Now")');
await expect(page.getByText('Enrolment complete')).toBeVisible();

6 console.log left in test code

Problem: console.log('API response:', await response.json()) dumps potentially sensitive enrolment data to CI logs. It also pollutes the test output, making real failures harder to spot.

Risk: PII or test data leaks into build logs. Noisy output slows down debugging.

// Remove entirely. Use Playwright trace viewer for debugging:
// npx playwright test --trace on

7 Hardcoded IRD number in test code

Problem: '123-456-789' is a real-looking IRD number baked into source code. Different test environments may need different values. Rotating test data requires a code change.

Risk: Sensitive-looking data in source control. Inflexible across environments.

await page.fill('[name="irdNumber"]', process.env.TEST_IRD_NUMBER ?? '000-000-000');

Key principle: A test that always passes — regardless of whether the feature works — is worse than no test. It creates false confidence and delays real failures being found. The most critical issue here is Issue 5 (no assertion), because it means the test suite is lying about coverage.

3 Go Further

Rewrite the whole test

Fix all 7 issues and write the corrected version of kiwisaver-enrolment.spec.ts from scratch. Your fixed version should:

Use baseURL from config — no hardcoded hostnames
Use role-based or label-based locators throughout
Replace the sleep with a proper web-first assertion
Assert that enrolment actually completed
Read the IRD number from an environment variable
No console.log

Bonus: Add a test.beforeEach that logs in before the enrolment flow starts, so the test is isolated and doesn't depend on session state from a previous test.

← Practice 03 Practice 05 →