From bdf5a89aa3c32ca2c21c0311e67a6b39e4738338 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Thu, 27 Aug 2026 17:52:33 -1000 Subject: [PATCH 1/4] Test runner: import modules by package name, not as top-level copies ModuleGather.getModule loaded each file with load_source() under its bare name ('key' for music21/key.py), which re-executed the file as a separate top-level module. Every unittest the single-core runner ran therefore tested a shadow copy of its module: `mod.KeySignature is not music21.key.KeySignature`, and `isinstance(ks, music21.key.KeySignature)` was False. It also gave those classes a `__module__` of 'key', so a test asserting a full repr saw `` under CI and `` under pytest -- which is why tests import names from `music21.x` rather than relying on their own module globals. Import by fully-qualified name instead, which returns the module music21 already imported. load_source() is now unused and removed. multiprocessTest was unaffected -- it uses getModuleWithoutImp, which walks the real package tree. DO NOT MERGE AS IS -- reported coverage drops from 93.33% to 77.20%. testSingleCoreAll calls coverageM21.getCoverage() at module import time, after its own `from music21 import ...` lines have already run, so music21's module-level code (class bodies, defs, constants) executes before cov.start(). Re-executing every file through load_source() is what currently gets those lines counted. A real fix has to start coverage before music21 is imported (measured on the branch: 93.33% at 15c08545d, 77.20% with this commit). Then drop the `from music21. import ...` lines inside Test methods that exist only to work around the shadow modules. AI-assisted (Claude) --- music21/test/commonTest.py | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/music21/test/commonTest.py b/music21/test/commonTest.py index d473a02ab0..64e06d18ca 100644 --- a/music21/test/commonTest.py +++ b/music21/test/commonTest.py @@ -16,9 +16,7 @@ import copy import doctest import importlib -import importlib.util import os -import sys import typing import types import unittest.runner @@ -61,28 +59,6 @@ def testCopyAll(testInstance: unittest.TestCase, globals_: typing.Dict[str, typi testInstance.fail(f'Could not deepcopy obj {part}: {e}') -def load_source(name: str, path: str) -> types.ModuleType: - ''' - Replacement for deprecated imp.load_source() - - Thanks to: - https://github.com/epfl-scitas/spack for pointing out the - important missing "spec.loader.exec_module(module)" line. - ''' - spec = importlib.util.spec_from_file_location(name, path) - if spec is None or spec.loader is None: - raise FileNotFoundError(f'No such file or directory: {path!r}') - if name in sys.modules: - module = sys.modules[name] - else: - module = importlib.util.module_from_spec(spec) - if module is None: - raise FileNotFoundError(f'No such file or directory: {path!r}') - sys.modules[name] = module - spec.loader.exec_module(module) - - return module - # noinspection PyPackageRequirements def testImports(): ''' @@ -417,11 +393,11 @@ def getModule(self, fp, restoreEnvironmentDefaults=False): if skip: return None - name = self._getNamePeriod(fp, addM21=False) + name = self._getNamePeriod(fp, addM21=True) try: with warnings.catch_warnings(): - mod = load_source(name, fp) + mod = importlib.import_module(name) except Exception as excp: # pylint: disable=broad-exception-caught environLocal.warn(['failed import:', name, '\t', fp, '\n', '\tEXCEPTION:', str(excp).strip()]) From 1afa86e8cd89da6dfd6347e7672c26c0f7144331 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Fri, 28 Aug 2026 10:55:02 -1000 Subject: [PATCH 2/4] Start coverage before music21 is imported Splitting the runner fix from its own consequence: importing modules by package name stopped re-executing them, and reported coverage fell from 93.33% to 77.20%. The re-execution was propping up the number. testSingleCoreAll called coverageM21.getCoverage() at import time, several lines below its own `from music21 import ...` statements, so every class body, def and module constant had already run before cov.start(). Measured directly: with coverage started first, importing music21 alone covers 88/430 statements in key.py and 119/710 in tempo.py; started afterwards, zero. That is the missing 16%. Nothing inside the package can start coverage early, since importing it imports music21. So coverage now wraps the process: `coverage run -m music21.test.testSingleCoreAll ci` on the pinned Python, plain python elsewhere. The omit and exclude lists move to .coveragerc, which the in-process Coverage() was already reading for everything else, and coverageM21.py goes away with the plumbing it existed to hold. Local full run: 93.150% on 3.12, against coveralls' 93.331% on 3.13. Also drops the `from music21 import key` in key.Test.testNonTraditional, which existed only to dodge the shadow modules. AI-assisted (Claude) --- .coveragerc | 8 +++- .github/workflows/maincheck.yml | 11 ++++- music21/key.py | 8 ++-- music21/test/coverageM21.py | 70 ------------------------------- music21/test/testSingleCoreAll.py | 17 ++------ 5 files changed, 24 insertions(+), 90 deletions(-) delete mode 100644 music21/test/coverageM21.py diff --git a/.coveragerc b/.coveragerc index a134e125e3..e5a0bef7f6 100644 --- a/.coveragerc +++ b/.coveragerc @@ -3,10 +3,16 @@ source = music21/ omit = - music21/test/timeGraph* + music21/test/* + music21/configure.py + music21/figuredBass/examples.py + music21/alpha/* + dist/dist.py [report] exclude_lines = + import music21 + music21.mainTest() if TYPE_CHECKING: if t.TYPE_CHECKING: if __name__ == .__main__.: diff --git a/.github/workflows/maincheck.yml b/.github/workflows/maincheck.yml index d046ccaec2..0537f2c6f1 100644 --- a/.github/workflows/maincheck.yml +++ b/.github/workflows/maincheck.yml @@ -35,10 +35,19 @@ jobs: - name: Setup Lilypond run: uv run python -c 'from music21 import environment; environment.UserSettings()["lilypondPath"] = "/home/runner/bin/lilypond"' - name: Run Main Test script + if: ${{ matrix.python-version != '3.13' }} run: uv run python -c 'from music21.test.testSingleCoreAll import ciMain as ci; ci()' + # Coverage has to wrap the whole process: anything imported from inside the + # music21 package imports music21 first, and by then every class body and def + # has already run and would read as uncovered. Run on a MIDDLE supported + # Python, so failures on the newest and oldest stay quick to see. + # When changing the version, change the two conditions below it as well. + - name: Run Main Test script with coverage + if: ${{ matrix.python-version == '3.13' }} + run: uv run coverage run -m music21.test.testSingleCoreAll ci - name: Coveralls if: ${{ matrix.python-version == '3.13' }} - env: # when changing number above also change coverageM21.getCoverage + env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} COVERALLS_SERVICE_NAME: github run: uv run coveralls diff --git a/music21/key.py b/music21/key.py index b23054f513..05a2d69a01 100644 --- a/music21/key.py +++ b/music21/key.py @@ -1390,12 +1390,10 @@ def testNonTraditional(self): ''' AI-assisted (Claude). ''' - from music21 import key - - ks = key.KeySignature(3) + ks = KeySignature(3) self.assertFalse(ks.isNonTraditional) - ks = key.KeySignature() + ks = KeySignature() ks.isNonTraditional = True ks.alteredPitches = [pitch.Pitch('E`')] self.assertEqual(repr(ks), '') @@ -1403,7 +1401,7 @@ def testNonTraditional(self): # a non-traditional key signature is not equal to the C-major signature # it shares a `sharps` count with. - self.assertNotEqual(ks, key.KeySignature()) + self.assertNotEqual(ks, KeySignature()) def testSharpsNoneDeprecated(self): ''' diff --git a/music21/test/coverageM21.py b/music21/test/coverageM21.py deleted file mode 100644 index 51f7d10fd3..0000000000 --- a/music21/test/coverageM21.py +++ /dev/null @@ -1,70 +0,0 @@ -# ------------------------------------------------------------------------------ -# Name: coverageM21.py -# Purpose: Starts Coverage w/ default arguments -# -# Authors: Christopher Ariza -# Michael Scott Asato Cuthbert -# -# Copyright: Copyright © 2014-15 Michael Scott Asato Cuthbert -# License: BSD, see license.txt -# ------------------------------------------------------------------------------ -from __future__ import annotations - -import sys - -omit_modules = [ - 'dist/dist.py', - 'music21/test/*', - 'music21/configure.py', - 'music21/figuredBass/examples.py', - 'music21/alpha/*', -] - -# THESE ARE NOT RELEVANT FOR coveralls.io -- edit .coveragerc to change that -exclude_lines = [ - r'\s*import music21\s*', - r'\s*music21.mainTest\(\)\s*', - r'.*#\s*pragma:\s*no cover.*', - r'class TestExternal.*', - r'class TestSlow.*', - r'\s*if TYPE_CHECKING:\s*', - r'\s*if t.TYPE_CHECKING:\s*', -] - - -def getCoverage(overrideVersion=False): - # MEMORY / NOTE FOR UPDATING PYTHON: - # Run this on a MIDDLE supported Python version so that we can - # check timing of newest vs oldest, AND so that - # we can quickly see failures on newest and oldest. - # (The odds of a failure on the middle version are low if - # the newest and oldest are passing.) - # - # Note the .minor == 13 -- that makes it only run on 3.13 - # - # When changing the version, be sure also to change - # .github/workflows/maincheck.yml's line: - # if: ${{ matrix.python-version == '3.13' }} - if overrideVersion or sys.version_info.minor == 13: - try: - # noinspection PyPackageRequirements - import coverage # type: ignore - cov = coverage.Coverage(omit=omit_modules) # , debug='trace') - for e in exclude_lines: - cov.exclude(e, which='exclude') - cov.start() - import music21 # pylint: disable=unused-import # noqa: F401 - except ImportError: - cov = None - else: - cov = None - return cov - -def startCoverage(cov): - if cov is not None: - cov.start() - -def stopCoverage(cov): - if cov is not None: - cov.stop() - cov.save() diff --git a/music21/test/testSingleCoreAll.py b/music21/test/testSingleCoreAll.py index 6fd06d6dda..72c99d92f4 100644 --- a/music21/test/testSingleCoreAll.py +++ b/music21/test/testSingleCoreAll.py @@ -25,17 +25,11 @@ from music21 import environment from music21.test import commonTest -from music21.test import coverageM21 from music21.test import testRunner environLocal = environment.Environment('test.testSingleCoreAll') -# this is designed to be None for all but one system and a Coverage() object -# for one system. -cov = coverageM21.getCoverage() - - def main(testGroup: Sequence[str] = ('test',), restoreEnvironmentDefaults=False, limit: bool|None = None, @@ -118,8 +112,6 @@ def main(testGroup: Sequence[str] = ('test',), runner = unittest.TextTestRunner(verbosity=verbosity) finalTestResults = runner.run(s1) - coverageM21.stopCoverage(cov) - if (finalTestResults.errors or finalTestResults.failures or finalTestResults.unexpectedSuccesses): @@ -136,17 +128,16 @@ def ciMain(): # and TestExternal (without doctests) with show=False # exits with the aggregated returnCode returnCodeTest = main(testGroup=('test',), verbosity=1) - # restart coverage if running main() twice - coverageM21.startCoverage(cov) returnCodeExternal = main(testGroup=('external',), verbosity=1, show=False) sys.exit(returnCodeTest + returnCodeExternal) # ------------------------------------------------------------------------------ if __name__ == '__main__': - # if optional command line arguments are given, assume they are - # test group arguments - if len(sys.argv) >= 2: + # 'ci' runs what GitHub Actions runs; other arguments are test group names. + if sys.argv[1:2] == ['ci']: + ciMain() + elif len(sys.argv) >= 2: unused_returnCode = main(sys.argv[1:]) else: unused_returnCode = main() From 19cee5c618f21e28728f0f260a012199eaaaf72a Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Fri, 28 Aug 2026 11:36:06 -1000 Subject: [PATCH 3/4] Flatten the __main__ dispatch in testSingleCoreAll sys.argv[1:2] == ['ci'] was a slice used only to dodge IndexError on the no-argument run. Test the length first and the rest reads straight through. Drop unused_returnCode with it; nothing consumed the value. AI-assisted (Claude) --- music21/test/testSingleCoreAll.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/music21/test/testSingleCoreAll.py b/music21/test/testSingleCoreAll.py index 72c99d92f4..547416de2a 100644 --- a/music21/test/testSingleCoreAll.py +++ b/music21/test/testSingleCoreAll.py @@ -135,10 +135,10 @@ def ciMain(): # ------------------------------------------------------------------------------ if __name__ == '__main__': # 'ci' runs what GitHub Actions runs; other arguments are test group names. - if sys.argv[1:2] == ['ci']: + if len(sys.argv) < 2: + main() + elif sys.argv[1] == 'ci': ciMain() - elif len(sys.argv) >= 2: - unused_returnCode = main(sys.argv[1:]) else: - unused_returnCode = main() + main(sys.argv[1:]) From f597000662d38cd02c63a48a6ff63f6117954974 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Fri, 28 Aug 2026 11:37:52 -1000 Subject: [PATCH 4/4] Name the coverage Python version once PY_VERSION_WITH_COVERAGE at workflow level; the three step conditions read it from the env context. AI-assisted (Claude) --- .github/workflows/maincheck.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/maincheck.yml b/.github/workflows/maincheck.yml index 0537f2c6f1..45ece16361 100644 --- a/.github/workflows/maincheck.yml +++ b/.github/workflows/maincheck.yml @@ -9,6 +9,11 @@ on: branches: - '*' +env: + # We run coverage on a middle supported Python so that failures on newest and + # oldest versions return first. + PY_VERSION_WITH_COVERAGE: '3.13' + jobs: run_tests: runs-on: ubuntu-latest @@ -35,18 +40,13 @@ jobs: - name: Setup Lilypond run: uv run python -c 'from music21 import environment; environment.UserSettings()["lilypondPath"] = "/home/runner/bin/lilypond"' - name: Run Main Test script - if: ${{ matrix.python-version != '3.13' }} + if: ${{ matrix.python-version != env.PY_VERSION_WITH_COVERAGE }} run: uv run python -c 'from music21.test.testSingleCoreAll import ciMain as ci; ci()' - # Coverage has to wrap the whole process: anything imported from inside the - # music21 package imports music21 first, and by then every class body and def - # has already run and would read as uncovered. Run on a MIDDLE supported - # Python, so failures on the newest and oldest stay quick to see. - # When changing the version, change the two conditions below it as well. - name: Run Main Test script with coverage - if: ${{ matrix.python-version == '3.13' }} + if: ${{ matrix.python-version == env.PY_VERSION_WITH_COVERAGE }} run: uv run coverage run -m music21.test.testSingleCoreAll ci - name: Coveralls - if: ${{ matrix.python-version == '3.13' }} + if: ${{ matrix.python-version == env.PY_VERSION_WITH_COVERAGE }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} COVERALLS_SERVICE_NAME: github