Add a pytest scaffold and run it in CI

[mmacferrin]

Closes #18 — or rather, opens it properly. This is the groundwork only: the
layout, the configuration, the isolation fixtures, and a CI job, plus enough
tests to prove the whole path works end to end. The actual test coverage comes
after, and the plan for it is sketched at the bottom.

Infrastructure

Layout. The suite lives in tests/ at the repository root.

IVERT/
├── pyproject.toml              # modified: 3 blocks
├── .github/workflows/test.yml  # new: the CI job
└── tests/
    ├── conftest.py             # isolation fixture, autouse
    ├── test_cli.py             # smoke tests
    └── test_isolation.py       # tests for the fixture itself

Root rather than src/ for the usual src-layout reason — src/ holds only
what ships in the wheel, and [tool.hatch.build.targets.wheel] already says
packages = ["src/ivert"].

The other real option was src/ivert/tests/ — inside the package, shipped in
the wheel, the way numpy and astropy do it, so an installed copy can be checked
with pytest --pyargs ivert. I went the other way because the on-disk and
end-to-end tests below will need fixture data, and under that layout it ships
to every user and grows the conda package. publish-to-pypi.yml already
smoke-tests the built wheel, which covers most of that benefit.

tests/ is deliberately not a package — no __init__.py — so test file
basenames must stay unique across the tree. Files are named after the module
they cover.

Configuration. One [tool.pytest.ini_options] block in pyproject.toml,
read by both a local run and the workflow. No pytest.ini, no tox.ini,
nothing that can drift between them.

• testpaths = ["tests"] — Tells the pytest regime to look for tests there,
  nowhere else.
• addopts carries -m 'not network and not slow', which is what makes
  pytest with no arguments mean the fast, offline tests. That default is
  what makes it safe in a pre-push hook and safe on a machine with no Earthdata
  credentials. Run the excluded ones deliberately with pytest -m network, or
  clear the filter with pytest -m "".
• --strict-markers and --strict-config — a mistyped @pytest.mark becomes
  an error instead of a decorator that silently does nothing.
• Two markers declared: network and slow. Nothing carries them yet; they
  exist so the tests that can't run on a PR have a defined home rather than
  being an argument against starting. network would include tests that rely
  on network access, external data source downloads. slow indicates longer-
  running tests (such as end-to-end workflows) that are appropriate to test
  prior to version releases but not for basic PR-level unit testing.

Dependencies. A test group next to the existing docs group, so it
installs the same way Read The Docs already installs docs:

pip install --group test -e .   # --group needs pip 25.1+

Two packages, deliberately — it won't perturb a conda environment whose
geospatial stack took effort to build.

CI. .github/workflows/test.yml, on push to main and every PR. It runs
the same pytest you run locally, plus coverage flags. Following the
conventions the existing three workflows establish, since zizmor.yml audits
every workflow on change: empty top-level permissions, persist-credentials: false, commit-pinned actions (both SHAs copied verbatim from
publish-to-pypi.yml). A concurrency group cancels superseded runs.

Python 3.12 and 3.13. 3.14 is out for the same reason publish-to-pypi.yml
pins 3.13 — fiona, via globato, has no cp314 wheels and would build from
source.

Docs. A Tests section in CONTRIBUTING.md: how to run them, what the
markers mean, the no-__init__.py naming constraint, and that fixture data is
generated rather than committed.

No CHANGELOG.md entry — CI and test-only changes have no user-visible effect,
which CONTRIBUTING.md already carves out.

The initial tests

28 tests, 1.5 seconds. Two files.

tests/test_cli.py — smoke tests

These import and invoke every command in the cli.py tree, which catches a
broken import, a malformed decorator, or two options sharing a flag — the
failures that otherwise reach a user as a traceback on the first thing they
type.

The command tree is walked, not listed: _command_paths() recurses through
click.Group.commands, so a command added later is covered without anyone
remembering to extend the test. 18 paths in this check-in. A guard test asserts
the walk found something, since an empty parametrization would pass vacuously.

Plus three that run real commands: --version, classes (which also exercises
the globato docstring parse behind photon classification), and options list
(which reads config, under the isolation below).

tests/conftest.py — the isolation fixture

IVERT carries four pieces of process-global state. Left alone, tests read and
write the developer's real configuration and pass or fail depending on whose
machine they run on. One autouse fixture closes all four off. It applies
unconditionally, in CI as well as locally, which matters most on a developer's
own machine — that is where a real ~/.ivert exists to be clobbered.

State	Handling
The user config file	IVERT_USER_CONFIG → a per-test tmp_path. Config.user_config_path honours it ahead of the packaged default, which makes it the clean seam.
configfile.ivert_config	Config.__init__ assigns this module global; reset so the first test to build one doesn't leak it into the rest.
Platform detection	is_aws() pinned off — a laptop and an EC2 instance otherwise disagree, and Config switches to the [AWS] section on it.
photon_classes()	lru_cached over a parse of globato's docstring; cleared on both setup and teardown.

