diff --git a/.agents/skills/running-tests/SKILL.md b/.agents/skills/running-tests/SKILL.md index 136555437..92601b045 100644 --- a/.agents/skills/running-tests/SKILL.md +++ b/.agents/skills/running-tests/SKILL.md @@ -1,75 +1,121 @@ --- name: running-tests description: >- - How to correctly run music21's tests and doctests with pytest. Use this - whenever you need to run, verify, or judge the pass/fail of tests or doctests - in this repo — confirming a change works, checking that doctests still pass, - running a single module's tests, or sanity-checking before a PR or push. - Especially consult this before concluding that a doctest "fails": the repo's - pytest plugin normalizes object addresses and injects the doctest namespace, - and running doctests any other way (e.g. raw doctest.testmod) produces FALSE - failures. + How to run music21's tests and doctests and judge pass/fail: pytest for one + module, the project's own runners for the whole suite. Read it before running + tests, and before calling any doctest failure real. --- # Running music21 tests and doctests -The reliable way to run tests/doctests for a music21 module is plain pytest with -explicit file paths: +Every function, method and class needs documentation and at least one passing +test. See `documentation/source/developerReference/testing.ipynb` for the +project's own account of testing; this skill covers how to run what is there. + +## One module: pytest with explicit file paths ```bash uv run pytest music21/abcFormat/__init__.py music21/abcFormat/translate.py ``` -`pyproject.toml` `[tool.pytest.ini_options]` already wires up everything that -makes this work: +`pyproject.toml` `[tool.pytest.ini_options]` already wires up everything: ``` addopts = ['--doctest-modules', '-p', 'music21.test.pytest_plugin'] doctest_optionflags = ['NORMALIZE_WHITESPACE', 'ELLIPSIS'] ``` -So you do not pass `--doctest-modules` yourself — doctests in every `.py` are +So do not pass `--doctest-modules` yourself -- doctests in every `.py` are collected automatically, and `music21/test/pytest_plugin.py` is loaded. +Pass explicit file paths so the in-module `Test(unittest.TestCase)` classes run +too. `python_files = ['test_*.py', '*_test.py', 'tests.py']`, so for a module +that keeps its `Test` class inside `__init__.py` / `translate.py` (e.g. +`abcFormat`), pointing pytest at the directory collects ONLY doctests and +silently skips the unittest classes: + +- `uv run pytest music21/abcFormat/` -> ~43 items (doctests only) +- `uv run pytest music21/abcFormat/__init__.py music21/abcFormat/translate.py` + -> ~82 items (doctests **and** `Test` methods) + +Modules whose tests live in a `tests.py` are already picked up by the directory +form; the gotcha is specifically a `Test` class in a non-`tests.py` file. + +A module also runs its own tests directly, via the `mainTest` call at the foot +of each file: + +```bash +uv run python -m music21.note # whole module +uv run python music21/note.py testHello # one test method +``` + ## Why the plugin matters (and why not to use raw doctest) -The plugin does two things that make doctests pass the way the project intends: +`music21/test/pytest_plugin.py` does three things that make doctests behave as +the project intends: - It runs `stripAddresses(example.want, '0x...')` (from - `music21.test.testRunner`) on every doctest, so docstrings that hardcode a - repr like `` are - normalized and DO pass. The literal hex address in the docstring is expected - and fine. + `music21.test.testRunner`) on every doctest, so a docstring that hardcodes + `` is normalized and DOES + pass. The literal hex address in the docstring is expected and fine. - It injects the `music21.__all__` names plus `music21` itself into the doctest - namespace, so examples can reference `meter`, `key`, `corpus`, etc. without - importing them. - -This is why you must judge doctest pass/fail **through pytest**. Do NOT use -`doctest.testmod(...)` or `doctest.DocTestSuite(...)` directly to decide whether -doctests pass: that path applies neither the address normalizer nor the -namespace injection, so it manufactures false failures on any docstring that -prints a `<... object at 0x...>` repr or relies on the injected names. If you -ever see only an object-address example "failing," that is the tell that you -ran doctests the wrong way — rerun through pytest. - -## Pass explicit file paths, not a bare directory - -Use explicit file paths so the in-module `Test(unittest.TestCase)` classes run -too, not just doctests. `python_files = ['test_*.py', '*_test.py', 'tests.py']`, -so for a module that keeps its `Test` class inside `__init__.py` / -`translate.py` (e.g. `abcFormat`), pointing pytest at the directory -(`uv run pytest music21/abcFormat/`) collects ONLY its doctests and silently -skips the unittest classes. - -Concretely, for `abcFormat`: -- `uv run pytest music21/abcFormat/` → ~43 items (doctests only) -- `uv run pytest music21/abcFormat/__init__.py music21/abcFormat/translate.py` - → ~82 items (doctests **and** `Test` methods) + namespace, so examples reference `meter`, `key`, `corpus` without importing. +- It keeps only `Test` classes, dropping `TestSlow` and `TestExternal`. + +Judge doctest pass/fail **through pytest**. Do NOT use `doctest.testmod(...)` or +`doctest.DocTestSuite(...)` directly: that path applies neither the address +normalizer nor the namespace injection, so it manufactures false failures on any +docstring printing a `<... object at 0x...>` repr or relying on injected names. +An object-address example failing on its own is the tell that doctests were run +the wrong way -- rerun through pytest. -Both apply the address normalizer; only the explicit-paths form also runs the -unittest classes. When a module's tests live in a `tests.py` file (much of -music21's house style), the directory form already picks those up — the gotcha -is specifically modules that keep `Test` inside a non-`tests.py` file. +## Whole suite: the project's runners, not pytest + +pytest is for a module. For the whole suite use the runners, and note that a +green pytest run does not mean CI will be green -- they gather and order modules +differently. + +```bash +# everyday full run, on n-1 cores +uv run python music21/test/multiprocessTest.py + +# exactly what GitHub Actions runs (~1 minute); use before pushing to a PR +uv run python -c 'from music21.test.testSingleCoreAll import ciMain as ci; ci()' +``` + +The two runners see slightly different sets of modules -- `multiprocessTest` +walks the package tree for modules reachable from `import music21`, while +`testSingleCoreAll` gathers module files from disk. Run both before a release. + +Module **order** differs between the two runners and again from pytest. +`multiprocessTest` goes in reverse-alphabetical order (with the known-slow +modules hoisted to the front on machines with more than 4 cores); +`testSingleCoreAll` re-sorts through `common.misc.sortModules`, by file mtime, +most recently modified first, falling back to reverse-alphabetical when mtimes +tie as they do on a fresh clone; pytest walks alphabetically. So a test that +depends on module order can pass in one runner and fail in another -- and +editing a file locally sorts it to the front of the CI runner, changing the +order again. + +That makes leaked module-level state the usual cause of "passes alone, fails in +the suite." A doctest in `duration.py` once set +`humdrum.spineParser.flavors['JRP'] = True` and never restored it; `flavors` is +a module-level dict, so every later humdrum test in the same process parsed in +the wrong flavor. Under pytest's alphabetical order `duration` runs before +`humdrum` and the failure appeared; under CI's order it did not. + +**So: a doctest that mutates module-level state must restore it**, using +`#_DOCS_HIDE` so the bookkeeping stays out of the published docs: + +``` +>>> saved_JRP_flavor = humdrum.spineParser.flavors['JRP'] #_DOCS_HIDE +>>> humdrum.spineParser.flavors['JRP'] = True +... +>>> humdrum.spineParser.flavors['JRP'] = saved_JRP_flavor #_DOCS_HIDE +``` + +When a test fails only in a full run, re-run it alone. If it then passes, +suspect state left behind by an earlier module rather than the test itself. ## Speed budget @@ -94,9 +140,26 @@ if __name__ == '__main__': Because they have those side effects, skip `TestExternal` when running a file directly. -## Whole suite and the other gates +## What tests may do + +Exactly one `Test(unittest.TestCase)` class per module, methods named `test...`. +It must produce no output and require nothing outside the music21 ecosystem. +Anything slow goes in a `TestSlow` sibling, excluded from the normal run like +`TestExternal`. + +Regression cases for a bug fix belong in `Test`, never in a doctest. See the +`writing-docs` skill. + +## The other gates before a PR or a push + +```bash +uv run ruff check music21 +uv run mypy music21 +uv run pylint -j4 music21 --rcfile=.pylintrc # optional: run only if major refactoring since it was last run. +``` -- Full suite: `python music21/test/multiprocessTest.py` (or - `music21/test/testSingleCoreAll.py` on a single-core machine). -- Before a PR or a push to an open PR, also run the lint and type gates: - `uv run ruff check music21` and `uv run mypy music21`. +Coverage is expected to rise with each contribution +(https://coveralls.io/github/cuthbertLab/music21). CI measures it on one pinned +Python, a middle supported version, so failures on the newest and oldest return +first. `# pragma: no cover` exists for genuinely untriggerable code and is +otherwise discouraged. diff --git a/AGENTS.md b/AGENTS.md index 37adebbe3..c61555b62 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -82,7 +82,7 @@ - `music21` supports at least the last two released versions of Python and up to whatever Python version Google Colab runs (unless it gets EOL). Policy can change as features are added. - The coverage CI run is intentionally pinned to the **middle** supported - Python version. See `coverageM21.getCoverage`. + Python version. See `PY_VERSION_WITH_COVERAGE` in `.github/workflows/maincheck.yml`. # PRs and Issues