DEV Community

Emery Lin
Emery Lin

Posted on

Don't Merge the First Green Check: Hooks, Fixtures, and a Flake Budget

A green check is a temperature reading. It is not permission to merge.

You merge when three mechanical gates have already fired: a hook refused undeclared fixtures, each test wrote into an isolated directory, and a flake budget was spent in the open. A model can comment after that path. It should not be the gate.

The rest of this article is a proposed workflow you can paste into a throwaway repo. None of the files below claim a flake-rate improvement. They only make the merge rule visible.

Why "retry until green" is a merge bug

Retries without a cap convert a race into a shippable artifact. You see green. The next pull request sees the same race on a colder runner. Shared fixtures amplify it. One test rewrites tests/fixtures/users.json. The next test reads a slightly newer file. CI then lies in both directions.

Agent-authored tests make the pattern common. They add snapshots. They add time.sleep. They skip with no ticket. Your merge path has to assume that input even when the author is human.

You will implement four numbered stops. Skip one and the others get weaker.

1. Install a pre-push hook that CI also runs

A hook that exists only on your laptop is a suggestion. Commit the script. Call it from GitHub Actions so a teammate who skipped git config core.hooksPath still fails the same way.

Create .githooks/pre-push:

#!/usr/bin/env bash
# Proposed local+CI gate. Fail closed on undeclared fixtures and silent skips.
set -euo pipefail

base="${MERGE_BASE:-origin/HEAD}"
if ! git rev-parse --verify "$base" >/dev/null 2>&1; then
  base="HEAD"
fi

changed=$(git diff --name-only "$base"...HEAD)
manifest="tests/fixtures/MANIFEST.txt"
fail=0

while IFS= read -r f; do
  [ -z "$f" ] && continue
  case "$f" in
    tests/fixtures/*)
      if [ ! -f "$manifest" ] || ! grep -Fxq "$f" "$manifest"; then
        echo "unmanifested fixture: $f"
        fail=1
      fi
      ;;
  esac
done <<< "$changed"

# Snapshot-only diffs rot golden files. Require a non-snap source change.
snaps=$(echo "$changed" | grep -E '\.(snap|golden)$' || true)
sources=$(echo "$changed" | grep -Ev '\.(snap|golden|md)$' || true)
if [ -n "$snaps" ] && [ -z "$sources" ]; then
  echo "snapshot-only diff; add a production or test change"
  fail=1
fi

# New skips must name a ticket.
if git diff "$base"...HEAD -- '*.py' | grep -E '^\+.*pytest\.mark\.skip' | grep -vq 'TICKET='; then
  echo "new pytest.mark.skip without TICKET= in the reason"
  fail=1
fi

exit "$fail"
Enter fullscreen mode Exit fullscreen mode

Point Git at the directory, then prove the script is executable:

git config core.hooksPath .githooks
chmod +x .githooks/pre-push
printf 'tests/fixtures/users.json\n' > tests/fixtures/MANIFEST.txt
Enter fullscreen mode Exit fullscreen mode

In CI you run the same file. The job name can stay honest.

# .github/workflows/merge-path.yml (excerpt)
jobs:
  hook:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - name: Run the in-repo hook
        env:
          MERGE_BASE: origin/${{ github.base_ref || 'main' }}
        run: bash .githooks/pre-push
Enter fullscreen mode Exit fullscreen mode

Yes, a pre-push script inside Actions is a misnomer. That is the point. You want one implementation and two call sites.

The hook rejects three classes of diff:

  1. Fixture files not listed in tests/fixtures/MANIFEST.txt.
  2. New pytest.mark.skip without a TICKET= token in the reason string.
  3. Diffs that only change *.snap / *.golden files.

The third rule is harsh. Keep it. Snapshot-only pull requests are how golden files rot.

2. Isolate fixtures per test, not per module

Module-scoped temp directories feel faster. They also leak. You want function-scoped isolation for anything an agent might generate, because agents reuse filenames.

# tests/conftest.py
from __future__ import annotations

from pathlib import Path

import pytest


@pytest.fixture
def isolated_tmp(tmp_path: Path) -> Path:
    """Function-scoped work dir. Do not promote this to module scope."""
    work = tmp_path / "work"
    work.mkdir()
    return work


@pytest.fixture
def fixture_file(isolated_tmp: Path) -> Path:
    path = isolated_tmp / "users.json"
    path.write_text('{"users": []}\n', encoding="utf-8")
    return path
Enter fullscreen mode Exit fullscreen mode

Add a guard test so the policy is not a comment in CONTRIBUTING.md:

# tests/test_fixture_policy.py
from pathlib import Path

REPO = Path(__file__).resolve().parents[1]
SHARED_FIXTURE_ROOT = REPO / "tests" / "fixtures"


def test_tests_do_not_write_shared_fixtures(tmp_path, monkeypatch):
    monkeypatch.chdir(tmp_path)
    before = {
        p: p.stat().st_mtime_ns
        for p in SHARED_FIXTURE_ROOT.glob("**/*")
        if p.is_file()
    }
    # Importing the suite should not mutate golden files.
    assert SHARED_FIXTURE_ROOT.exists()
    after = {
        p: p.stat().st_mtime_ns
        for p in SHARED_FIXTURE_ROOT.glob("**/*")
        if p.is_file()
    }
    assert before == after
