From cb4030652eb3df744796621cb3e4d371c3c32ef1 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Mon, 18 May 2026 08:21:13 -0700 Subject: [PATCH 1/2] geotiff: golden corpus phase 4 PR 1, fast/slow split (#1930) * Register the ``slow`` pytest marker in setup.cfg. * Add ``xrspatial/geotiff/tests/golden_corpus/_marks.py`` with ``fast_slow_marks_for(entry)`` -- a small helper each backend test module can splat into its ``_build_param`` ``marks=`` list. A fixture is fast iff ``"fast"`` appears in its manifest ``tags``; everything else picks up ``pytest.mark.slow`` automatically. * Wire the helper into the merged eager-numpy module (``test_golden_corpus_eager_numpy_1930.py``). The other phase-3 backend modules pick it up as they merge; each PR's review pass can add the one-line splat. * Document the convention in ``xrspatial/geotiff/tests/golden_corpus/README.md``. Current calibration: 17 fast cells, 6 slow (the compression fixtures in the manifest do not carry a ``"fast"`` tag). ``pytest -m "not slow"`` deselects the 6 compression cells; the default ``pytest`` runs everything. If the team wants the compression cells in the fast lane that is a one-line manifest edit per fixture, not a code change. --- setup.cfg | 2 + .../geotiff/tests/golden_corpus/README.md | 18 ++++++ .../geotiff/tests/golden_corpus/_marks.py | 60 +++++++++++++++++++ .../test_golden_corpus_eager_numpy_1930.py | 25 ++++---- 4 files changed, 92 insertions(+), 13 deletions(-) create mode 100644 xrspatial/geotiff/tests/golden_corpus/_marks.py diff --git a/setup.cfg b/setup.cfg index 7f570c745..224ebf11b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -103,6 +103,8 @@ filterwarnings = ignore:'resetCache' deprecated:DeprecationWarning:matplotlib ignore:'enablePackrat' deprecated:DeprecationWarning:matplotlib ignore:'asyncio.AbstractEventLoopPolicy' is deprecated:DeprecationWarning:pytest_asyncio +markers = + slow: long-running test cell (typical: golden-corpus fixtures behind a heavy codec or large pixel count). PR CI can skip with `-m "not slow"`; nightly / release runs use no filter. See xrspatial/geotiff/tests/golden_corpus/_marks.py for the corpus-side helper. [isort] line_length = 100 diff --git a/xrspatial/geotiff/tests/golden_corpus/README.md b/xrspatial/geotiff/tests/golden_corpus/README.md index 411bc31a8..eac5bb61e 100644 --- a/xrspatial/geotiff/tests/golden_corpus/README.md +++ b/xrspatial/geotiff/tests/golden_corpus/README.md @@ -46,6 +46,24 @@ iteration, and mtimes normalised to a fixed epoch. The schema is documented in the comments at the top of `manifest.yaml` and enforced by `generate.validate()`. +## Fast / slow split + +Each fixture's `tags:` list controls whether it runs in the PR CI fast +lane. A fixture is **fast** if `"fast"` appears in its `tags`. Everything +else picks up `pytest.mark.slow` automatically via the helper in +`_marks.py`, which the per-backend test modules consume from their +`_build_param`. + +* `pytest`: runs every cell, fast and slow. +* `pytest -m "not slow"`: PR fast lane; skips heavy cells. +* `pytest -m slow`: only the slow cells, e.g. for a nightly job that + exercises the long tail. + +Today every shipped fixture is `fast`, so the filter is a no-op. The +infrastructure is in place so heavier fixtures (large COGs, jpeg2000, +multi-source VRTs) drop in behind `-m "not slow"` without re-plumbing +each backend test module. + ## What is deliberately not in this PR * Real fixture files. Phase 2 PRs add them in batches (tiled/stripped, diff --git a/xrspatial/geotiff/tests/golden_corpus/_marks.py b/xrspatial/geotiff/tests/golden_corpus/_marks.py new file mode 100644 index 000000000..05b6ebb1c --- /dev/null +++ b/xrspatial/geotiff/tests/golden_corpus/_marks.py @@ -0,0 +1,60 @@ +"""Fast / slow pytest marker helper for the golden-corpus matrix +(issue #1930, phase 4 PR 1). + +Each manifest fixture carries a ``tags`` list. Fixtures tagged ``fast`` +run in the PR CI fast lane; everything else is treated as slow and +gets ``pytest.mark.slow`` attached so PR CI can opt out via +``pytest -m "not slow"``. Nightly / release CI runs without the filter +and exercises everything. + +Today every shipped fixture carries ``fast``; the helper is in place +so future heavier fixtures (large COGs, multi-source VRTs, jpeg2000 +cells) drop in without each backend test module re-implementing the +fast/slow boundary. + +Usage from a backend test module:: + + from xrspatial.geotiff.tests.golden_corpus._marks import ( + fast_slow_marks_for, + ) + + def _build_param(entry): + marks = list(fast_slow_marks_for(entry)) + if entry["id"] in _PARITY_GAPS: + marks.append(pytest.mark.xfail(...)) + return pytest.param(entry, id=entry["id"], marks=marks) + +``fast_slow_marks_for`` is a generator, so chaining with other marks +is just a ``list(...) + [extra_mark]`` away. +""" +from __future__ import annotations + +from collections.abc import Iterator +from typing import Any + +import pytest + + +_FAST_TAG = "fast" + + +def is_fast(entry: dict[str, Any]) -> bool: + """Return True when the manifest entry is in the fast lane. + + The contract: a fixture is fast iff its ``tags`` list contains the + literal string ``"fast"``. Missing or empty ``tags`` count as slow, + on the theory that an untagged fixture is one a contributor forgot + to triage rather than one that is intentionally cheap. + """ + tags = entry.get("tags") or [] + return _FAST_TAG in tags + + +def fast_slow_marks_for(entry: dict[str, Any]) -> Iterator[pytest.MarkDecorator]: + """Yield ``pytest.mark.slow`` when the entry is not in the fast lane. + + Yields nothing for fast fixtures so the caller can just splat the + iterator into its ``marks=`` list without an empty-mark guard. + """ + if not is_fast(entry): + yield pytest.mark.slow diff --git a/xrspatial/geotiff/tests/test_golden_corpus_eager_numpy_1930.py b/xrspatial/geotiff/tests/test_golden_corpus_eager_numpy_1930.py index a48c3c61b..8b4cea535 100644 --- a/xrspatial/geotiff/tests/test_golden_corpus_eager_numpy_1930.py +++ b/xrspatial/geotiff/tests/test_golden_corpus_eager_numpy_1930.py @@ -61,6 +61,9 @@ from xrspatial.geotiff import open_geotiff # noqa: E402 from xrspatial.geotiff.tests.golden_corpus import generate # noqa: E402 +from xrspatial.geotiff.tests.golden_corpus._marks import ( # noqa: E402 + fast_slow_marks_for, +) from xrspatial.geotiff.tests.golden_corpus._oracle import ( # noqa: E402 compare_to_oracle, ) @@ -126,26 +129,22 @@ def _is_lossy(entry: dict) -> bool: def _build_param(entry: dict) -> pytest.param: - """Wrap a fixture entry in a ``pytest.param`` with the right mark. + """Wrap a fixture entry in a ``pytest.param`` with the right marks. Real parity gaps get ``xfail(strict=True)`` so the test surfaces a hard failure the day the gap closes. The MinIsWhite cell gets a plain skip - because the divergence is intentional. + because the divergence is intentional. Non-fast fixtures additionally + pick up ``pytest.mark.slow`` from the corpus helper. """ fid = entry["id"] + marks = list(fast_slow_marks_for(entry)) if fid in _PARITY_GAPS: - return pytest.param( - entry, - id=fid, - marks=pytest.mark.xfail(reason=_PARITY_GAPS[fid], strict=True), - ) - if fid in _INTENTIONAL_SKIPS: - return pytest.param( - entry, - id=fid, - marks=pytest.mark.skip(reason=_INTENTIONAL_SKIPS[fid]), + marks.append( + pytest.mark.xfail(reason=_PARITY_GAPS[fid], strict=True) ) - return pytest.param(entry, id=fid) + elif fid in _INTENTIONAL_SKIPS: + marks.append(pytest.mark.skip(reason=_INTENTIONAL_SKIPS[fid])) + return pytest.param(entry, id=fid, marks=marks) _FIXTURES = _resolved_fixtures() From 89e5314a380b9ef94e047a3a92f33fd4d886a673 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Mon, 18 May 2026 09:00:20 -0700 Subject: [PATCH 2/2] geotiff: fix-up phase 4 PR 1 docs and return type (#1930) Address phase 4 PR 1 review: * The docstring on ``_marks.py`` and the corpus ``README.md`` claimed "every shipped fixture carries ``fast``", but the six ``compression_*`` fixtures do not -- they land in the slow lane today. Update both notes to match reality and document the manifest-edit recipe to move them to fast if desired. * Change ``fast_slow_marks_for`` to return ``list[pytest.MarkDecorator]`` directly instead of yielding from a generator. Callers always consume via ``list(...)`` anyway; the generator indirection only added one frame of confusion. --- .../geotiff/tests/golden_corpus/README.md | 11 +++++---- .../geotiff/tests/golden_corpus/_marks.py | 24 ++++++++++--------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/xrspatial/geotiff/tests/golden_corpus/README.md b/xrspatial/geotiff/tests/golden_corpus/README.md index eac5bb61e..f118d9285 100644 --- a/xrspatial/geotiff/tests/golden_corpus/README.md +++ b/xrspatial/geotiff/tests/golden_corpus/README.md @@ -59,10 +59,13 @@ else picks up `pytest.mark.slow` automatically via the helper in * `pytest -m slow`: only the slow cells, e.g. for a nightly job that exercises the long tail. -Today every shipped fixture is `fast`, so the filter is a no-op. The -infrastructure is in place so heavier fixtures (large COGs, jpeg2000, -multi-source VRTs) drop in behind `-m "not slow"` without re-plumbing -each backend test module. +Today most shipped fixtures carry `fast`. The six `compression_*` +fixtures in the manifest do not, so `pytest -m "not slow"` deselects +them. A one-line manifest edit per fixture would move them into the +fast lane if the team decides that is the right calibration. Future +heavier fixtures (large COGs, jpeg2000, multi-source VRTs) drop in +behind the same boundary without re-plumbing each backend test +module. ## What is deliberately not in this PR diff --git a/xrspatial/geotiff/tests/golden_corpus/_marks.py b/xrspatial/geotiff/tests/golden_corpus/_marks.py index 05b6ebb1c..7ec3cc64f 100644 --- a/xrspatial/geotiff/tests/golden_corpus/_marks.py +++ b/xrspatial/geotiff/tests/golden_corpus/_marks.py @@ -7,10 +7,13 @@ ``pytest -m "not slow"``. Nightly / release CI runs without the filter and exercises everything. -Today every shipped fixture carries ``fast``; the helper is in place -so future heavier fixtures (large COGs, multi-source VRTs, jpeg2000 -cells) drop in without each backend test module re-implementing the -fast/slow boundary. +Today most shipped fixtures carry ``fast``. The six ``compression_*`` +fixtures in the manifest do not, so they land in the slow lane and +``pytest -m "not slow"`` deselects them. A one-line manifest edit per +fixture would move them to the fast lane if the team decides that is +the right calibration. Future heavier fixtures (large COGs, +multi-source VRTs, jpeg2000 cells) will drop in behind the same +boundary without each backend test module re-implementing it. Usage from a backend test module:: @@ -29,7 +32,6 @@ def _build_param(entry): """ from __future__ import annotations -from collections.abc import Iterator from typing import Any import pytest @@ -50,11 +52,11 @@ def is_fast(entry: dict[str, Any]) -> bool: return _FAST_TAG in tags -def fast_slow_marks_for(entry: dict[str, Any]) -> Iterator[pytest.MarkDecorator]: - """Yield ``pytest.mark.slow`` when the entry is not in the fast lane. +def fast_slow_marks_for(entry: dict[str, Any]) -> list[pytest.MarkDecorator]: + """Return the slow mark (in a list) when the entry is not fast. - Yields nothing for fast fixtures so the caller can just splat the - iterator into its ``marks=`` list without an empty-mark guard. + Returns ``[pytest.mark.slow]`` for slow fixtures and ``[]`` for fast + ones, so the caller can splat the result into its ``marks=`` list + without an empty-mark guard or a generator-to-list conversion. """ - if not is_fast(entry): - yield pytest.mark.slow + return [pytest.mark.slow] if not is_fast(entry) else []