From d3e222de22e3599523be42a6d3eddd4a7e5a6c0e Mon Sep 17 00:00:00 2001 From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:11:03 -0500 Subject: [PATCH] CI: a push to main runs what it changes; everything weekly A push to main now runs the tests its own change can move, from the commit main moved from (github.event.before), the same way a pull request does against its base. A shared change still runs everything, every class and every node; so do a manual run and a new weekly run on Monday. tools/scoped_tests.py --ci also maps an instrument module to the tests that name it and the ones that go through every instrument, a helper in tests/ or tests/support/ to the tests that import it, and an effect to the tests that name it or go through every effect, where each used to run the whole suite or only the class's own tests. --- .github/workflows/tests.yml | 135 ++++++++++++++++---------- CONTRIBUTING.md | 8 ++ tests/support/KIT.md | 13 ++- tests/test_lifecycle_matrix.py | 5 +- tests/test_node_matrix.py | 5 +- tools/scoped_tests.py | 167 +++++++++++++++++++++++++++++++-- 6 files changed, 262 insertions(+), 71 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3a5ffcb2..c0466b52 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -6,7 +6,8 @@ # otherwise would turn a real gate into a skipped one. See AGENTS.md. name: Tests -# Once per change: pushes to main and pull requests. Documentation-only changes +# Once per change: pushes to main and pull requests, each checked for what it +# changes (tools/scoped_tests.py --ci). Documentation-only changes # skip it; the two package READMEs under lib/ are not skipped, because tests # read them and the wheels carry them. So does a release pull request, which # changes only VERSION and CHANGELOG.md, and its merge: the code it releases @@ -15,20 +16,25 @@ name: Tests on: push: branches: [main] - paths-ignore: ['docs/**', 'README.md', 'ROADMAP.md', 'CHANGELOG.md', 'AGENTS.md', 'LICENSE', 'VERSION'] + paths-ignore: ['docs/**', 'README.md', 'ROADMAP.md', 'CHANGELOG.md', 'AGENTS.md', 'CLAUDE.md', 'CONTRIBUTING.md', 'LICENSE', 'VERSION'] pull_request: # `labeled` too: adding the `lifecycle: all classes` label re-runs the # pull request with the whole lifecycle matrix (see the test step). types: [opened, synchronize, reopened, labeled] - paths-ignore: ['docs/**', 'README.md', 'ROADMAP.md', 'CHANGELOG.md', 'AGENTS.md', 'LICENSE', 'VERSION'] + paths-ignore: ['docs/**', 'README.md', 'ROADMAP.md', 'CHANGELOG.md', 'AGENTS.md', 'CLAUDE.md', 'CONTRIBUTING.md', 'LICENSE', 'VERSION'] # The full matrix on demand, for a branch that should see every Python # before it merges rather than after. workflow_dispatch: + # Everything, every Python, every class and node, once a week: the scoped + # runs above trust a mapping from files to tests, and this is the run that + # would catch a dependency the mapping does not know about. + schedule: + - cron: '23 6 * * 1' # A new push to a pull request cancels the run it supersedes: only the newest # commit's result is ever read. Pushes to main are never cancelled or queued # behind one another (each gets a group of its own), so every commit on main -# keeps its own full-matrix record. +# keeps its own record. concurrency: group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} @@ -39,8 +45,8 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest] - # Every supported Python on main and on a manual run; on a pull - # request only 3.13, the Python the other two platforms run. A failure + # Every supported Python on main, on a manual run and weekly; on a + # pull request only 3.13, the Python the other two platforms run. A failure # only another Python shows surfaces on the push to main, or sooner # from a manual run on the branch. python: ${{ fromJSON(github.event_name == 'pull_request' && '["3.13"]' || '["3.11", "3.12", "3.13", "3.14"]') }} @@ -53,30 +59,15 @@ jobs: - os: macos-latest python: "3.13" runs-on: ${{ matrix.os }} - env: - # The lifecycle matrices (quick mode). All 45 classes take about 4.4 - # CPU hours, over an hour on this runner, and the 13 audiodsp nodes - # under them half an hour more on one worker, so a pull request runs - # only what its change can move: the class or node whose own code - # changed, and when a shared file changed (the base class, the - # matrix, the pin) a fixed sample of each, nine classes in about 13 - # minutes and four nodes in about five (SAMPLE in - # tests/test_lifecycle_matrix.py, NODE_SAMPLE in - # tests/test_node_matrix.py). A push to main runs every class and - # every node, and so does a manual run (workflow_dispatch) or a pull - # request labelled `lifecycle: all classes`. - LIFECYCLE_CHANGED_FROM: "${{ github.event_name == 'pull_request' && !contains(github.event.pull_request.labels.*.name, 'lifecycle: all classes') && format('origin/{0}', github.base_ref) || '' }}" steps: - # A pull request fetches its history, so the matrices can ask git what - # the change touches against origin/ (LIFECYCLE_CHANGED_FROM). - # Written `!= ... && 1 || 0` because `== ... && 0 || 1` is always 1: - # 0 is false in an expression, so the `||` took over and every pull - # request checked out at depth 1. With no base to diff from, the - # matrix then fell back to all 45 classes, and a pull request ran - # 1 h 43 min. A base git cannot diff from is now an error instead. + # Full history, so the scope and the matrices can ask git what the + # change touches: against origin/ for a pull request, against + # the commit main moved from for a push. A base git cannot diff from + # is an error, not a silent fall back to everything (a pull request + # once ran 1 h 43 min that way, checked out at depth 1). - uses: actions/checkout@v7 with: - fetch-depth: ${{ github.event_name != 'pull_request' && 1 || 0 }} + fetch-depth: 0 - uses: actions/setup-python@v7 with: python-version: ${{ matrix.python }} @@ -141,6 +132,61 @@ jobs: # a test. The tests would generate them too; this makes it explicit. - run: python tools/effect_probes/make_probes.py --ensure + # What this run checks. A pull request: what it changes against its + # base branch. A push to main: what that push changes, from the commit + # main moved from (github.event.before), so a merge is checked for the + # whole pull request it brings in. Everything when that commit is + # missing (a new branch, a force push that dropped it), on a manual + # run and on the weekly run. + # + # The lifecycle matrices (quick mode). All 45 classes take about 4.4 + # CPU hours, over an hour on this runner, and the 13 audiodsp nodes + # under them half an hour more on one worker, so a change runs only + # what it can move: the class or node whose own code changed. When a + # shared file changed (the base class, the matrix, the pin), a pull + # request runs a fixed sample of each, nine classes in about 13 + # minutes and four nodes in about five (SAMPLE in + # tests/test_lifecycle_matrix.py, NODE_SAMPLE in + # tests/test_node_matrix.py), and its push to main runs every class + # and every node. So do a manual run (workflow_dispatch), the weekly + # run and a pull request labelled `lifecycle: all classes`. This + # step sets LIFECYCLE_CHANGED_FROM for them; empty means all. + - name: Scope + id: scope + shell: bash + env: + EVENT: ${{ github.event_name }} + BASE_REF: ${{ github.base_ref }} + BEFORE: ${{ github.event.before }} + ALL_CLASSES: "${{ contains(github.event.pull_request.labels.*.name, 'lifecycle: all classes') }}" + run: | + base= + if [[ "$EVENT" == pull_request ]]; then + base="origin/$BASE_REF" + elif [[ "$EVENT" == push && -n "$BEFORE" && ! "$BEFORE" =~ ^0+$ ]]; then + if git cat-file -e "$BEFORE^{commit}" 2>/dev/null; then + base="$BEFORE" + else + echo "the commit main moved from ($BEFORE) is not in the history: everything runs" + fi + fi + if [[ -z "$base" ]]; then + files=full + else + files=$(python tools/scoped_tests.py --ci "$base") + fi + # The matrices pick their own classes and nodes from the same base. + # A push whose change is shared runs all of them; a pull request + # runs the sample unless it is labelled for all. + lifecycle="$base" + if [[ "$EVENT" == push && "$files" == full ]]; then lifecycle=; fi + if [[ "$ALL_CLASSES" == true ]]; then lifecycle=; fi + echo "LIFECYCLE_CHANGED_FROM=$lifecycle" >> "$GITHUB_ENV" + if [[ "$files" == full ]]; then files=tests; fi + echo "files=$files" >> "$GITHUB_OUTPUT" + echo "base: ${base:-none, everything}" + echo "running: $files" + # What the two matrices will run, in the log where a reader can check # it, and a scope git cannot work out fails here by name. - name: Show the lifecycle scope @@ -152,31 +198,16 @@ jobs: print('classes:', ' '.join(classes()) or 'none'); print('nodes:', ' '.join(names()) or 'none')" - # A pull request runs only the tests its change can move: the test - # files it edits, the tests of each effect class whose module it edits, - # and the kit, the contract and the two lifecycle matrices, which - # every pull request runs. A change to shared code (the kit, the base - # class, tests/support, the probes, the pin, this workflow) or to a - # file with no tests of its own runs the whole suite. So do pushes to - # main and manual runs, on every Python. tools/scoped_tests.py --ci - # holds the mapping and prints why it chose what it chose. - - name: Choose the tests - id: scope - shell: bash - run: | - if [[ "${{ github.event_name }}" == pull_request ]]; then - files=$(python tools/scoped_tests.py --ci "origin/${{ github.base_ref }}") - else - files=full - fi - if [[ "$files" == full ]]; then - files=tests - fi - echo "files=$files" >> "$GITHUB_OUTPUT" - echo "running: $files" - - # The same tests `python -m unittest discover -s tests -p "test_*.py"` - # finds (or the pull request's share of them, above), spread across + # The tests the Scope step chose. A change runs the test files it + # edits, the tests of each effect or instrument whose module it edits + # (with the tests that name it and the ones that go through every + # class), the tests that import a helper it edits, and the kit, the + # contract and the two lifecycle matrices, which every change runs. A + # change to shared code (the kit, the base classes, the probes, the + # pin, this workflow) or to a file with no tests of its own runs the + # whole suite: the same tests `python -m unittest discover -s tests -p + # "test_*.py"` finds. tools/scoped_tests.py --ci holds the mapping and + # prints why it chose what it chose. They are spread across # every core the runner has: serially the suite # takes over an hour. A test that writes a file writes it into its own # temporary directory, so no two workers collide. loadscope keeps each diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a5dd4aad..348a776b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -53,6 +53,14 @@ python tests/parity/effects_library_smoke.py python -m flake8 ``` +CI doesn't run all of the first line on every change. It runs the tests +the change can move, chosen by `tools/scoped_tests.py --ci` from the diff: +for a pull request against its base, for a push to main against the commit +main moved from. A change to shared code (the base classes, the kit, the +probes, `AUDIODSP_PIN`, the workflow) runs everything. The run's "Scope" +step prints what it chose and why. The whole suite runs on every Python on +a manual run of the Tests workflow and every Monday. + Most work does not need the whole suite. [`tools/scoped_tests.py`](tools/scoped_tests.py) picks the files a change can break: diff --git a/tests/support/KIT.md b/tests/support/KIT.md index abb81ee8..f7933687 100644 --- a/tests/support/KIT.md +++ b/tests/support/KIT.md @@ -301,11 +301,14 @@ CI doesn't run all of it on every pull request: (`NODE_SAMPLE`) when it changes a shared file: `AUDIODSP_PIN`, the base class, `lifecycle.py`, the class matrix test, or a line of the adapters outside every node's own. -- A push to main runs all 45 classes and every node, and so does a manual - run of the workflow or a pull request labelled `lifecycle: all classes`. - -To tell what changed, CI diffs the pull request against its base branch, -so the checkout fetches the history. A base git can't diff from fails the +- A push to main runs what that push changes, the same way, except that a + shared file runs all 45 classes and every node rather than the sample. + So do a manual run of the workflow, the weekly run on Monday and a pull + request labelled `lifecycle: all classes`. + +To tell what changed, CI diffs a pull request against its base branch and +a push against the commit main moved from, so the checkout fetches the +history. A base git can't diff from fails the run by name rather than quietly running everything: a depth-1 checkout once did that, and a pull request's run took 1 h 43 min. The workflow prints what each matrix will run in its "Show the lifecycle scope" step. diff --git a/tests/test_lifecycle_matrix.py b/tests/test_lifecycle_matrix.py index d1ebeefc..9c2e61a9 100644 --- a/tests/test_lifecycle_matrix.py +++ b/tests/test_lifecycle_matrix.py @@ -265,8 +265,9 @@ def effect_class(name): #: (LowPass), Rack, Saturation (the recentre path) and Distortion (the graph #: rebuild). Measured on CPython at audiodsp v0.6.6rc8: 3 114 CPU seconds, #: about 13 minutes on a 4-core runner, where all 45 take 4.4 CPU hours. -#: A push to main runs all 45, and so does a manual run or a pull request -#: labelled `lifecycle: all classes` (.github/workflows/tests.yml). +#: A push to main that changes a shared file runs all 45, and so do a +#: manual run, the weekly run and a pull request labelled `lifecycle: all +#: classes` (.github/workflows/tests.yml). SAMPLE = ("Bitcrusher", "Tremolo", "DigitalDelay", "Reverb", "Octaver", "LowPass", "Rack", "Saturation", "Distortion") diff --git a/tests/test_node_matrix.py b/tests/test_node_matrix.py index eadfb93f..6867311c 100644 --- a/tests/test_node_matrix.py +++ b/tests/test_node_matrix.py @@ -103,8 +103,9 @@ #: (NodeSplitMix) and a free-running phase (NodeShimmerLoop, cheaper than #: NodeFlanger's LFO by minutes). Measured on CPython at audiodsp #: v0.6.6rc9: 86 s, where all 13 take 541 s (about 30 minutes on CI). -#: A push to main runs every node, and so does a manual run or a pull -#: request labelled `lifecycle: all classes` (.github/workflows/tests.yml). +#: A push to main that changes a shared file runs every node, and so do a +#: manual run, the weekly run and a pull request labelled `lifecycle: all +#: classes` (.github/workflows/tests.yml). NODE_SAMPLE = ("NodeBiquad", "NodeEcho", "NodeSplitMix", "NodeShimmerLoop") diff --git a/tools/scoped_tests.py b/tools/scoped_tests.py index dcdcb4d6..d26f08e1 100755 --- a/tools/scoped_tests.py +++ b/tools/scoped_tests.py @@ -13,6 +13,8 @@ tools/scoped_tests.py contract # the shared promises only tools/scoped_tests.py full # everything, for a gate tools/scoped_tests.py --ci origin/main # a pull request's tests, for CI + tools/scoped_tests.py --ci # a push's tests, from the commit + # it moved main from It prints the scope it chose and why before running, so a report can quote it. Exit status is the suite's own. `--ci` runs nothing: it prints the test files @@ -231,36 +233,140 @@ def choose(scope, modules): return sorted(picked), "; ".join(why[:4]) -#: What a pull request always runs, whatever it touched: the kit's tests, the +#: What CI always runs, whatever the change touched: the kit's tests, the #: shared promises, and the two lifecycle matrices, which pick their own #: classes and nodes from the same diff (LIFECYCLE_CHANGED_FROM). CI_ALWAYS = KIT + ("test_kit",) + CONTRACT + ( "test_lifecycle_matrix", "test_node_matrix") -#: Paths every test leans on. A pull request that touches one runs the whole +#: Paths every test leans on. A change that touches one runs the whole #: suite, because no mapping can say which tests it moved. CI_SHARED = ( ".github/", - "tests/support/", + "tests/support/ulab/", "tests/conftest.py", "tools/effect_measurements", "tools/effect_probes/", "tools/scoped_tests.py", "lib/audioeffects/_", "lib/audioeffects/__init__.py", + "lib/audioinstruments/_", + "lib/audioinstruments/__init__.py", "AUDIODSP_PIN", ) -#: The roster tests that name every effect class, run when one class moves. +#: The roster tests that name every class, run when one class moves. _ROSTERS = ("test_metadata_contract", "test_shared_circuit_overlap", - "test_cpython_effects_library") + "test_cpython_effects_library", "test_audio_component_api") #: Paths no test reads, besides the ones the workflow already ignores. _CI_SILENT = ("docs/", "LICENSE", "VERSION") +#: A docs page a test compares with the classes, so it is not silent. +_DOC_TESTS = {"docs/reference/effect-controls.md": "test_metadata_contract"} + +#: A test that goes through every effect class, or every instrument, runs +#: when any one of them changes. +_EVERY_EFFECT = re.compile(r"audioeffects\.ALL\b|\bkit_options\b|\bGATED\b") +_EVERY_INSTRUMENT = re.compile( + r"audioinstruments\.(ALL|MELODIC|DRUM_MACHINES)\b|" + r"from audioinstruments import [^\n]*\b(ALL|MELODIC|DRUM_MACHINES)\b") + + +def _source(rel): + try: + with open(os.path.join(ROOT, rel), encoding="utf-8", + errors="replace") as handle: + return handle.read() + except OSError: + return "" + + +def _helpers(): + """{stem: path} for the python files under tests/ that are not tests: + tests/support/*.py and the helpers beside the tests. Their names are + unique, because the tests import them bare (tests/support is on + sys.path), so a stem is all an import has to name.""" + out = {} + for folder in ("tests", "tests/support"): + for name in sorted(os.listdir(os.path.join(ROOT, folder))): + if name.endswith(".py") and not (folder == "tests" and + name.startswith("test_")): + out.setdefault(name[:-3], "%s/%s" % (folder, name)) + return out + + +def _imported_names(rel): + """Every name an import in `rel` mentions, dotted parts split, so that + `import kit_common as kc`, `from support import kit` and `from kit_probes + import x` all name the helper they load. Imports inside functions count.""" + import ast + try: + tree = ast.parse(_source(rel)) + except SyntaxError: + return None + names = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + names.update(alias.name.split(".")) + elif isinstance(node, ast.ImportFrom): + names.update((node.module or "").split(".")) + names.update(alias.name for alias in node.names) + return names + + +def _importers(changed_helpers, modules): + """The test modules that import any of `changed_helpers` (stems), directly + or through other helpers. None when a file cannot be parsed.""" + helpers = _helpers() + files = dict(("tests/%s.py" % m, m) for m in modules) + files.update((path, None) for path in helpers.values()) + imports = {} + for path in files: + names = _imported_names(path) + if names is None: + return None + imports[path] = names & set(helpers) + reached = set(changed_helpers) + grew = True + while grew: + grew = False + for path, names in imports.items(): + stem = os.path.basename(path)[:-3] + if files[path] is None and stem not in reached and \ + names & reached: + reached.add(stem) + grew = True + return sorted(m for path, m in files.items() + if m is not None and imports[path] & reached) + + +def _mentions(words, modules, pattern=None): + """The test modules whose text names one of `words`, or matches + `pattern`: a test that builds a class by its name reaches its module.""" + word = re.compile(r"\b(%s)\b" % "|".join(re.escape(w) for w in words)) + out = [] + for module in modules: + text = _source("tests/%s.py" % module) + if word.search(text) or (pattern and pattern.search(text)): + out.append(module) + return out + + +def _effect_classes(rel): + import ast + try: + tree = ast.parse(_source(rel)) + except SyntaxError: + return [] + return [n.name for n in tree.body + if isinstance(n, ast.ClassDef) and not n.name.startswith("_")] + def _changed_since(base): - """The files this branch changed since it left `base`.""" + """The files changed since `base`: a pull request's base branch, or the + commit a push to main moved from.""" done = subprocess.run(["git", "diff", "--name-only", base + "...HEAD"], cwd=ROOT, capture_output=True, text=True) if done.returncode: @@ -276,30 +382,71 @@ def choose_ci(changed, modules): narrower run that misses a break is worse than a slow one. """ picked, why = set(m for m in CI_ALWAYS if m in modules), [] + helpers = [] for path in changed: if path.startswith(CI_SHARED): return None, ["%s is shared, so the whole suite runs" % path] + if path in _DOC_TESTS: + if _DOC_TESTS[path] in modules: + picked.add(_DOC_TESTS[path]) + why.append("%s is checked by %s" % (path, _DOC_TESTS[path])) + continue if path.startswith(_CI_SILENT) or ( - path.endswith(".md") and not path.startswith("lib/")): + path.endswith(".md") and not path.startswith(("lib/", + "tests/"))): continue name = os.path.basename(path) - if path.startswith("tests/") and "/" not in path[6:] and \ - name.endswith(".py"): + top = "/" not in path[6:] + if path.startswith("tests/") and top and name.startswith("test_") \ + and name.endswith(".py"): if name[:-3] in modules: picked.add(name[:-3]) why.append("%s edited" % name[:-3]) continue + if name.endswith(".py") and ( + (path.startswith("tests/") and top) or + (path.startswith("tests/support/") and + "/" not in path[14:])): + helpers.append(name[:-3]) + continue if path.startswith("lib/audioeffects/") and \ "/" not in path[17:] and name.endswith(".py"): hits = _test_for_effect(name[:-3], modules) if hits and hits != ["test_cpython_effects_library"]: + words = [name[:-3]] + _effect_classes(path) + more = _mentions(words, modules, _EVERY_EFFECT) picked.update(hits) + picked.update(more) # The rosters that name every class read this one too. picked.update(m for m in _ROSTERS if m in modules) - why.append("%s -> %s" % (name[:-3], ", ".join(hits))) + why.append("%s -> %s, and %d tests that name it or go " + "through every effect" + % (name[:-3], ", ".join(hits), + len(set(more) - set(hits)))) + continue + if path.startswith("lib/audioinstruments/") and \ + "/" not in path[21:] and name.endswith(".py"): + hits = _mentions([name[:-3]], modules, _EVERY_INSTRUMENT) + if hits: + picked.update(hits) + picked.update(m for m in INSTRUMENTS + _ROSTERS + if m in modules) + why.append("%s -> %d tests that name it or go through " + "every instrument" % (name[:-3], len(hits))) continue return None, ["%s has no tests of its own, so the whole suite runs" % path] + if helpers: + users = _importers(helpers, modules) + if users is None: + return None, ["a test file does not parse, so the whole suite " + "runs"] + if not users: + return None, ["%s is imported by no test, so the whole suite " + "runs" % ", ".join(helpers)] + picked.update(users) + why.append("%s -> the %d tests that import it" + % (", ".join(helpers), len(users))) return sorted(picked), why or ["only the tests every change runs"]