Files
chemenu/instructions/dev/testing-conventions.md
T
torben 23307c3c5f
CI / verify (push) Successful in 55s
Release / release (push) Successful in 38s
fix: Testisolation - kb_dir repointet config.ROOT, lint loest Kollektionen gegen den uebergebenen Baum auf (4.1.1, #44)
Files changed:
- CHANGES.md
- VERSION
- instructions/dev/testing-conventions.md
- tools/chemenu/lint_core.py
- tools/chemenu/tests/conftest.py
- tools/chemenu/tests/test_hermetic_env.py
- tools/chemenu/tests/test_lint.py
2026-09-03 06:33:38 +02:00

172 lines
9.1 KiB
Markdown

---
type: types/instruction.md
name: testing-conventions
description: How to write a test for this stack so it passes on a machine that is not yours - what the hermetic environment fixture already handles, and what a test still has to establish itself.
---
# Write tests that do not depend on the machine they run on
Every test in `tools/chemenu/tests/` runs against a deliberately empty machine. That is not
a convention you have to remember: the autouse `hermetic_environment` fixture in
`tools/chemenu/tests/conftest.py` enforces it before each test, and
`test_hermetic_env.py` asserts that the fixture still does. What you have to remember is the
consequence - **a test that needs an identity, a token, or a home directory establishes it
itself.**
This exists because the suite once did not. `config.default_author()` shells out to
`git config user.name`, and for months the answer came from the global git configuration of
whoever ran pytest. 628 tests were green on every developer machine and two of them failed on
the first CI run that ever reached pytest, in a container that had no such configuration
(Gitea #8). Two more tests of the same kind were written afterwards, by someone who had read
that issue first - which is the argument for a fixture rather than a rule.
## What the fixture already neutralizes
Do not re-do any of this per test; it is done for you, per test, via `monkeypatch`.
| Neutralized | To |
|---|---|
| `HOME` | a fresh empty directory in that test's `tmp_path` (also the fixture's return value) |
| `XDG_CONFIG_HOME` | `$HOME/.config`, which does not exist |
| `GIT_CONFIG_GLOBAL`, `GIT_CONFIG_SYSTEM` | `/dev/null` - git's own way to say "no such file" |
| `GIT_DIR`, `GIT_WORK_TREE`, `GIT_AUTHOR_*`, `GIT_COMMITTER_*`, `EMAIL` | unset |
| `WIKI_AUTHOR`, `WIKI_TRACE`, `WIKI_TRACE_CONTENT`, `WIKI_TRACE_MAX_CONTENT`, `WIKITOOL_SESSION_ID`, `WIKITOOL_UPDATE_URL`, `WIKITOOL_UPDATE_TOKEN` | unset |
`WIKI_TRACE_DIR` is the one variable that stays *set*: the separate `isolated_trace_dir`
fixture redirects it into `tmp_path`. Tracing is never disabled suite-wide, because two
telemetry tests assert that a trace gets written.
Two in-process caches are cleared alongside the environment, for the same reason: `config`'s
resolved paths and `conventions`' parsed `kb/CONVENTIONS.md`. A test that *rewrites* the
conventions file mid-test calls `conventions.reset_cache()` itself - the fixture answers for the
boundary between tests, not for one inside a test.
## Which tree a test writes into
The environment is one half of the isolation; `config.ROOT` is the other. With `CHEMENU_ROOT`
cleared, `ROOT` falls back to the checkout pytest is running from - deliberately, because most
tests want the shipped `types/`. It also means that any code path resolving a file through
`config.ROOT` or `config.KB_DIR` reaches **the real repository**, no matter which tree the
fixture built.
Both corpus fixtures therefore repoint it: `raw_dir` and `kb_dir` each set
`config.ROOT` to their `tmp_path` and re-declare the shipped `types/` through
`use_shipped_type_specs()`. `config`'s module `__getattr__` resolves the derived paths on
access, so repointing `ROOT` carries `KB_DIR`, `RAW_DIR` and the rest with it. A new fixture
that builds a tree does the same thing - that is the rule here, not a per-test judgment.
`kb_dir` did not, until Gitea #44. Two things came of that. A test calling
`kb_state.write_kb_state()` overwrote the real `.wikitool-kb.json`, which `git status` made
visible within the minute. Quieter and worse: `lint`'s collection lookup resolved a page
against `config.KB_DIR`, so every fixture page read back as "no collection" and the
`unauthorised_labels` check skipped every edge in silence - the finding had no working test at
all, and its green run read like an assurance.
Two guards came out of it, both in `conftest.py`:
| Guard | Default | Cost |
|---|---|---|
| `repository_tree_guard` (session) | on | two `git status --porcelain` calls per run |
| `per_test_tree_guard` | off, `CHEMENU_TREE_GUARD=each` turns it on | one `git status` per test |
The session guard compares the working tree before against after and fails the run if anything
moved, so it says nothing about uncommitted work a developer already had. It cannot name the
test that did it; `CHEMENU_TREE_GUARD=each` can, and is the way to bisect once it fires. Where
git is unavailable or the checkout is not a repository, both are silently inert.
Neither guard sees the second, quieter half: a check that silently *does nothing* under test
writes no file. That one is only caught by a test that asserts the finding actually fires -
which is why `test_unauthorised_label_is_judged_in_a_tree_that_is_not_the_configured_kb`
lints a tree `ROOT` deliberately points away from.
**A function that takes a directory resolves against that directory.** `run_lint(kb_dir)`
reading `config.KB_DIR` for one of its own lookups was the defect behind the quiet half, and
no fixture can fix that shape from the outside.
## When to run
Whenever you add or change a test under `tools/chemenu/tests/`.
## Steps
1. **Decide whether the test needs an author identity.** It does if it reaches
`wikitool new source` (through the `CliRunner` or otherwise), `doctor`, `migrate`, or
anything else that stamps a page. Under the fixture there is no ambient identity, so the
call fails with `ERROR No author configured for this instance.` if you skip this.
2. **Establish it explicitly, one of two ways** - pick by what the test is actually about:
- The test is about *something else* and just needs a page to exist:
```python
monkeypatch.setenv("WIKI_AUTHOR", "Fixture Author") # no ambient identity under the fixture
```
- The test is about *authorship itself* - then make the fixture root a real repository with
a local identity, and assert the concrete name:
```python
subprocess.run(["git", "init", "-q", "-b", "main"], cwd=root, check=True)
subprocess.run(["git", "config", "user.name", "Fixture Author"], cwd=root, check=True)
```
`-b main` is not cosmetic: without a global configuration git prints an
`init.defaultBranch` advisory that clutters the output of a test that is failing for an
unrelated reason.
3. **Never set an identity in `conftest.py` for everyone.** A shared default would make
`default_author()`'s fallback untestable - the branch that returns `None` only exists on a
machine that knows nobody, and `test_hermetic_env.py` covers it precisely because the
fixture creates that machine.
4. **Adding a new environment variable to the tool?** Add it to `_WIKITOOL_ENV` in
`conftest.py` in the same change. A variable the tool reads and the fixture does not clear
is the exact hole this whole file is about, reopened.
5. **Writing a fixture that builds a tree?** Repoint `config.ROOT` at it and call
`use_shipped_type_specs(monkeypatch)`, as `raw_dir` and `kb_dir` do - see
[Which tree a test writes into](#which-tree-a-test-writes-into). A fixture that returns a
path without repointing hands the code under test the real repository.
6. **Verify against an empty machine before publishing**, not only in your own shell:
```bash
cd tools && env -i PATH="$PATH" HOME="$(mktemp -d)" \
GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null \
.venv/bin/python -m pytest -q
```
With the fixture in place this must produce exactly the same result as a plain
`.venv/bin/python -m pytest -q`. A difference between the two is a leak, and the leaking
variable belongs in step 4's list.
7. **Check the coverage report when adding tests to close a gap**, rather than guessing which
lines were uncovered:
```bash
cd tools && .venv/bin/python -m pytest -q --cov # needs pytest-cov, CI-only
```
Read it by module, not by total. A thin Typer wrapper sitting low is evidence that the logic
was cut out from under it and tested there; the list worth acting on is the modules whose
*logic* is uncovered. EVALS.md § "How much of the stack the suite reaches" names both, and
the measured baseline. There is no threshold to satisfy - the suite is not graded on the
number.
## Decision points
- **A test genuinely needs the developer's real environment?** There is no such test, and a new
one is a design problem rather than an exception: what it wants is a fixture that *builds*
the state it needs inside `tmp_path`. Building it is also the only version CI can run.
- **A test patches `config.default_author` directly** (as
`test_new_source_fails_hard_without_any_author` does)? Keep the patch. It is not made
redundant by the fixture - it pins the value under test regardless of what the environment
would have resolved to, and it is what keeps that test about the CLI's error path rather than
about the environment.
## Scope
Applies to `tools/chemenu/tests/` only. It says nothing about what to test - the test/review
expectations for a stack change are the `stack-dev` skill's step 4 (`docs verify`,
`instructions verify`, pytest). CI runs the suite once, unhardened, because the fixture makes a
second hardened run redundant; see the note on the Tests step in `.gitea/workflows/ci.yml`.