tests/test_isolation.py — testing the fixture

This is the part I'd most like reviewed. An autouse fixture is invisible — no
test mentions it — so if it silently stopped applying, all 22 other tests would
still pass, while reading and writing a real ~/.ivert. These six make that
failure loud.

The first draft had five tests and two of them were decorative. I disabled the
fixture body and re-ran to see which ones actually noticed: three failed, two
passed. test_aws_detection_is_pinned_off asserted is_aws is False, which is
true anyway on any machine that isn't EC2, and the cache check asserted an
empty cache that was empty anyway. Both looked like coverage and provided none.

They were replaced with tests of the seam rather than the state — patch the
thing to its non-default value and confirm IVERT actually reads it there:

def test_config_reads_aws_detection_through_the_patchable_seam(monkeypatch):
    monkeypatch.setattr("ivert.utils.is_aws.is_aws", lambda: True)
    assert configfile.Config().is_aws is True

configfile does from ivert.utils import is_aws and calls is_aws.is_aws(),
so patching the module attribute reaches Config. Rebinding that import to the
function itself would strand the fixture's patch — and Config would quietly
start reading the [AWS] section on a laptop. Verified: rewriting the import to
from ivert.utils.is_aws import is_aws fails this test and only this test.

Of note: the [AWS] parsing in the configfile is currently stale — the
packaged ivert_defaults.ini has no [AWS] section at all, simply because
IVERT is set up currently as a desktop/laptop/workstation implementation, not
the client-server architecture it had when being used in the cloud. However, we
are keeping the special [AWS] section handling in configfile.py (triggered if
is_aws.py::is_aws() returns True upon execution) to make it easy to
re-enable a client-server architecture in the future.

The cache check became an ordered pair — one test populates the lru_cache, the
next asserts the teardown emptied it. The ordering is load-bearing and is
commented as such.

Verified with the fixture body disabled: 4 of the 6 fail. The two that don't
are the seam tests, which do their own patching and are meant to pass
independently — they guard the refactor, not the fixture's application.

What's deliberately not here

Coverage of the actual logic. The plan, roughly in the order we'll write these
tests:

1. Pure functions — Recent refactors already pulled the hard
   parts into plain-in/plain-out functions: _check_dem_geotransform,
   _normalize_export_formats, _photon_results_filename, resolve_vdatum,
   normalize_format_keys, split_bbox_into_parts.
2. utils/cuboid_funcs.py — Pure geometry over numpy, determines whether
   filter_query_bbox re-downloads granules we already have. Two property
   tests pin it: volume conservation under subtraction, and no overlap between
   remainder pieces, since each becomes its own NSIDC request.
3. Synthetic fixtures on disk — a small GeoTIFF built in a fixture, never
   committed, for dem_geom and split_dem.
4. network / slow — granule downloads and a real end-to-end validation
   run, behind the markers.

Two follow-ups once this is green: needs: "test" on the build job in
publish-to-pypi.yml, so a release can't publish over a failing suite; and a
pre-push hook running the fast tiers.

Questions

1. tests/ at the root, or src/ivert/tests/ shipped in the wheel?
   Argument for root tests/: we don't have to ship artifact data needed for
   tests with the code package on PyPI and Conda Forge.
   Argument for src/ivert/tests/: Given IVERT's correctness depends on
   GDAL/PROJ/pyproj versions we don't control, there's a real argument for the
   shipped testing layout, that users who install the package could run
   independently. (Note:
   Even if a user can't install the tests with a package, they could still do
   so by cloning the repository directly.)
2. Is -m 'not network and not slow' in addopts too clever? It makes
   pytest safe by default but means the bare command doesn't run everything,
   which can surprise people. The alternative is an explicit -m in the
   workflow and no default filter.
3. Python matrix. 3.12 and 3.13, ubuntu only for now for testing. Worth
   adding macOS now, or wait until something platform-specific breaks?

Verification

• pytest → 28 passed in 1.5s
• prek run --all-files → all hooks pass
• ruff check src tests → clean. The "tests/**" = ["INP001"] carve-out was
  confirmed necessary by removing it and seeing three real errors, not added
  speculatively. It's scoped to tests/, so it doesn't count against Progressively enable ~all Ruff rules #40.
• test.yml parses as valid YAML.

---

🔍 Docs preview: https://ivert--102.org.readthedocs.build/en/102/

[mmacferrin]

@mfisher87, I would love to hear your feedback on whether this architecture for pytest'ing makes sense to you, or whether you would suggest alternatives. This code isn't merged yet, would really appreciate your review before I start adding test that are more difficult to re-arrange later.

@matth-love, I believe this is more-or-less consistent with how you arrange unit testing in fetchez and other packages. If you see major inconsistencies here, feel free to point them out. I'd like to stick to the house style if possible.

[matth-love]

From the look of it, this is basically how I setup tests in fetchez/transformez as well. Since i'm currently working on transformez this is how it's set up there:

tests live in transformez/tests (as you do here). In pyproject.toml I have:

[dependency-groups]
test = [
    "pytest",
    "pytest-cov",
]

and further down:

[tool.pytest.ini_options]
addopts = "-m 'not validation'"
markers = [
    "health: tests requiring live network access to external APIs",
    "validation: scientific validation against external reference systems",
    "slow: tests that are too expensive for the normal unit-test suite",
    "vdatum: tests requiring the locally installed NOAA VDatum Java engine",
]

currently only have default tests, which live in transformez/tests/test_*.py and validation tests (which require a live network connection and runs full transformez workflow) which live in transformez/tests/validation.

in the 'tests/validation/test_*.py` files I have them defined like:

@pytest.mark.validation
@pytest.mark.health
@pytest.mark.slow
def test_station_validation():
    results = []

    for name, config in TEST_REGIONS.items():
        region = Region(*config["bounds"])

        result = validate_against_stations(
            name,
            region,
            config["challenge"],
        )

        if result is not None:
            results.append(result)

    assert results

Then just running uv run pytest or the like skips these validation tests (runs fast and this is what runs in github). If I want to run the validation tests I add -m validation and the adopts= "-m 'not validation'" is what tells the default to not run them.

Not sure if this is the best way to set this up, but seems to work. You don't have to follow this model, if you come up with something better, I can follow your lead as well...

[matth-love]

This is a great description! We should probably add this, or something like it, to the community compass contributing guide when we decide on an ecosystem-wide standard. I really like this though.

[matth-love]

And yes, looking closer here I think we have the tests set up almost exactly the same, other than a few small configuration differences. I think you've been a bit more deliberate here than I've been so I think I'll just follow whatever you end up pushing in this PR so the eco-system is set up with basically the same configurations/naming conventions, etc. And we can come up with a 'policy' we (and contributors) can follow in the community compass (based off your CONTRIBUTING.md)

[mfisher87]

Sorry, this is a LOT of stuff for a review, but it's also a big PR :) Hope the feedback feels compassionate. I had a rough day today and I'm sorry if any of that leaks through in my comments. ❤️

[mfisher87]

Suggested change

	# 3.12 is the floor in pyproject; 3.13 is what the release build pins;
	# 3.14 is the newest release. Every compiled dependency ships cp314
	# wheels, so nothing here has to build from source.

Hope this isn't too nitpicky, but I would suggest cleaning up some of these comments that aren't adding much useful context.

[mfisher87]

Suggested change

	python-version: ["3.12", "3.13", "3.14"]
	python-version: ["3.12", "3.14"]

I usually do oldest and newest only. If some change in Python breaks 3.13, it will almost certainly break 3.14. Testing all of them is more thorough, but IMO not worth the electricity! I don't feel strongly about this, feel free to reject :)

[mfisher87]

This comment could easily become false if we forget to use these markers when we eventually add slow/network-dependent tests. I would remove.

[mfisher87]

This is a useful comment, but also calls out some fragility we probably want to fix. These monkeypatches can easily go out of date and silently cause some weird test-time behavior. I largely consider singletons to be a code smell, but sometimes they're necessary and I use them. A "problem" for another day, IMO, just calling it out :)

Let's open an issue to consider removing singletons / global state where possible?

[mfisher87]

I especially don't like this. We should expose a variable for photon classes instead of parsing a docstring. Another problem for the future though :)

[mfisher87]

"autouse" is lingo and we should probably link to relevant docs

[matth-love]

I had to look this up as i'd never heard this term before (i thought it was a typo at first). A doc link would be helpful I think.

[mfisher87]

This stuff is not really relevant to writing a test, I might move this to its own section. Most developers won't need to care about this.

[mfisher87]

We could be less specific about this marker. A slow test may not run a full DEM validation and still be slow.

Suggested change

	"slow: runs a full DEM validation; minutes, not seconds",
	"slow: A test that takes too long to run every time (i.e. tens of seconds or more)",

[mfisher87]

Same reason I suggested a change to slow marker description. We could even break "needs credentials" into its own separate marker since tests can have >1 marker. There may be network tests that don't require credentials.

Suggested change

	"network: hits NSIDC/Harmony; needs Earthdata credentials in ~/.netrc",
	"network: hits a third-party network dependency; needs Earthdata credentials in ~/.netrc",

[mfisher87]

Suggested change

	# tests/ is deliberately not a package, which INP001 objects to.
	# tests/ is deliberately not a package, which INP001 forbids.

Nitpick. I read "objects" as a plural noun at first :P

[mmacferrin]

No apologies needed; this is exactly the kind of feedback I'm looking for here. I'll go through all these individually tomorrow. Thank you for putting the time & attention on it! I really appreciate it.