Pull Request Workflows

Why PRs Matter for QA

QA engineers both submit and review pull requests. A well-written PR gets merged faster, reviewed more thoroughly, and causes fewer integration issues. A poorly written PR wastes everyone's time, gets rubber-stamped, and introduces problems that could have been caught in review.

Writing Good PR Descriptions

A PR description should answer three questions: what changed, why it changed, and how to verify it.

The Template

## What
Added retry logic to flaky payment API tests and split the checkout
suite into two shards for parallel execution.

## Why
Payment tests were failing 15% of the time due to third-party API
timeouts. The checkout suite was the bottleneck in our pipeline (12 min).

## How to verify
- Run `npm run test:payment` -- should see retry on timeout, 0% flake rate
- Check pipeline -- checkout stage should complete in ~6 min (down from 12)

## Test evidence
- 10 consecutive green runs on this branch: [link to CI]
- Flake rate before: 15% | after: <1%

## Related
- Fixes #234 (flaky payment tests)
- Part of Epic: Pipeline Speed (#100)

What Makes This Template Effective

• "What" tells the reviewer what to focus on. Without it, they must reverse-engineer your intent from the diff.
• "Why" explains the motivation. "Reduce flake rate from 15% to <1%" is much more compelling than "fix tests."
• "How to verify" gives the reviewer concrete steps. They do not have to guess how to validate the change.
• "Test evidence" shows that you have already validated the change. This builds trust and speeds up approval.
• "Related" links to issues, epics, and discussions for context.

PR Sizing: Small Is Beautiful

Large PRs are reviewed poorly. Research shows that review quality drops significantly after 400 lines of changes. For test code, the threshold is even lower because test logic is harder to follow than application logic.

How to Keep PRs Small

• Split by feature area: Instead of one PR with 20 test files, submit separate PRs for login tests, checkout tests, and payment tests.
• Split by type of change: One PR for new tests, another for refactoring existing tests, a third for infrastructure changes.
• Split by layer: One PR for page objects, another for test specs that use them.
• Use stacked PRs: PR 1 adds the base infrastructure. PR 2 (based on PR 1) adds the first set of tests. PR 3 adds more tests.

When Large PRs Are Unavoidable

Sometimes you must submit a large PR (e.g., migrating a test framework). In these cases:

• Add a detailed PR description explaining the scope
• Use PR comments to guide the reviewer through the changes
• Break the review into sessions: "Please review page-objects/ first, then specs/"
• Consider a walkthrough meeting for PRs over 1000 lines

Commit Message Best Practices

Good commit messages make history searchable and bisect results meaningful.

Format

type(scope): short description

Longer description if needed. Explain why, not what.
The code diff shows what changed; the message should explain why.

Refs: #234

Types for QA Work

Bad vs Good Commit Messages

# Bad
fix tests
update
WIP
changes
asdfasdf

# Good
fix(checkout): stabilize payment test by increasing API timeout to 30s
test(login): add 2FA verification tests for SMS and authenticator app
ci: split browser tests into 4 shards to reduce pipeline from 20m to 6m
refactor(page-objects): extract checkout form into reusable component

The PR Review Process

As a PR Author

1. Self-review first: Read your own diff before requesting review. You will often catch issues yourself.
2. Add context comments: If a section of code is non-obvious, add a PR comment explaining it before the reviewer has to ask.
3. Mark draft PRs as draft: If the PR is not ready for review, use GitHub's draft PR feature so reviewers do not waste time.
4. Respond to feedback promptly: A PR that sits for days after review feedback stalls the team.
5. Do not take feedback personally: The reviewer is critiquing the code, not you.

As a PR Reviewer

1. Understand the context: Read the PR description and linked issues before looking at the code.
2. Review the tests: Are assertions meaningful? Is the test deterministic? Does it clean up after itself?
3. Check for edge cases: What happens with empty inputs, null values, network errors?
4. Look for hardcoded values: Are URLs, timeouts, and credentials configurable?
5. Verify naming: Can you understand what each test does from its name alone?

PR Workflow Automation

Required Checks

Configure your repository to require specific checks before merging:

Branch protection rules for main:
  ✅ Require pull request reviews (1 approval minimum)
  ✅ Require status checks (unit-tests, integration-tests, lint)
  ✅ Require branches to be up to date before merging
  ✅ Require conversation resolution before merging

Auto-Assignment

Use CODEOWNERS to automatically assign reviewers based on changed files:

# .github/CODEOWNERS
tests/             @qa-team
playwright.config.ts @qa-team
.github/workflows/ @qa-team @devops-team

PR Templates

Create a .github/pull_request_template.md to standardize PR descriptions:

## What
<!-- What changed? -->

## Why
<!-- Why was this change needed? -->

## How to verify
<!-- Steps to verify the change works -->

## Checklist
- [ ] Tests pass locally
- [ ] No hardcoded credentials or URLs
- [ ] Test names are descriptive
- [ ] PR is reasonably sized (< 400 lines)

Hands-On Exercise

1. Write a PR description for a recent test change you made using the template above
2. Review one of your old PRs. Would the template have improved it?
3. Set up a CODEOWNERS file for your test directories
4. Create a PR template in .github/pull_request_template.md
5. Practice giving feedback on a colleague's test PR using the reviewer checklist
6. Find a PR in your repository over 500 lines. How could it have been split?