From 5b42eee8bdef16948e0bc315a3f78a1ff602193b Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Thu, 27 Aug 2026 17:51:16 -1000 Subject: [PATCH 1/4] Expand the running-tests skill to cover the whole-suite runners The skill only described pytest, which is right for one module but is not what CI runs. Adds: the two project runners and how their module sets and orders differ from each other and from pytest, why that makes leaked module-level state show up as "passes alone, fails in the suite," the #_DOCS_HIDE save/restore pattern for a doctest that must mutate module state, the mainTest/-m forms for a single module or test, the Test/TestSlow/TestExternal conventions from documentation/source/developerReference/testing.ipynb, and pylint alongside ruff and mypy. AI-assisted (Claude) --- .agents/skills/running-tests/SKILL.md | 173 ++++++++++++++++++-------- 1 file changed, 122 insertions(+), 51 deletions(-) diff --git a/.agents/skills/running-tests/SKILL.md b/.agents/skills/running-tests/SKILL.md index 685143ea2..064476355 100644 --- a/.agents/skills/running-tests/SKILL.md +++ b/.agents/skills/running-tests/SKILL.md @@ -1,79 +1,150 @@ --- 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 correctly run music21's tests and doctests -- with pytest for a module, + and with the project's own runners for the whole suite. 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 it + before concluding that a doctest "fails" (the repo's pytest plugin normalizes + object addresses and injects the doctest namespace, so raw doctest.testmod + produces FALSE failures), and before concluding that a green pytest run means + CI will be green. --- # 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. + +## 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. -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. +```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 +``` -## Whole suite and the other gates +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. + +## What tests may do + +- Exactly one `Test(unittest.TestCase)` class per module, methods named + `test...`. It must produce no output, open no windows, play nothing, and not + require packages outside the music21 ecosystem. +- Anything that produces external output goes in `TestExternal`; anything slow + goes in `TestSlow`. Both are excluded from the normal run. +- Tests should be fast: a major new module may add a few seconds, a small + addition should add milliseconds. +- 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 # catches what the first two miss +``` -- 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 a single +pinned Python -- the **middle** supported version, see +`coverageM21.getCoverage`. `# pragma: no cover` exists for genuinely +untriggerable code and is otherwise discouraged. From 728ec247fc891870aa6dd9986c617c0fc754a5e4 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Thu, 27 Aug 2026 18:30:14 -1000 Subject: [PATCH 2/4] Shorten the running-tests skill description It loads into every session; 90 words down to 34. AI-assisted (Claude) --- .agents/skills/running-tests/SKILL.md | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/.agents/skills/running-tests/SKILL.md b/.agents/skills/running-tests/SKILL.md index 064476355..a93741c4f 100644 --- a/.agents/skills/running-tests/SKILL.md +++ b/.agents/skills/running-tests/SKILL.md @@ -1,15 +1,9 @@ --- name: running-tests description: >- - How to correctly run music21's tests and doctests -- with pytest for a module, - and with the project's own runners for the whole suite. 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 it - before concluding that a doctest "fails" (the repo's pytest plugin normalizes - object addresses and injects the doctest namespace, so raw doctest.testmod - produces FALSE failures), and before concluding that a green pytest run means - CI will be green. + 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 From e786c52ba5f71f2a3ac5c1508571d0b43c96a34b Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Fri, 28 Aug 2026 12:20:29 -1000 Subject: [PATCH 3/4] pylint is optional; drop the coverageM21 pointer AI-assisted (Claude) --- .agents/skills/running-tests/SKILL.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.agents/skills/running-tests/SKILL.md b/.agents/skills/running-tests/SKILL.md index 116e4be7a..92601b045 100644 --- a/.agents/skills/running-tests/SKILL.md +++ b/.agents/skills/running-tests/SKILL.md @@ -155,11 +155,11 @@ Regression cases for a bug fix belong in `Test`, never in a doctest. See the ```bash uv run ruff check music21 uv run mypy music21 -uv run pylint -j4 music21 --rcfile=.pylintrc # catches what the first two miss +uv run pylint -j4 music21 --rcfile=.pylintrc # optional: run only if major refactoring since it was last run. ``` Coverage is expected to rise with each contribution -(https://coveralls.io/github/cuthbertLab/music21). CI measures it on a single -pinned Python -- the **middle** supported version, see -`coverageM21.getCoverage`. `# pragma: no cover` exists for genuinely -untriggerable code and is otherwise discouraged. +(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. From ee8808d5663a31bbf4c7ba6fd0e3b2ba62307bb4 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Fri, 28 Aug 2026 12:24:29 -1000 Subject: [PATCH 4/4] Point AGENTS.md at the workflow variable for the coverage Python coverageM21.py was removed in #2014; the pin is now PY_VERSION_WITH_COVERAGE. AI-assisted (Claude) --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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