Enter fullscreen mode Exit fullscreen mode

If that test fails, you do not have a flake. You have shared state. Fix the fixture before you touch retry counts.

3. Spend a flake budget in public

Infinite pytest --looponfail in CI is not stability. Publish a budget file that expires. When the date passes, quarantined tests become hard failures again.

{
  "max_reruns_per_test": 1,
  "max_flaky_tests_per_sha": 2,
  "quarantine": [
    "tests/test_search.py::test_eventual_consistency"
  ],
  "quarantine_expires_utc": "2026-09-21T00:00:00Z",
  "unknown_flake_policy": "fail_closed"
}
Enter fullscreen mode Exit fullscreen mode

Keep the accountant small. Do not classify failures by scraping log prose. Count pytest's own JSONL report, then apply the budget.

# scripts/flake_account.py
# Proposed accountant. Label every branch you have not executed.
from __future__ import annotations

import json
import sys
from datetime import datetime, timezone
from pathlib import Path


def load_budget(path: Path) -> dict:
    return json.loads(path.read_text(encoding="utf-8"))


def outcomes(report_path: Path) -> dict[str, str]:
    found: dict[str, str] = {}
    for line in report_path.read_text(encoding="utf-8").splitlines():
        if not line.strip():
            continue
        row = json.loads(line)
        nodeid = row.get("$nodeid") or row.get("nodeid")
        outcome = row.get("outcome")
        if nodeid and outcome:
            found[nodeid] = outcome
    return found


def main() -> int:
    budget = load_budget(Path("flake-budget.json"))
    expires = datetime.fromisoformat(
        budget["quarantine_expires_utc"].replace("Z", "+00:00")
    )
    now = datetime.now(timezone.utc)
    results = outcomes(Path("pytest-report.jsonl"))

    rerun_like = [k for k, v in results.items() if v in {"flaky", "rerun"}]
    failed = [k for k, v in results.items() if v == "failed"]
    quarantine = set(budget["quarantine"])

    if now >= expires and quarantine:
        print("quarantine expired; failing closed")
        return 2

    unknown = [n for n in rerun_like if n not in quarantine]
    if unknown and budget["unknown_flake_policy"] == "fail_closed":
        print("unknown flakes:", *unknown, sep="\n")
        return 3

    if len(rerun_like) > budget["max_flaky_tests_per_sha"]:
        print("flake budget exceeded:", len(rerun_like))
        return 4

    product_bugs = [n for n in failed if n not in quarantine]
    if product_bugs:
        print("product failures:", *product_bugs, sep="\n")
        return 5

    print("budget ok")
    return 0


if __name__ == "__main__":
    sys.exit(main())
Enter fullscreen mode Exit fullscreen mode

Wire pytest so the report exists before the accountant runs:

pip install pytest
pytest -q --report-log=pytest-report.jsonl
python scripts/flake_account.py
Enter fullscreen mode Exit fullscreen mode

If your pytest version has no --report-log, write a 20-line pytest_runtest_logreport plugin instead. Do not parse ANSI logs. String matching on FAILED is how you invent flakes that never happened.

Cap reruns at the budget, not at comfort:

pip install pytest-rerunfailures
pytest -q --reruns 1 --reruns-delay 0 --report-log=pytest-report.jsonl
Enter fullscreen mode Exit fullscreen mode

--reruns 1 is a budget. --reruns 9 is a coin flip you scheduled.

