Skip to content

Instantly share code, notes, and snippets.

@NicolasMassart
Created June 5, 2025 13:18
Show Gist options
  • Select an option

  • Save NicolasMassart/b280ace47fc35fc32bd300743340c202 to your computer and use it in GitHub Desktop.

Select an option

Save NicolasMassart/b280ace47fc35fc32bd300743340c202 to your computer and use it in GitHub Desktop.

Unit Testing: A Guide for PR Reviewers and Authors

A full reference file derived from MetaMask’s internal slides to teach and assist LLMs in writing and reviewing high-quality unit tests. This document contains philosophy, structure, examples, glossary, and anti-patterns in a format compatible with llmstxt.org standards.

Project Goals

Enable LLMs and IDE agents to:

  • Suggest correct and maintainable unit test patterns
  • Detect common mistakes in code under review
  • Promote test-first development strategies
  • Understand nuanced code review scenarios

Key Concepts

Ensure Code Reliability

  • Validate logic under both normal and edge cases
  • Prevent regressions by confirming behavior consistency
  • Detect issues in dependencies and architecture via mocking needs
  • Protect critical features from silent failure
  • Encourage writing clear, defensive code

Example Practices:

// Boolean logic
expect(validateInput(true)).toBe(true);
expect(validateInput(false)).toBe(false);

// Parameterized test
['A', 'B', 'C'].forEach(letter => {
  expect(isAllowed(letter)).toBe(true);
});

// Exception check
expect(() => doSomethingBad()).toThrow('Invalid');

Snapshot usage

// Wrong: snapshot as correctness check
expect(wrapper).toMatchSnapshot();

// Correct: check specific behavior
expect(screen.getByText('Retry')).toBeInTheDocument();

Determinism tips

  • Avoid hardcoded values (e.g., fixed dates)
  • Use dynamic timestamps or mock time libraries
  • Avoid global state and external systems

Naming examples

  • 🚫 "Returns false if date > expiration"
  • ✅ "indicates expired milk"

Brittle & Polluting Tests

Symptoms

  • Breaks from style or formatting changes
  • Relies on internal state or environment
  • Fails on timezone, locale, or date changes

Fixes

// Don’t assert internal state
// Bad
expect(component.state.step).toBe(3);

// Good
expect(screen.getByTestId('step-indicator')).toHaveTextContent('Step 3');
  • Mock randomness, clocks, APIs
  • Reset shared state between tests

Snapshot Guidelines

  • Use for long object diffs, not UI correctness
  • Always review snapshot changes in PR
  • Don’t use snapshots as test coverage tools

Refactoring with Confidence

  • Unit tests provide a safety net during migrations or cleanups
  • Ensure behavior is preserved while structure evolves
  • Tests expose unintended side effects
// Example: JS to TS migration
// Bad: code relies on loose types, breaks downstream
// Test catches misused arguments or untyped assumptions

Collaboration & Code Review Enablement

  • Tests ensure minimum bar before PR review
  • Reviewers should verify tests on first pass
  • Unit tests express expected behavior explicitly
  • Help onboard new devs: tests double as documentation

Example Naming Guidance

  • 🚫 should return null if error
  • does not render form when submission fails

The AAA Pattern

Arrange – Act – Assert is the standard structure for clean unit tests

Breakdown

  • Arrange: setup data, mocks, environment
  • Act: trigger the behavior
  • Assert: verify expectations
// Arrange
const input = createTestInput();

// Act
const output = process(input);

// Assert
expect(output).toMatchObject(expected);

Best Practices

  • One responsibility per test
  • Blank lines between AAA phases
  • Descriptive test titles

Common Pitfalls

  • Assertions inside Act block
  • Missing Arrange phase
  • Too much setup or multiple actions

PR Reviewer Checklist

  • ✅ All code paths tested
  • ✅ Edge cases included
  • ✅ AAA pattern applied
  • ✅ Tests readable and maintainable
  • ✅ Mocks used only when justified
  • ✅ No shared state between tests
  • ✅ Snapshot updates are reviewed
  • ✅ CI green

Common Mistakes & Fixes

❌ Mistake ✅ Better Practice
Test controller indirectly via app Test controller in isolation
Use toBeDefined on nullable queries Use .toBeOnTheScreen()
Test snapshot as validation Check specific output
Snapshot changed but not reviewed Always review diffs
Aim for 100% coverage Aim for meaningful behavior coverage

Additional Developer Resources

Glossary

Term Meaning
SuT System under Test
AAA Arrange-Act-Assert
Snapshot Saved UI/output to detect change
CDD Contract-Driven Development
TDD Test-Driven Development
Mock Simulated object to isolate SuT
Regression A previously fixed behavior breaks again
Coverage Code executed by test, not same as correctness

TL;DR

  • Write unit tests to verify behavior, not implementation
  • Use AAA for clarity and consistency
  • Avoid brittle or over-mocked code
  • Review tests as thoroughly as production code
  • Coverage is helpful, not sufficient

Attribution

Created from: "Unit Testing, A Guide for PR Reviewers"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment