developmentseed / developmentseed/titiler-covjson

Move the schema-validation helpers out of tests/conftest.py

Open
#74 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
1
Forks
1
Avg merge
2d 1h
Merged PRs (30d)
12

Description

`tests/conftest.py` (349 lines) holds two unrelated things: 10 pytest fixtures, and 5 plain helper functions: `validate_covjson`, `assert_schema_valid`, `parse`, `roundtrip`, `roundtrip_is_stable`.

The fixtures are fine: pytest auto-discovers them and nothing imports them. The helpers get no auto-discovery, so every test module that wants one writes `from conftest import ...`, which pytest's own docs advise against:

> If you have `conftest.py` files which do not reside in a python package directory (i.e., one containing an `__init__.py`) then "import conftest" can be ambiguous... It is thus good practice for projects to either put `conftest.py` under a package scope or to never import anything from a `conftest.py` file.
> — https://docs.pytest.org/en/stable/how-to/writing_plugins.html

We satisfy neither branch: there is no `tests/__init__.py`, and 6 modules import from conftest (`test_conftest`, `test_factory`, `test_helpers`, `test_modeler`, `test_spec_roundtrip`, `test_playground_roundtrip`).

**Severity: latent, not live.** The hazard the docs name is ambiguity between multiple `conftest.py` files on `sys.path`, and this repo has exactly one, so nothing is broken today. The guidance is "good practice", not a MUST. It becomes a problem the day a second conftest appears (e.g., `tests/integration/conftest.py`).

**Suggested fix:** move the 5 helpers to a normal test-support module and leave conftest to its fixtures. That takes the docs' "never import anything from a conftest.py" branch and also fixes the underlying cohesion problem, which is the real one: a 349-line conftest doing two jobs. Adding `tests/__init__.py` is the other sanctioned branch, but it only legitimizes the import and leaves the two jobs fused.

The module name is currently undecided. Note `tests/helpers.py` would be actively confusing next to `tests/test_helpers.py`, which tests `src/titiler_covjson/helpers.py`. Candidates: `tests/support.py`, `tests/covjson.py`.

Touches the 6 importing modules and the AGENTS.md testing-conventions section (L173), which currently codifies the pattern: "`tests/conftest.py` provides `validate_covjson` / `assert_schema_valid`".

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with tests/conftest.py and the six importing modules: test_conftest, test_factory, test_helpers, test_modeler, test_spec_roundtrip, and test_playground_roundtrip. Choose a normal test-support module, move the five named helpers there, update imports and the AGENTS.md testing-conventions section, then run the test suite to confirm all tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.