4. Make merge depend on the budget, not on a model

Put the jobs in order. The review job is allowed to exist. It is not allowed to be required.

name: green-to-merge
on:
  pull_request:

jobs:
  hook:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - run: bash .githooks/pre-push
        env:
          MERGE_BASE: origin/${{ github.base_ref }}

  fixtures:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-python@v5
        with:
          python-version: "3.12"
      - run: pip install pytest
      - run: pytest -q tests/test_fixture_policy.py

  tests:
    needs: [hook, fixtures]
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-python@v5
        with:
          python-version: "3.12"
      - run: pip install pytest pytest-rerunfailures
      - run: pytest -q --reruns 1 --report-log=pytest-report.jsonl
      - run: python scripts/flake_account.py

  optional-review:
    needs: [tests]
    if: success()
    runs-on: ubuntu-latest
    continue-on-error: true
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - name: Non-blocking diff review
        run: |
          git diff origin/${{ github.base_ref }}...HEAD > /tmp/pr.diff
          echo "Send /tmp/pr.diff to your coding assistant here"
          echo "This step must not be a required check"
Enter fullscreen mode Exit fullscreen mode

Protect the branch with hook, fixtures, and tests only. Leave optional-review off the required list. If the review host is slow, merge still proceeds. That is the design, not a compromise.

This is the only place a coding assistant belongs on this path. After the budget is honest, you can send the already-green diff to a review job. Disclosure: This article was prepared as part of MonkeyCode's product outreach. If you use MonkeyCode, its free model access and free server option can host that optional step so CI minutes stay on tests and hooks. The product is not the merge gate. Remove the job tomorrow and the three required checks still mean the same thing.

Keep the prompt boring. Ask for leaked fixtures, unmarked skips, and new sleeps. Do not ask the model whether the suite is "good enough to merge." That question is what flake_account.py already answered.

Green-to-merge decision table

Use the table as the branch rule. If a row is ambiguous, you failed closed too late.

Hook Fixture guard Tests Unknown flakes Quarantine expired Required result Optional review
fail n/a n/a n/a n/a block do not run
pass fail n/a n/a n/a block do not run
pass pass product fail no no block do not run
pass pass pass with unknown flake yes no block do not run
pass pass pass, quarantined flake no yes block do not run
pass pass pass, under budget no no allow merge may comment

Print the row in the tests job summary. Humans argue less with a table than with a red X.

What this path will not do

The hook is stringly typed. A skip reason that says TICKET=none will satisfy grep and still tell you nothing. Tighten the regex when you have a real ticket prefix.

The quarantine expiry is a date in a JSON file. It is not an owner, a calendar event, or a Slack reminder. When the date passes, CI goes red. Someone has to delete the line or fix the test.

--report-log formats differ across pytest versions. Pin the version in CI. If the accountant cannot parse the file, fail closed. A silent budget ok on an empty report is a merge bug.

Model review is non-deterministic. That is why it is continue-on-error: true and why it is not required. Do not promote it to a required check because a comment looked thorough.

Who should not use this

Do not install this path if you have no suite yet. A hook that blocks undeclared fixtures will not save a repository that never asserts.

Do not use it as a security review. Secret scanning belongs in a dedicated required job with a deterministic scanner. A free model comment is not that scanner.

Do not force a repo-wide MANIFEST.txt onto a monorepo that already isolates packages. Scope the manifest per package, or the hook becomes noise and people start pushing with --no-verify.

Skip the optional review job entirely if your diffs are large generated files. You will spend free-tier tokens describing lockfiles. The three required gates still work.

A small drill you can run today

You do not need a production app to see the gates move. Copy the files. Then break them on purpose.

  1. Add tests/fixtures/orphan.json without listing it in the manifest. Push. The hook must fail.
  2. Add pytest.mark.skip(reason="flaky") with no TICKET=. The hook must fail again.
  3. Write a test that updates a file under tests/fixtures/ during collection. The fixture guard must fail.
  4. Mark a passing test as quarantined and set quarantine_expires_utc to yesterday. The accountant must fail closed.
  5. Only after those four are red, restore the files and confirm tests is green. Then, and only then, turn on optional-review.

If step 5 is the only step you run, you are back to merging the first green check.

Wire any optional review to the post-green job. Keep the hook and the budget even if you never add a model.

Top comments (0)