diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3a5ffcb..c0466b5 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 a5dd4aa..348a776 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 abb81ee..f793368 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 d1ebeef..9c2e61a 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 eadfb93..6867311 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 dcdcb4d..d26f08e 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"]