diff --git a/conftest.py b/conftest.py new file mode 100644 index 0000000..597314b --- /dev/null +++ b/conftest.py @@ -0,0 +1,43 @@ +# -*- coding: utf-8 -*- +# Copyright © 2026 SparkMeter, Inc. +# All Rights Reserved. +"""Top level pytest configuration. + +This file exists at the top of the checkout, rather than alongside the tests +in sparkmeter/conftest.py, because pytest only calls pytest_addoption for +*initial* conftests -- the ones found by walking down to the invocation's +anchor directory before the command line is parsed. With no path argument the +anchor is the rootdir, so a conftest.py one directory further down is loaded +too late for its options to exist: `pytest --regenerate-snapshots` would be +rejected as an unrecognized argument. +""" + +# pytest's own `pytester` fixture, used by the tests that run pytest on a +# throwaway test to check this file's wiring end to end. `pytest_plugins` is +# only honored in the top level conftest, which is here. +pytest_plugins = ["pytester"] + + +def pytest_addoption(parser): + """Register our own command line options.""" + parser.addoption( + "--regenerate-snapshots", + action="store_true", + default=False, + help=( + "Rewrite the .page snapshot of any content test whose rendered " + "output differs, instead of failing the test." + ), + ) + + +def pytest_configure(config): + """Propagate command line options to the code that needs them. + + ContentTester is a plain class with no fixture access, so the option is + handed to it as a module level flag. This runs once per process, which + covers each xdist worker as well as a plain single-process run. + """ + from sparkmeter.web import unittestutils + + unittestutils.regenerate_snapshots = config.getoption("--regenerate-snapshots") diff --git a/deploy/Dockerfile b/deploy/Dockerfile index 733e0e9..59e3597 100755 --- a/deploy/Dockerfile +++ b/deploy/Dockerfile @@ -169,6 +169,12 @@ USER spark COPY --chown=spark:spark setup.cfg setup.cfg +# The top level conftest.py, which registers the suite's own command line +# options. It has to be at the top of the tree, next to setup.cfg: pytest only +# calls pytest_addoption for initial conftests, and with no path argument the +# rootdir is the only directory searched for one. +COPY --chown=spark:spark conftest.py conftest.py + # Test fixtures: the committed snapshot baseline that the snapshot tests # (verify()) compare generated output against. Without it the tests find # the expected files absent and write them fresh every run instead of diff --git a/regenerate_snapshots.sh b/regenerate_snapshots.sh new file mode 100755 index 0000000..bb0c286 --- /dev/null +++ b/regenerate_snapshots.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# +# Regenerate the .page snapshots under test-data/ from the current sources. +# +# Run from the top level of the checkout to regenerate -- the main checkout or +# any worktree: +# +# ./regenerate_snapshots.sh +# +# The suite runs with --regenerate-snapshots, which makes a content test whose +# rendered output differs rewrite its .page file instead of failing; see +# ContentTester.verify in sparkmeter/web/unittestutils.py. Matching snapshots +# are not touched, so the diff is the set of pages the source change affected. +# Orphans are not pruned: the .page file of a renamed or deleted test is left +# in place and is not reported, so finding those is a manual step. +# +set -euo pipefail + +# The image build derives the version from .git via hatch-vcs. In a worktree +# .git is a file pointing elsewhere rather than a directory, so that fails and +# SETUPTOOLS_SCM_PRETEND_VERSION supplies the version instead. Built as its own +# step because `docker compose run --build` cannot forward --build-arg. +docker compose -f docker-compose.test.yml build \ + --build-arg SETUPTOOLS_SCM_PRETEND_VERSION="0.0.0+g$(git rev-parse --short HEAD)" \ + test + +# Compose namespaces its own containers by project, but this one is created +# with an explicit --name, so it needs the checkout in its name too. Otherwise +# the pre-run `docker rm -f` below would remove a container another checkout is +# using: docker-compose.test.yml is written so two checkouts can run their +# suites concurrently. +container="$(basename "$PWD")-snapshot-regen" + +docker rm -f "$container" >/dev/null 2>&1 || true + +# The run's status is captured rather than allowed to abort the script: a test +# failing for a reason unrelated to snapshots would otherwise end the run +# before the copy back, discarding every snapshot it had already rewritten. +status=0 +docker compose -f docker-compose.test.yml run --name "$container" test \ + uv run pytest -n auto --regenerate-snapshots || status=$? + +# The test image COPYs the source in rather than bind-mounting the working +# tree, so the rewritten files exist only inside the container -- hence the +# name instead of --rm, and this copy back. +if ! docker cp "$container:/app/test-data/." ./test-data/; then + echo "Failed to copy the regenerated snapshots out of the container." >&2 + echo "Leaving the container in place so the run can still be retrieved:" >&2 + echo " docker cp $container:/app/test-data/. ./test-data/" >&2 + exit 1 +fi + +docker rm -f "$container" >/dev/null || true + +exit "$status" diff --git a/scripts/run_coverage.sh b/scripts/run_coverage.sh index 2d27801..0c822d1 100644 --- a/scripts/run_coverage.sh +++ b/scripts/run_coverage.sh @@ -24,9 +24,30 @@ docker compose -f docker-compose.test.yml build \ # pytest-coverage.txt for the PR report comment. All three are produced in the # container, tarred to stdout, and unpacked on the host. pytest's output is teed # to stderr, not stdout, so stdout carries only the tar stream. +# +# pytest's exit status has to be carried out by hand. `pipefail` makes the +# pipeline report pytest's status rather than tee's, it is captured before tar +# runs, and the container exits with it. Otherwise the container's status is +# tar's: an ordinary red suite still failed the job, but only because +# --no-cov-on-fail suppresses .coverage.xml and tar then errors on the missing +# file. A run that writes the reports and still exits non-zero -- pytest's +# exit 5, "no tests collected", from a -k that matches nothing -- left tar +# succeeding and the job green. On the host the status is captured rather than +# allowed to abort, so the script runs to the end and reports it there instead +# of dying mid-script; when coverage did reach the host, the diff-cover report +# below is still produced. +# +# The artifacts are cleared first so that a run which fails before writing them +# cannot leave the previous run's files behind to be read as current -- the gate +# below reads .coverage.xml, and the CI report steps read the other two. +rm -f .coverage.xml junit.xml pytest-coverage.txt + +set +e docker compose -f docker-compose.test.yml run --rm test \ - sh -c 'uv run pytest -n auto --junitxml=/app/junit.xml 2>&1 | tee /app/pytest-coverage.txt 1>&2; tar -c -C /app .coverage.xml junit.xml pytest-coverage.txt' \ + sh -c 'set -o pipefail; uv run pytest -n auto --junitxml=/app/junit.xml 2>&1 | tee /app/pytest-coverage.txt 1>&2; status=$?; tar -c -C /app .coverage.xml junit.xml pytest-coverage.txt; exit $status' \ | tar -x +tests_rc=${PIPESTATUS[0]} +set -e # diff-cover gates the changed lines and always prints a terminal report. It # also writes a markdown report, which is appended to the GitHub Actions job @@ -40,16 +61,35 @@ fi # --fail-under exits non-zero when changed-line coverage is below the threshold. # Capture that instead of aborting so the report still reaches the job summary. -set +e -uvx "diff-cover==10.3.0" .coverage.xml \ - --compare-branch "$compare_branch" \ - --fail-under "$fail_under" \ - --format "$formats" -diff_cover_rc=$? -set -e +# +# A suite that fails writes no coverage at all (--no-cov-on-fail in setup.cfg), +# so there is nothing to gate. Skip it and say so, rather than letting +# diff-cover fail on the missing file with a traceback that reads like a broken +# tool instead of a failing test. The suite's own status is reported below. +diff_cover_rc=0 +if [ -f .coverage.xml ]; then + set +e + uvx "diff-cover==10.3.0" .coverage.xml \ + --compare-branch "$compare_branch" \ + --fail-under "$fail_under" \ + --format "$formats" + diff_cover_rc=$? + set -e +else + # Drop any report from a previous run so the summary below cannot append a + # stale one for a gate that did not run. + rm -f "$report_md" + echo "No coverage was produced, so the coverage gate is skipped." >&2 +fi if [ -n "${GITHUB_STEP_SUMMARY:-}" ] && [ -f "$report_md" ]; then cat "$report_md" >> "$GITHUB_STEP_SUMMARY" fi +# A failing suite fails the job. It is reported ahead of the coverage gate +# because a red run makes the coverage number meaningless. +if [ "$tests_rc" -ne 0 ]; then + exit "$tests_rc" +fi + exit "$diff_cover_rc" diff --git a/sparkmeter/web/tests/test_unittestutils.py b/sparkmeter/web/tests/test_unittestutils.py new file mode 100644 index 0000000..42a7574 --- /dev/null +++ b/sparkmeter/web/tests/test_unittestutils.py @@ -0,0 +1,249 @@ +# -*- coding: utf-8 -*- +# Copyright © 2026 SparkMeter, Inc. +# All Rights Reserved. +"""Unittests for the snapshot comparison in ContentTester.verify().""" + +import os +import warnings + +import pytest + +from sparkmeter.web import unittestutils +from sparkmeter.web.unittestutils import ContentTester, SnapshotRegenerated + +# An mtime far enough in the past that any rewrite moves it. +OLD_MTIME = 1000000000 + + +class SnapshotTester(ContentTester): + """A ContentTester whose snapshot lives at an explicit path. + + ContentTester derives expected_filename from the caller's stack frame and + the test-data tree; these tests need a throwaway file under tmp_path + instead. The frame is still the test method that built this object, so + ``name`` -- which is what the regeneration warning reports -- keeps its + normal shape. + """ + + def __init__(self, path): + """Create a tester reading and writing the snapshot at ``path``.""" + super(SnapshotTester, self).__init__(frame=2) + self.path = str(path) + + @property + def expected_filename(self): + """Get the snapshot path this tester was pointed at.""" + return self.path + + +def write_snapshot(path, content): + """Write a snapshot file and backdate it so a rewrite is detectable.""" + with open(path, "w", encoding="utf-8") as fp: + fp.write(content) + os.utime(path, (OLD_MTIME, OLD_MTIME)) + + +def read_snapshot(path): + """Read a snapshot file back.""" + with open(path, encoding="utf-8") as fp: + return fp.read() + + +def regeneration_warnings(caught): + """Pick the snapshot regeneration warnings out of recorded warnings. + + Unrelated warnings from application imports surface in the same recorder, + so select the dedicated category rather than every warning raised. + """ + return [w for w in caught if issubclass(w.category, SnapshotRegenerated)] + + +class ContentTesterVerifyTest(object): + @pytest.fixture(autouse=True) + def flag_off(self, monkeypatch): + """Keep the module level flag off unless a test turns it on. + + It is process global, so leaking it would change how later tests in + the same worker compare their snapshots. + """ + monkeypatch.setattr(unittestutils, "regenerate_snapshots", False) + + @pytest.fixture() + def regenerating(self, monkeypatch): + """Turn on snapshot regeneration for the duration of a test.""" + monkeypatch.setattr(unittestutils, "regenerate_snapshots", True) + + @pytest.fixture() + def snapshot(self, tmp_path): + """Path of the snapshot file under test.""" + return tmp_path / "example.page" + + def test_matching_content_passes_without_touching_the_file(self, snapshot): + """Flag off, content matches: test passes and the file is untouched.""" + write_snapshot(snapshot, "hello\n") + tester = SnapshotTester(snapshot) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + tester.verify("hello") + + assert regeneration_warnings(caught) == [] + assert read_snapshot(snapshot) == "hello\n" + assert os.stat(snapshot).st_mtime == OLD_MTIME + + def test_differing_content_fails_and_leaves_the_file_alone(self, snapshot): + """Flag off, content differs: the existing failure path runs.""" + write_snapshot(snapshot, "hello\n") + tester = SnapshotTester(snapshot) + + with pytest.raises(pytest.fail.Exception): + tester.verify("goodbye") + + assert read_snapshot(snapshot) == "hello\n" + assert os.stat(snapshot).st_mtime == OLD_MTIME + + def test_differing_content_is_rewritten_when_regenerating(self, snapshot, regenerating): + """Flag on, content differs: the file is rewritten and a warning names its path.""" + write_snapshot(snapshot, "hello\n") + tester = SnapshotTester(snapshot) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + tester.verify("goodbye") + + assert read_snapshot(snapshot) == "goodbye\n" + assert os.stat(snapshot).st_mtime != OLD_MTIME + regenerated = regeneration_warnings(caught) + assert [str(w.message) for w in regenerated] == ["Regenerated snapshot: %s" % (snapshot,)] + + def test_rewriting_leaves_no_temporary_file_behind(self, snapshot, regenerating): + """The rewrite goes through a temporary file, which must not survive it. + + The whole test-data tree is copied out of the container after a + regeneration run, so a leftover temporary would be committed too. + """ + write_snapshot(snapshot, "hello\n") + tester = SnapshotTester(snapshot) + + with warnings.catch_warnings(record=True): + warnings.simplefilter("always") + tester.verify("goodbye") + + assert sorted(os.listdir(os.path.dirname(str(snapshot)))) == ["example.page"] + + def test_matching_content_is_not_rewritten_when_regenerating(self, snapshot, regenerating): + """Flag on, content matches: no rewrite, no warning, no mtime churn.""" + write_snapshot(snapshot, "hello\n") + tester = SnapshotTester(snapshot) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + tester.verify("hello") + + assert regeneration_warnings(caught) == [] + assert read_snapshot(snapshot) == "hello\n" + assert os.stat(snapshot).st_mtime == OLD_MTIME + + @pytest.mark.parametrize("regenerate", [False, True]) + def test_absent_file_is_written_without_warning(self, snapshot, monkeypatch, regenerate): + """Either flag state, no file: it is written and no warning is emitted.""" + monkeypatch.setattr(unittestutils, "regenerate_snapshots", regenerate) + tester = SnapshotTester(snapshot) + assert not os.path.exists(snapshot) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + tester.verify("hello") + + assert regeneration_warnings(caught) == [] + assert read_snapshot(snapshot) == "hello\n" + + def test_snapshot_differing_only_in_leading_whitespace_stays_stale(self, snapshot, regenerating): + """Leading whitespace is stripped from both sides before comparing. + + Nothing on disk is normalized as a result: a snapshot whose only + difference from the rendered content is leading whitespace compares + equal, so it is not rewritten and its stored form survives verbatim. + """ + write_snapshot(snapshot, "\n\n hello\n") + tester = SnapshotTester(snapshot) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + tester.verify(" hello") + + assert regeneration_warnings(caught) == [] + assert read_snapshot(snapshot) == "\n\n hello\n" + assert os.stat(snapshot).st_mtime == OLD_MTIME + + +# --------------------------------------------------------------------------- +# The conftest wiring, exercised by running pytest on a throwaway test +# --------------------------------------------------------------------------- + +# The top level conftest.py, i.e. the one actually shipped. The runs below use +# its real source rather than a restatement of it, so they cover the option's +# registration, its default, and pytest_configure handing it to unittestutils. +ROOT_CONFTEST = os.path.join(os.path.abspath(unittestutils.rootdir), "conftest.py") + +# A test rendering content that differs from its snapshot -- the one case whose +# outcome --regenerate-snapshots changes. +DIFFERING_SNAPSHOT_TEST = ''' +from sparkmeter.web.unittestutils import ContentTester + + +class SnapshotTester(ContentTester): + """A ContentTester whose snapshot lives at an explicit path.""" + + def __init__(self, path): + """Create a tester reading and writing the snapshot at ``path``.""" + super(SnapshotTester, self).__init__(frame=2) + self.path = path + + @property + def expected_filename(self): + """Get the snapshot path this tester was pointed at.""" + return self.path + + +def test_content_differs_from_its_snapshot(): + """Render content that does not match the snapshot on disk.""" + SnapshotTester(%r).verify("goodbye") +''' + + +@pytest.fixture() +def stale_snapshot(pytester, monkeypatch): + """Build a pytest run of one test whose snapshot on disk is out of date. + + Returns the path of that snapshot. The inner runs set the module level + flag exactly as a real run does -- that is the point of them -- so it is + pinned through monkeypatch here and restored when the test ends. + """ + monkeypatch.setattr(unittestutils, "regenerate_snapshots", False) + + with open(ROOT_CONFTEST, encoding="utf-8") as fp: + pytester.makeconftest(fp.read()) + + snapshot = pytester.path / "example.page" + write_snapshot(snapshot, "hello\n") + pytester.makepyfile(test_snapshot=DIFFERING_SNAPSHOT_TEST % (str(snapshot),)) + return snapshot + + +def test_run_without_the_flag_fails_and_keeps_the_snapshot(pytester, stale_snapshot): + """The shipped default is off: a differing snapshot fails and is not touched.""" + result = pytester.runpytest() + + result.assert_outcomes(failed=1) + assert read_snapshot(stale_snapshot) == "hello\n" + assert os.stat(stale_snapshot).st_mtime == OLD_MTIME + + +def test_run_with_the_flag_rewrites_the_snapshot_and_warns(pytester, stale_snapshot): + """With --regenerate-snapshots the same run passes, rewriting the snapshot.""" + result = pytester.runpytest("--regenerate-snapshots") + + result.assert_outcomes(passed=1) + assert read_snapshot(stale_snapshot) == "goodbye\n" + result.stdout.fnmatch_lines(["*SnapshotRegenerated: Regenerated snapshot: %s*" % (stale_snapshot,)]) diff --git a/sparkmeter/web/unittestutils.py b/sparkmeter/web/unittestutils.py index c52dfe3..142ad5d 100644 --- a/sparkmeter/web/unittestutils.py +++ b/sparkmeter/web/unittestutils.py @@ -7,8 +7,11 @@ import difflib import os import re +import stat import sys +import tempfile import urllib.parse +import warnings from builtins import str import html5lib @@ -24,6 +27,46 @@ rootdir = os.path.join(os.path.dirname(__file__), "..", "..") +# When true, ContentTester.verify() rewrites a snapshot whose content differs +# instead of failing the test, and warns naming the rewritten snapshot. Set from +# pytest_configure() in the top level conftest.py when --regenerate-snapshots is +# passed; each xdist worker is its own process and sets it independently. +regenerate_snapshots = False + + +class SnapshotRegenerated(UserWarning): + """Warning issued for each .page snapshot rewritten by --regenerate-snapshots. + + A dedicated category so the warnings are greppable in a run's output and so + tests can select them by category rather than by message text. + """ + + +def rewrite_snapshot(path, content): + """Replace the snapshot at ``path`` with ``content``, atomically. + + A regeneration run rewrites up to one file per content test and the + results are copied out of the container afterwards, so a run interrupted + part way through must not leave a half written .page file behind to be + committed as the new baseline. The content is written to a temporary file + in the same directory (same filesystem, so the rename is atomic), flushed + and closed, then moved onto the snapshot path with os.replace(). + """ + directory = os.path.dirname(path) or "." + fd, tmppath = tempfile.mkstemp(dir=directory, prefix=os.path.basename(path) + ".", suffix=".tmp") + try: + with os.fdopen(fd, "w", encoding="utf-8") as fp: + fp.write(content) + # mkstemp creates the temporary 0600; carry over the permissions of the + # file being replaced so a regenerated snapshot is readable as before. + if os.path.exists(path): + os.chmod(tmppath, stat.S_IMODE(os.stat(path).st_mode)) + os.replace(tmppath, path) + except BaseException: # pragma: nocoverage + if os.path.exists(tmppath): + os.unlink(tmppath) + raise + def validate_html(data, ignores=None): """Validate the html in the request's response.""" @@ -139,6 +182,17 @@ def verify(self, content): expected_content = open(expected_filename, encoding="utf-8").read() expected_content = expected_content.lstrip() + if expected_content == content: + return + + if regenerate_snapshots: + rewrite_snapshot(expected_filename, content) + warnings.warn( + "Regenerated snapshot: {}".format(expected_filename), + SnapshotRegenerated, + ) + return + self._diff_lines( expected_content.split("\n"), content.split("\n"),