Skip to content

geotiff tests: consolidate release-gate registry#2409

Merged
brendancol merged 3 commits into
mainfrom
issue-2403
May 26, 2026
Merged

geotiff tests: consolidate release-gate registry#2409
brendancol merged 3 commits into
mainfrom
issue-2403

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

PR 10 of the GeoTIFF test consolidation epic (#2390). Closes #2403.

What changed

13 test_release_gate_*.py files fold into one registry:

xrspatial/geotiff/tests/release_gates/
├── __init__.py
└── test_stable_features.py

Every test that used to carry @pytest.mark.release_gate -- and every
test that lived in a test_release_gate_*.py file but did not carry
the marker (overview / sidecar metadata, rotated negative cases, the
#2321 cross-cutting meta-gates) -- moves into the single file. The
marker is applied to all 159 tests. After this PR the release engineer
has one audit point:

pytest xrspatial/geotiff/tests/release_gates/ -m release_gate
# or
pytest xrspatial/geotiff/tests/ -m release_gate

Both invocations select the same 159 tests from the single registry
file. No other file in the tree carries the release_gate marker.

Files folded (deleted in the same commit)

  • test_release_gate_2321.py
  • test_release_gate_attrs_contract.py
  • test_release_gate_codec_round_trip_2341.py
  • test_release_gate_codecs.py
  • test_release_gate_cog.py
  • test_release_gate_dask_parity.py
  • test_release_gate_eager_dask_parity_2341.py
  • test_release_gate_local_read.py
  • test_release_gate_local_write.py
  • test_release_gate_negative_2341.py
  • test_release_gate_overview_sidecar_metadata_2341.py
  • test_release_gate_windowed_read.py
  • test_release_gate_windowed_reads_2341.py

Doc update

docs/source/reference/release_gate_geotiff.rst (touched by PR 1 of
the #2345 doc epic) now points at the consolidated module. The
per-feature grouping is preserved in the prose. A handful of references
in docs/source/reference/geotiff.rst move to the new path too.

The cross-cutting meta-gates in the consolidated file parse this same
rst file, so the parity gate test_release_gate_cites_only_existing_test_files
covers the doc update end-to-end.

Audit deliverable

xrspatial/geotiff/tests/CLUSTER_AUDIT_PR10.md maps every old
file::test to its new release_gates/test_stable_features.py::test_id.
Deleted on the final commit before merge per epic protocol.

Verification

$ pytest xrspatial/geotiff/tests/release_gates/ -v -m release_gate
155 passed, 3 xfailed, 1 xpassed

The three xfail cases (PAM sidecar CRS conflict, integer nodata
float-promotion, mixed-tier VRT children) and the single xpass
(uppercase HTTP SSRF) carry over the same dispositions from the source
files; epic-tracked follow-up issues are linked in the section docstring.

Test plan

  • pytest xrspatial/geotiff/tests/release_gates/ -v -m release_gate -- all green
  • pytest xrspatial/geotiff/tests/ -m release_gate -v -- selects the 159 tests in the new file and no others
  • test_release_gate_cites_only_existing_test_files (consolidated meta-gate) passes against the updated rst
  • test_release_contract_parity_2389.py and test_supported_features_tiers_2137.py still pass

Epic: #2390. PR 1 of the epic merged at 04514ad0; _helpers/
already lives on main.

Fold the 13 ``test_release_gate_*.py`` files (and the unmarked tests
that lived alongside them) into a single registry at
``xrspatial/geotiff/tests/release_gates/test_stable_features.py``. The
release engineer now has one audit point: ``pytest -m release_gate``
selects exactly this file and no others.

Every test keeps ``@pytest.mark.release_gate``; tests previously living
in a ``test_release_gate_*.py`` file but missing the marker (overview
sidecar metadata, rotated negative cases, the #2321 meta-gates) pick it
up here. Helper-function name collisions across the source files
(``_write_known_good`` in four files, ``_make_data_array`` in two)
are resolved with section prefixes so the consolidation does not
introduce cross-section coupling.

``docs/source/reference/release_gate_geotiff.rst`` and the smaller
``docs/source/reference/geotiff.rst`` references now point at the
consolidated module. The per-feature grouping in the doc is preserved
in prose even though the underlying file is one.

PR 10 of 11 in epic #2390. Closes #2403.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 26, 2026
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: geotiff tests: consolidate release-gate registry

Tests-only restructure, so the usual domain checks (algorithm accuracy, NaN handling, backend dispatch, dask depth math, GPU kernel correctness) don't apply. Review focus: test-suite mechanics, marker propagation, fixture wiring, doc parity, and what the gate suite does in CI cells without optional deps.

Blockers (must fix before merge)

None. The consolidated meta-gate test_release_gate_cites_only_existing_test_files passes against the updated rst, which is the end-to-end check on the doc move.

Suggestions (should fix, not blocking)

  • xrspatial/geotiff/tests/release_gates/test_stable_features.py:743 and :1400 -- module-level pytest.importorskip("dask") and pytest.importorskip("rasterio") now gate the entire 159-test file. In the original layout only the dask-parity and overview-sidecar files carried these. A CI cell without dask (or without rasterio) used to still run the local-read, local-write, codec, and attrs-contract gates; after this PR it skips every one of them. A release engineer running pytest -m release_gate on a minimal install gets an all-skip result with no signal about whether the pure-numpy promises still hold. Move the skips down to per-test decorators, or wrap the dask / rasterio imports in fixtures that skip only their callers. Same goes for the bare from rasterio.enums import Resampling on line 1403: the importorskip one line up saves it today but the coupling is fragile.

Nits (optional improvements)

  • xrspatial/geotiff/tests/release_gates/test_stable_features.py (the _wsp_corpus_file fixture, used by the windowed-shifted-transform tests) has a leading underscore. Pytest accepts it and indirect parametrize works (test IDs confirm), but the rest of the suite names fixtures without an underscore. Renaming to wsp_corpus_file matches the surrounding style.
  • xrspatial/geotiff/tests/CLUSTER_AUDIT_PR10.md is in the diff; the PR body says it gets deleted on the final commit. Flagging so the deletion doesn't get missed in the review-fixes pass.
  • The xpass on test_release_gate_http_ssrf_rejects_loopback_uppercase_scheme carries over from the original file unchanged. The xfail reason says "Locks in once sub-PR 5 of #2321 (PR #2326) lands"; if #2326 has already merged, flipping the marker off is a follow-up, but it isn't something this PR introduced.

What looks good

  • Helper-name collision strategy is clean. The four _write_known_good and two _make_data_array definitions are scoped per section (_local_read_write_known_good, _dask_parity_write_known_good, _attrs_write_known_good, _overview_make_raster), so the consolidation doesn't introduce cross-section coupling that bites a future edit.
  • The meta-gate's self-reference path (_META_SELF_REF) updated to the new location, and the path-discovery regex xrspatial/geotiff/tests/[\w/]+\.py accepts the slash-bearing path so the cited-test-files gate still runs.
  • Marker coverage went from 134/159 to 159/159. Tests that used to live in a test_release_gate_*.py file but lacked the marker (the seven overview / sidecar metadata tests, the four rotated negative tests, the five #2321 cross-cutting meta-gates) now carry it.
  • Doc citations are propagated, including the small ones in geotiff.rst and the docstring in test_release_contract_parity_2389.py. No dangling references to deleted files.

Checklist

  • Tests-only restructure -- no source code changed
  • All 159 tests still collected
  • -m release_gate selects exactly the consolidated file
  • Meta-gate against the updated release_gate_geotiff.rst passes
  • Helper-name collisions resolved with section prefixes
  • requires_gpu re-sourced from _helpers.markers
  • Module-level pytest.importorskip over-skips the gate suite on minimal installs (see Suggestion)
  • Audit deliverable present; flagged for deletion before merge

…xfail (#2403)

* Replace module-level ``pytest.importorskip("dask")`` and
  ``pytest.importorskip("rasterio")`` with per-test ``skipif``
  decorators sourced from a single set of constants at the top of the
  file. The previous module-level gates would skip the entire 159-test
  registry on a minimal install; the pure-numpy local-read,
  local-write, codec, and attrs-contract gates now run regardless of
  whether dask or rasterio is present. The rasterio overview helpers
  import rasterio (and ``Resampling``) lazily inside the helpers so
  the bare ``from`` import no longer races the skip.
* The windowed-shifted-transform parity tests parametrize over
  ``(eager, dask)``; the dask reader param carries
  ``marks=_requires_dask`` so the eager cell still runs without dask.
* Rename ``_wsp_corpus_file`` -> ``wsp_corpus_file`` to match the
  no-leading-underscore convention used by every other fixture in the
  suite.
* Drop the stale xfail on
  ``test_release_gate_http_ssrf_rejects_loopback_uppercase_scheme``.
  PR #2326 (sub-PR 5 of #2321) landed the case-insensitive scheme
  check, so uppercase HTTP now raises ``UnsafeURLError`` for real.

Verified: ``pytest -m release_gate`` returns 156 passed + 3 xfailed
(the xpass became a clean pass). ``-m release_gate`` from the wider
tests root still selects exactly the 159 tests in the consolidated
file.
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up: review dispositions

Pushed 790b8042. Disposition of each finding from the first review:

Suggestion 1 -- module-level importorskip over-skips the gate suite

Fixed. Module-level pytest.importorskip("dask") (line 743) and
pytest.importorskip("rasterio") (line 1400, plus the bare
from rasterio.enums import Resampling on line 1403) replaced with
per-test _requires_dask / _requires_rasterio /
_requires_rasterio_and_dask decorators sourced from a single block
of importlib.util.find_spec constants. The rasterio overview
helpers now import rasterio and Resampling lazily inside the
helper bodies. The windowed-shifted-transform parity tests get the
skip via marks=_requires_dask on the dask pytest.param so the
eager cell still runs without dask.

Concrete effect: pytest -m release_gate on a no-dask install now
still runs the local-read, local-write, codec, attrs-contract, and
COG gates; the dask-only and dask-only-overview rows skip cleanly.

Nit 1 -- _wsp_corpus_file leading underscore

Fixed. Renamed to wsp_corpus_file (six call sites updated).
The fixture works the same way; just matches the surrounding style.

Nit 2 -- CLUSTER_AUDIT_PR10.md deletion

Deferred to the final commit before merge per the epic protocol.
Tracked explicitly in this comment so the deletion is not forgotten.

Nit 3 -- stale xfail on the uppercase HTTP SSRF test

Fixed. Confirmed PR #2326 merged on 2026-05-23; uppercase HTTP
raises UnsafeURLError for real. Removed the @pytest.mark.xfail
decorator and the obsolete raises=(ValueError, UnsafeURLError)
acceptance. The test now passes outright (previously XPASS).

Counts

Before: 155 passed, 3 xfailed, 1 xpassed = 159.
After: 156 passed, 3 xfailed = 159.

-m release_gate from the wider tests root still picks up exactly
this single file.

@brendancol brendancol merged commit 625facc into main May 26, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consolidate release-gate tests into single registry (epic #2390 PR 10)

1 participant