docs[next]: document test placement rule and marker semantics#2646
docs[next]: document test placement rule and marker semantics#2646havogt wants to merge 3 commits into
Conversation
- definitions.py: explain index-only vs. consequential uses_* markers next to BACKEND_SKIP_TEST_MATRIX (only matrix-listed markers affect execution; with xfail_strict, adding a consequential marker to a passing test xpasses->fails) - src/gt4py/next/AGENTS.md: note cases_utils.py (shared fixtures), the subject-under-test placement rule, and markers as a queryable feature index - CODING_GUIDELINES.md: fix test-tree typo feature_test/ -> feature_tests/
egparedes
left a comment
There was a problem hiding this comment.
A couple of comments/questions
| - Use the `cases` framework in | ||
| `tests/next_tests/integration_tests/cases.py`: the `cartesian_case` / | ||
| `unstructured_case` fixtures supply a backend + allocator; build inputs | ||
| with `cases.allocate(...)` and check with `cases.verify` / | ||
| `cases.verify_with_default_data`. | ||
| `cases.verify_with_default_data`. The shared fixtures themselves | ||
| (`exec_alloc_descriptor`, `mesh_descriptor`, grid descriptors, dimension / | ||
| offset aliases) live next to it in `integration_tests/cases_utils.py`. |
There was a problem hiding this comment.
Do the cases utilities have meaningful docstrings which expands on this? They should have, in my opinion, or at least human developers should be able to also access this information outside of the AGENTS.md
There was a problem hiding this comment.
Good point. Pushed fcb48ad: the per-helper docstrings (allocate, verify, verify_with_default_data, Case, the initializers) already exist, and cases.py was already documented for humans in CODING_GUIDELINES.md → §Testing → Integration Tests Utils. I added module docstrings to both cases.py and cases_utils.py (the missing overview), and mentioned cases_utils.py in that CODING_GUIDELINES section so its fixtures are discoverable outside AGENTS.md.
| - If a test exercises a feature some backend can't handle, mark it with the | ||
| matching `USES_*` / `CHECKS_SPECIFIC_ERROR` marker from `definitions.py` | ||
| and add that marker to the backend's skip list — do **not** silently drop | ||
| the backend. A new feature marker must be added to both places. | ||
| the backend. A new feature marker must be added to both places. Markers also | ||
| serve as a queryable feature index (`pytest -m uses_<feature>`); only markers | ||
| in `BACKEND_SKIP_TEST_MATRIX` affect execution (the rest are index-only and | ||
| safe to add) — see the note next to that matrix in `definitions.py`. | ||
| - Layout: `unit_tests/` (per-module, backend-free), `feature_tests/` (one DSL | ||
| feature across the matrix), `multi_feature_tests/` (end-to-end programs). | ||
| Place a test in the file for the feature it is the *subject* of; a feature it | ||
| merely uses as a vehicle is recorded as a marker, not a reason to move it (so | ||
| `pytest -m uses_<feature>` finds it regardless of which file it lives in). | ||
| - Run with `uv run pytest tests/next_tests/ -x -q`; matrix-level confidence | ||
| comes from `uv run nox -s "test_next-<py>(...)"`. GPU and `dace` sessions | ||
| may be unavailable locally and will skip. |
There was a problem hiding this comment.
Similar point about those: shouldn't these explanations be also available for developers?
There was a problem hiding this comment.
Agreed — moved the canonical text into human-facing homes and left AGENTS.md as a pointer: the placement rule is now in CODING_GUIDELINES.md §Testing (AGENTS.md references it); the marker semantics (index-only vs. consequential) already live in the code comment next to BACKEND_SKIP_TEST_MATRIX in definitions.py, which AGENTS.md also points to.
Address review on GridTools#2646: keep the canonical info in human-reachable homes and have AGENTS.md point to them. - add module docstrings to cases.py and cases_utils.py (the per-helper docstrings already existed; these give the missing module overview) - CODING_GUIDELINES.md: mention cases_utils.py in 'Integration Tests Utils' and add the subject-under-test placement rule next to the test-tree structure - AGENTS.md: point the placement note at CODING_GUIDELINES instead of being the sole home (marker semantics already documented in definitions.py)
|
Maybe I would leave it as it originally was (in the first commit)? Mainly the pointer for agents? |
| recorded as a marker, not a reason to move it (so `pytest -m uses_<feature>` | ||
| finds it regardless of which file it lives in). | ||
| - Run with `uv run pytest tests/next_tests/ -x -q`; matrix-level confidence | ||
| comes from `uv run nox -s "test_next-<py>(...)"`. GPU and `dace` sessions |
There was a problem hiding this comment.
dace sessions should always be available, now that dace is a non-optional dependency.
Co-authored-by: Hannes Vogt <hannes@havogt.de>
Writes down two test conventions that were previously only tribal knowledge:
uses_*markers — a note next toBACKEND_SKIP_TEST_MATRIXindefinitions.py(only matrix-listed markers affect execution; underxfail_strict, adding a consequential marker to a passing test xpasses → fails).src/gt4py/next/AGENTS.md, plus acases_utils.pypointer and afeature_test/→feature_tests/tree typo fix.No duplication:
AGENTS.mdpoints at the canonical homes.