diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 1aac173cfe..8e34c0076a 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -3,8 +3,19 @@ name: "CodeQL" on: push: branches: [main] + # #2243, the owner's option B: a docs-only or test-only pull request skips + # the scan. util/classify_changes.py's docs and tests, by docs.github.com's + # "Avoiding unnecessary scans of pull requests"; the config leaves tests/ out + # of the database anyway (#2130), so such a diff scans as its base does. A + # path filter rather than the `changes` job, as no ruleset requires a check + # of this workflow, so a run that never starts holds nothing pending. A + # push and the schedule have no filter. pull_request: branches: [main] + paths-ignore: + - 'docs/**' + - 'tests/**' + - '**.md' schedule: - cron: '0 2 * * 6' diff --git a/.github/workflows/deploy-pages.yml b/.github/workflows/deploy-pages.yml index 0d28a07997..6326187f66 100644 --- a/.github/workflows/deploy-pages.yml +++ b/.github/workflows/deploy-pages.yml @@ -15,8 +15,15 @@ on: # Vendor Update (#1634). workflows.rst gives the evidence. schedule: - cron: '0 2 * * 6' # Saturdays at 02:00 UTC + # #2243, the owner's option B: a test-only pull request, one of tests/ files + # alone, skips the docs build, which reads none of them; one that touches + # docs too still builds. A path filter rather than unit-tests.yml's + # `changes` job, as no ruleset requires a check of this workflow, so a run + # that never starts holds nothing pending. pull_request: branches: [main] + paths-ignore: + - 'tests/**' # A build-only run of the dispatched ref: `head`'s test gate and the Sphinx # build, whose site is kept as the `pages-html` artifact. Nothing is # published to gh-pages and no report is posted. diff --git a/.github/workflows/experimental.yml b/.github/workflows/experimental.yml index 505e6695b8..6db6ad135d 100644 --- a/.github/workflows/experimental.yml +++ b/.github/workflows/experimental.yml @@ -13,7 +13,8 @@ name: Experimental Tests # (experimental)` (#2060); no required context of ruleset 23497679 names one. # # The same events and `changes` job as unit-tests.yml, so a docs-only pull -# request skips the legs here as it skips `test` there. +# request skips the legs here as it skips `test` there, and a test-only one as +# it skips the KTLO jobs there (#2243). on: push: branches: [main] @@ -123,6 +124,7 @@ jobs: timeout-minutes: 5 outputs: code: ${{ steps.classify.outputs.code }} + test-only: ${{ steps.classify.outputs.test-only }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -163,7 +165,8 @@ jobs: # `changes` or `python-versions` still skips the legs. if: >- ${{ !cancelled() && needs.changes.result == 'success' && needs.python-versions.result == 'success' - && needs.current.outputs.current != 'false' && needs.changes.outputs.code != 'false' }} + && needs.current.outputs.current != 'false' && needs.changes.outputs.code != 'false' + && needs.changes.outputs.test-only != 'true' }} needs: [current, changes, python-versions] runs-on: ${{ matrix.os || 'ubuntu-latest' }} timeout-minutes: 140 @@ -626,7 +629,8 @@ jobs: name: Python ${{ needs.python-versions.outputs.latest }} on Windows (experimental) if: >- ${{ !cancelled() && needs.changes.result == 'success' && needs.python-versions.result == 'success' - && needs.current.outputs.current != 'false' && needs.changes.outputs.code != 'false' }} + && needs.current.outputs.current != 'false' && needs.changes.outputs.code != 'false' + && needs.changes.outputs.test-only != 'true' }} needs: [current, changes, python-versions] runs-on: windows-latest timeout-minutes: 80 diff --git a/.github/workflows/python-compatibility.yml b/.github/workflows/python-compatibility.yml index b5bdcd5e65..06405b820b 100644 --- a/.github/workflows/python-compatibility.yml +++ b/.github/workflows/python-compatibility.yml @@ -28,6 +28,7 @@ jobs: timeout-minutes: 5 outputs: compatibility: ${{ steps.read.outputs.compatibility }} + compatibility-supported: ${{ steps.read.outputs.compatibility-supported }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -42,6 +43,30 @@ jobs: id: read run: python3 util/python_versions.py --github-output + # #2243, the owner's option B: unit-tests.yml's `changes`, the same + # util/classify_changes.py step. This workflow is never called, so its own + # pull_request run is every pull_request run, and every other event, a push + # to main included, runs everything. `light` is `true` on a docs-only or a + # test-only diff, neither of which can change what this workflow checks: + # pcapkit/ and util/ compile and import as on the base branch. + changes: + name: Classify changed files + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + light: ${{ steps.classify.outputs.code == 'false' || steps.classify.outputs.test-only == 'true' }} + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 2 + + - name: Classify the diff + id: classify + env: + OWN_PR: ${{ github.event_name == 'pull_request' }} + run: python3 util/classify_changes.py + compatibility: # `Compat`, so `gh pr checks` cannot show two `Python 3.N` rows, this and # Unit Tests', with nothing to tell them apart. @@ -52,8 +77,19 @@ jobs: # as in unit-tests.yml's `test` job: job-level, a failed leg's check run # concludes `failure`, so every commit shows red (#1672). Nothing below # `requires-python`'s 3.8 runs (#2051). + # + # #2243: the supported legs are required contexts, so a docs-only or + # test-only pull request still runs them, by name, but they skip every + # step that sets up, installs or checks, and pass. A job-level `if:` would + # not do: a matrix job it skips reports one check under its unexpanded + # name, as Unit Tests' docs-only run 40238795866 did, which matches no + # required context, so each would stay pending. + # The KTLO legs, required by nothing, are left out of that matrix. + # `!cancelled()` replaces the implicit `success()`, so a failed `changes`, + # whose `light` is then empty, runs every leg in full. name: Compat Python ${{ matrix.python-version }}${{ matrix.ktlo && ' (KTLO)' || '' }} - needs: [python-versions] + needs: [python-versions, changes] + if: ${{ !cancelled() && needs.python-versions.result == 'success' }} runs-on: ${{ matrix.os || 'ubuntu-latest' }} env: ANNOTATION_LEVEL: ${{ matrix.ktlo && 'warning' || 'error' }} @@ -63,14 +99,24 @@ jobs: # the `os` pins: a KTLO leg carries `ktlo: true`, and `os` where # `ubuntu-latest` has no setup-python build for it. A tier change edits # ruleset 23497679 too. - matrix: ${{ fromJSON(needs.python-versions.outputs.compatibility) }} + matrix: ${{ fromJSON(needs.changes.outputs.light == 'true' && needs.python-versions.outputs.compatibility-supported || needs.python-versions.outputs.compatibility) }} steps: + - name: Skip on a docs-only or test-only diff + if: ${{ needs.changes.outputs.light == 'true' }} + env: + LEG: Python ${{ matrix.python-version }} + run: echo "::notice title=Nothing to check on ${LEG}::A docs-only or test-only diff leaves pcapkit/ and util/ as on the base branch, so this leg passes without checking them (#2243)." + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 id: checkout + if: ${{ needs.changes.outputs.light != 'true' }} continue-on-error: ${{ matrix.ktlo == true }} + # Every later step waits on this one's `outcome`, so on a light diff, + # where it skips, they skip too. - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 id: python + if: ${{ needs.changes.outputs.light != 'true' }} continue-on-error: ${{ matrix.ktlo == true }} with: python-version: ${{ matrix.python-version }} diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 6045b7150c..cfcefd735a 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -54,8 +54,10 @@ jobs: outputs: latest: ${{ steps.read.outputs.latest }} test: ${{ steps.read.outputs.test }} + test-latest: ${{ steps.read.outputs.test-latest }} test-ktlo: ${{ steps.read.outputs.test-ktlo }} integration: ${{ steps.read.outputs.integration }} + integration-latest: ${{ steps.read.outputs.integration-latest }} integration-ktlo: ${{ steps.read.outputs.integration-ktlo }} pypcap-parity: ${{ steps.read.outputs.pypcap-parity }} pypcap-parity-ktlo: ${{ steps.read.outputs.pypcap-parity-ktlo }} @@ -101,7 +103,11 @@ jobs: # `include` marks one leg `coverage: true`, `latest` (#1661), so coverage # adds no leg to the queue. Uncovered, 3.15's tests step took a median # 8.1 minutes and coverage cost about 1.8x, which fits the job's 45. - matrix: ${{ fromJSON(needs.python-versions.outputs.test) }} + # + # #2243, the owner's option B: a test-only pull request runs the + # `latest` leg alone, the coverage leg, which runs the whole suite with + # `integration`'s `latest` leg and `project-tests`. See `required-checks`. + matrix: ${{ fromJSON(needs.changes.outputs.test-only == 'true' && needs.python-versions.outputs.test-latest || needs.python-versions.outputs.test) }} # `&test-steps`: `test-ktlo` runs these same steps, by YAML alias. steps: &test-steps @@ -343,7 +349,7 @@ jobs: # leg before any step runs, which only the job-level guard contains. test-ktlo: name: Python ${{ matrix.python-version }} (KTLO) - if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' }} + if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' && needs.changes.outputs.test-only != 'true' }} needs: [changes, python-versions] runs-on: ${{ matrix.os || 'ubuntu-latest' }} timeout-minutes: 45 @@ -367,8 +373,9 @@ jobs: strategy: fail-fast: false # The supported tier of the `test` job above, without its coverage leg; - # the KTLO legs are `integration-ktlo`'s (#1738). - matrix: ${{ fromJSON(needs.python-versions.outputs.integration) }} + # the KTLO legs are `integration-ktlo`'s (#1738). On a test-only pull + # request, the `latest` leg alone, as in `test` (#2243). + matrix: ${{ fromJSON(needs.changes.outputs.test-only == 'true' && needs.python-versions.outputs.integration-latest || needs.python-versions.outputs.integration) }} # `&integration-steps`: `integration-ktlo` runs these same steps. steps: &integration-steps @@ -513,7 +520,7 @@ jobs: # `integration`'s KTLO legs, apart for the reasons on `test-ktlo` (#1738). integration-ktlo: name: Integration Python ${{ matrix.python-version }} (KTLO) - if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' }} + if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' && needs.changes.outputs.test-only != 'true' }} needs: [changes, python-versions] runs-on: ${{ matrix.os || 'ubuntu-latest' }} timeout-minutes: 45 @@ -562,7 +569,7 @@ jobs: # run only what `test` runs. engine-tests: name: Engines (${{ matrix.engine }}) - if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' }} + if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' && needs.changes.outputs.test-only != 'true' }} needs: [changes, python-versions] runs-on: ubuntu-latest timeout-minutes: 30 @@ -900,7 +907,7 @@ jobs: # `pypcap-parity-ktlo`'s (#1738). pypcap-parity: name: PyPCAP/PyPCAPFile parity Python ${{ matrix.python-version }} - if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' }} + if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' && needs.changes.outputs.test-only != 'true' }} needs: [changes, python-versions] runs-on: ubuntu-latest timeout-minutes: 45 @@ -1087,7 +1094,7 @@ jobs: # Same apt set and cache key; `pypcap-parity`'s supported leg saves it. pypcap-parity-ktlo: name: PyPCAP/PyPCAPFile parity Python ${{ matrix.python-version }} (KTLO) - if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' }} + if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' && needs.changes.outputs.test-only != 'true' }} needs: [changes, python-versions] runs-on: ubuntu-latest timeout-minutes: 45 @@ -1245,8 +1252,13 @@ jobs: # Which of a pull request's changed files are code, which decides whether # the expensive legs run at all. util/classify_changes.py, which - # experimental.yml's `changes` runs too (#1967), fails safe: anything it - # cannot classify is code. + # experimental.yml's and python-compatibility.yml's `changes` run too + # (#1967), fails safe: anything it cannot classify is code. + # + # #2243: `test-only` is `true` on a diff of tests and docs alone. `test` and + # `integration` then run their `latest` leg alone, and the KTLO jobs, + # `engine-tests` and `pypcap-parity` skip by their own `if:`, as on a + # docs-only diff (`code: false`). # # Only this workflow's own `pull_request` run is classified; every other # event reports `code: true`. A `workflow_call` sees the caller's @@ -1260,6 +1272,7 @@ jobs: timeout-minutes: 5 outputs: code: ${{ steps.classify.outputs.code }} + test-only: ${{ steps.classify.outputs.test-only }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -1448,8 +1461,14 @@ jobs: # `if:`, and `project-tests` runs the docs readers as well. A job-level `if:` # rather than workflow-level `paths-ignore`, which would leave this required # context pending forever. So `skipped` is accepted for those legs **only** - # when `changes` succeeded and said `code == 'false'`; `project-tests` must - # have succeeded either way, and every other combination fails this gate. + # when `changes` succeeded and said `code == 'false'`. + # + # The test-only path (#2243). On one whose diff touches tests and docs alone, + # `test` and `integration` run their `latest` leg alone and must succeed, and + # `engine-tests` and `pypcap-parity` are skipped by their own `if:`. So + # `skipped` is accepted for those two **only** when `changes` succeeded and + # said `code == 'true'` and `test-only == 'true'`. `project-tests` must have + # succeeded on every path, and every other combination fails this gate. required-checks: name: Required checks passed needs: [changes, test, integration, engine-tests, pypcap-parity, project-tests] @@ -1462,6 +1481,7 @@ jobs: env: CHANGES: ${{ needs.changes.result }} CODE: ${{ needs.changes.outputs.code }} + TEST_ONLY: ${{ needs.changes.outputs.test-only }} TEST: ${{ needs.test.result }} INTEGRATION: ${{ needs.integration.result }} ENGINE_TESTS: ${{ needs.engine-tests.result }} @@ -1469,7 +1489,7 @@ jobs: PROJECT_TESTS: ${{ needs.project-tests.result }} run: | set -eu - echo "changes: $CHANGES (code: '$CODE')" + echo "changes: $CHANGES (code: '$CODE', test-only: '$TEST_ONLY')" echo "test: $TEST" echo "integration: $INTEGRATION" echo "engine-tests: $ENGINE_TESTS" @@ -1477,33 +1497,44 @@ jobs: echo "project-tests: $PROJECT_TESTS" failed=0 - if [ "$CHANGES" != "success" ] || { [ "$CODE" != "true" ] && [ "$CODE" != "false" ]; }; then - echo "::error title=changes did not classify the diff::result was '$CHANGES' and code was '$CODE'; without a classification nothing may be skipped." + # A docs-only diff is never test-only too, so that pair is garbled. + if [ "$CHANGES" != "success" ] || { [ "$CODE" != "true" ] && [ "$CODE" != "false" ]; } \ + || { [ "$TEST_ONLY" != "true" ] && [ "$TEST_ONLY" != "false" ]; } \ + || { [ "$CODE" = "false" ] && [ "$TEST_ONLY" = "true" ]; }; then + echo "::error title=changes did not classify the diff::result was '$CHANGES', code was '$CODE' and test-only was '$TEST_ONLY'; without a classification nothing may be skipped." CODE=true + TEST_ONLY=false failed=1 fi # project-tests runs the project pins on every diff (#1954), and the - # docs readers on a docs-only one, so it must pass on both paths. - # Docs-only (code == false): a leg may be skipped. Otherwise every leg - # must have succeeded. + # docs readers on a docs-only one, so it must pass on every path. + # Docs-only (code == false): any leg may be skipped. Test-only + # (test-only == true): engine-tests and pypcap-parity may be. Otherwise + # every leg must have succeeded. [ "$PROJECT_TESTS" = "success" ] || { echo "::error title=project-tests did not succeed::result was '$PROJECT_TESTS'; it runs the project pins, and on a docs-only diff the docs readers too, so it has to pass."; failed=1; } if [ "$CODE" = "false" ]; then - allowed="success skipped" + may_skip="test integration engine-tests pypcap-parity" + elif [ "$TEST_ONLY" = "true" ]; then + may_skip="engine-tests pypcap-parity" else - allowed="success" + may_skip="" fi for pair in "test=$TEST" "integration=$INTEGRATION" "engine-tests=$ENGINE_TESTS" "pypcap-parity=$PYPCAP_PARITY"; do job="${pair%%=*}" result="${pair#*=}" + case " $may_skip " in + *" $job "*) allowed="success skipped" ;; + *) allowed="success" ;; + esac case " $allowed " in *" $result "*) ;; - *) echo "::error title=$job did not succeed::result was '$result'; accepted here: $allowed (code: '$CODE')."; failed=1 ;; + *) echo "::error title=$job did not succeed::result was '$result'; accepted here: $allowed (code: '$CODE', test-only: '$TEST_ONLY')."; failed=1 ;; esac done if [ "$failed" -ne 0 ]; then - echo "::error title=Required checks did not all succeed::See the per-job lines above. A skipped or cancelled dependency is only a pass on a docs-only diff; see this job's own comment." + echo "::error title=Required checks did not all succeed::See the per-job lines above. A skipped dependency is a pass only on a docs-only or test-only diff, and a cancelled one never; see this job's own comment." exit 1 fi echo "All required jobs reported an accepted result." diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index 50e0212d20..f1c0be5996 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -814,6 +814,107 @@ permission to the reusable workflow. The 3.15 engine cells are not among the cuts: 3.15 is in the supported tier (:issue:`1633`), so its cells block on every run. +Light Pull Requests (:issue:`2243`) +----------------------------------------------------------------------------------------------------- + +A docs-only or test-only pull request runs only the jobs its files can affect, by the +owner's ruling on :issue:`2243`: a docs-only one the docs build and ``Lint``, a +test-only one the tests on ``latest`` and ``Lint``. A push to ``main``, the schedule +and every other event run everything, and ``Required checks passed`` stays required on +both paths. + +The ``changes`` job ("Classify changed files") of ``unit-tests.yml``, +``experimental.yml`` and ``python-compatibility.yml`` runs +:file:`util/classify_changes.py` in its ``Classify the diff`` step, on the workflow's +own ``pull_request`` run alone. It writes two outputs: + +* ``code: false`` -- **docs-only**: every changed file is under ``docs/`` or ends in + ``.md``, outside ``.github/``, ``pcapkit/``, ``tests/``, ``util/`` and + ``examples/``. +* ``test-only: true`` -- **test-only**: every changed file is docs or under ``tests/``, + and at least one is under ``tests/``. That takes in the support files every test + runs under, such as :file:`tests/conftest.py` and :file:`tests/_tiers.py`, since the + ``latest`` legs still run the whole suite. A workflow, a ``util/`` script or an + ``examples/`` file is never a test. + +Anything else, and a diff the script cannot list, is code: ``code: true`` and +``test-only: false``. A rename lists both of its paths, so moving a code file under +``docs/`` or ``tests/`` is code too. + +.. list-table:: + :header-rows: 1 + + * - Workflow and job + - Docs-only + - Test-only + * - ``unit-tests.yml``: ``test``, ``integration`` + - skipped + - the ``latest`` leg alone, the coverage leg included + * - ``unit-tests.yml``: ``test-ktlo``, ``integration-ktlo``, ``engine-tests``, + ``pypcap-parity``, ``pypcap-parity-ktlo`` + - skipped + - skipped + * - ``unit-tests.yml``: ``project-tests`` + - the project pins' plain-unittest run and the docs readers + - the project pins + * - ``experimental.yml``: ``experimental``, ``windows`` + - skipped + - skipped + * - ``python-compatibility.yml``: ``compatibility`` + - the supported legs, checking nothing + - the supported legs, checking nothing + * - ``codeql-analysis.yml`` + - not run + - not run + * - ``deploy-pages.yml`` (the docs build) + - runs + - not run, unless a docs file changed too + * - ``lint.yml`` + - runs + - runs + +Each job skips by its own ``if:``, so its check reads ``skipped``, never ``cancelled``, +which GitHub counts against the commit. ``test`` and ``integration`` pick their matrix +with ``fromJSON``, from :file:`util/python_versions.py`'s ``test-latest`` and +``integration-latest`` outputs on a test-only diff. ``Required checks passed`` +accepts ``skipped`` from ``test``, ``integration``, ``engine-tests`` and +``pypcap-parity`` only where ``changes`` succeeded and said docs-only, and from +``engine-tests`` and ``pypcap-parity`` only where it said test-only; a skipped +``test`` or ``integration`` fails a test-only diff, and ``project-tests`` must +succeed on every diff (see `Required Status Checks`_). + +The ``Compat Python`` legs are required contexts, so they cannot be skipped the same +way. A conditionally skipped job reports ``Success`` (docs.github.com, +`Troubleshooting required status checks +`__, +"Handling skipped but required checks"), but a matrix job skipped by a job-level +``if:`` expands no legs: it reports one check under its unexpanded name, such as +``Python ${{ matrix.python-version }}``, as the docs-only Unit Tests run +40238795866 shows. No required context matches that name, so each ``Compat`` context +would stay pending. Instead ``compatibility`` runs on a light diff with the matrix of +``python_versions.py``'s ``compatibility-supported`` output, its supported legs alone, +under their required names. Each leg runs only its ``Skip on a docs-only or test-only +diff`` notice and the job summary's steps, since its diff leaves ``pcapkit/`` and +``util/`` as on the base branch. The ``changes`` job there writes ``light``, which is +``true`` on either light diff, and ``compatibility``'s ``if:`` opens with +``!cancelled()``, so a failed ``changes`` runs every leg in full. + +``codeql-analysis.yml`` and ``deploy-pages.yml`` gate by a ``paths-ignore`` filter on +``pull_request`` instead, as no ruleset requires a check of theirs. A workflow a path +filter skips never starts, so "checks associated with that workflow will remain in a +'Pending' state" (`Workflow syntax +`__, +"Example: Including paths"), and the troubleshooting page's advice is to "avoid +requiring workflows that can be skipped". So no workflow emitting a required context +has one. CodeQL's filter is +docs.github.com's own recipe (`Avoiding unnecessary scans of pull requests +`__), +and its config leaves :file:`tests` out of the database anyway (see `CodeQL +Configuration (#2130)`_). The docs build reads nothing under :file:`tests`. +:file:`tests/project/test_light_pull_requests_2243.py` evaluates every gate above on +each kind of diff, and :file:`tests/project/test_unit_tests_queue.py` runs the +classifier and the ``required-checks`` step. + Required Status Checks ---------------------- @@ -872,8 +973,9 @@ required job succeeded" step. Rulesets match check names literally, with no wildcard, so requiring the matrix cells one by one would mean editing the ruleset whenever a matrix changes; the aggregate keeps the required list fixed. It accepts the gating jobs as ``skipped`` only on a docs-only pull request, where -``project-tests`` runs every test that reads ``docs/`` or Markdown as well; -``project-tests`` must pass on every diff. The ``Compat`` legs stay required separately because they come from +``project-tests`` runs every test that reads ``docs/`` or Markdown as well, and +``engine-tests`` and ``pypcap-parity`` only on a test-only one (see `Light Pull +Requests (#2243)`_); ``project-tests`` must pass on every diff. The ``Compat`` legs stay required separately because they come from ``python-compatibility.yml``, and a job cannot ``needs:`` a job in another workflow file. This job runs on Unit Tests' own ``push``/``pull_request`` triggers; the ``gate-only: true`` reusable calls documented above skip it via @@ -977,7 +1079,7 @@ Tests run completes without it, and with it ``Required checks passed``, ``unittest-ordering`` and the workflows that wait on Unit Tests completing (``coverage-comment.yml``, ``deploy-pages.yml``, ``cron-vendor.yml`` and ``cron-conda.yml``). It runs on Unit Tests' ``push`` and ``pull_request`` events, -skips a docs-only pull request by the same ``changes`` script, and lets a newer +skips a docs-only or test-only pull request by the same ``changes`` script, and lets a newer commit supersede a run, a pull request's or, unlike Unit Tests, a push to ``main``'s (:issue:`1903`): since nothing waits on the legs, ``main``'s newest commit is the one worth testing, whereas Unit @@ -1375,8 +1477,8 @@ to a script. Each script's docstring says what it does; each has tests under and ``windows``, as ``leg_summary.py experimental``. - No. * - :file:`util/classify_changes.py` - - ``Classify the diff`` in the ``changes`` job of ``unit-tests.yml`` and of - ``experimental.yml``. + - ``Classify the diff`` in the ``changes`` job of ``unit-tests.yml``, of + ``experimental.yml`` and of ``python-compatibility.yml``. - No. * - :file:`util/coverage_report.py` - ``Report coverage`` in ``unit-tests.yml``'s ``test``, on the coverage leg. diff --git a/tests/project/test_actions_helper_2047.py b/tests/project/test_actions_helper_2047.py index cd256d1c8e..906747328b 100644 --- a/tests/project/test_actions_helper_2047.py +++ b/tests/project/test_actions_helper_2047.py @@ -307,7 +307,7 @@ def test_run_as_the_steps_run_it(self) -> None: self.assertIn('_actions', modules) self.assertNotIn('util._actions', modules) if name == 'classify_changes': - self.assertEqual(output.read_text(encoding='utf-8'), 'code=true\n') + self.assertEqual(output.read_text(encoding='utf-8'), 'code=true\ntest-only=false\n') def sparse_checkouts_missing_the_helper(spec: 'Any') -> 'list[tuple[str, list[str]]]': diff --git a/tests/project/test_ktlo_jobs_gate_nothing_1738.py b/tests/project/test_ktlo_jobs_gate_nothing_1738.py index 30cc789232..196dc83a39 100644 --- a/tests/project/test_ktlo_jobs_gate_nothing_1738.py +++ b/tests/project/test_ktlo_jobs_gate_nothing_1738.py @@ -155,7 +155,10 @@ def test_each_runs_its_twins_steps_on_ktlo_legs_only(self) -> None: with self.subTest(job=twin): self.assertEqual(spec['steps'], gate['steps']) self.assertEqual(needs(spec), needs(gate)) - self.assertEqual(spec['if'], gate['if']) + # #2243: and skips a test-only pull request, which `test` and + # `integration` run on `latest` alone and `pypcap-parity` skips. + skip = " && needs.changes.outputs.test-only != 'true'" + self.assertEqual(spec['if'], gate['if'].replace(skip, '').replace(' }}', f'{skip} }}}}')) self.assertEqual((spec['runs-on'], spec['timeout-minutes']), (gate['runs-on'], gate['timeout-minutes'])) self.assertEqual(spec['env']['ANNOTATION_LEVEL'], gate['env']['ANNOTATION_LEVEL']) self.assertTrue(all(leg.get('ktlo') is True for leg in matrix_legs(twin))) diff --git a/tests/project/test_light_pull_requests_2243.py b/tests/project/test_light_pull_requests_2243.py new file mode 100644 index 0000000000..8b8d7e6b66 --- /dev/null +++ b/tests/project/test_light_pull_requests_2243.py @@ -0,0 +1,333 @@ +# -*- coding: utf-8 -*- +"""#2243, the owner's option B: docs-only and test-only pull requests skip the heavy matrices. + +:file:`util/classify_changes.py` writes ``code`` and ``test-only`` (its cases are +:mod:`tests.project.test_unit_tests_queue`'s), and each workflow that runs heavy +work on ``pull_request`` acts on them. Pinned here, by evaluating each job's and +step's ``if:`` and each matrix expression, rather than reading their text: + +* :file:`.github/workflows/unit-tests.yml`: ``test`` and ``integration`` run + their ``latest`` leg alone on a test-only diff and skip a docs-only one; the + KTLO jobs, ``engine-tests`` and ``pypcap-parity`` skip both; ``project-tests`` + runs on every diff. +* :file:`.github/workflows/experimental.yml`: its legs skip both. +* :file:`.github/workflows/python-compatibility.yml`: its required legs still + run, under their required names, on either light diff, and skip every step + but a notice and the job summary's; its KTLO legs are left out. A job-level + ``if:`` would leave each required context pending, since a matrix job it skips + reports one check under its unexpanded name. +* :file:`.github/workflows/codeql-analysis.yml` and + :file:`.github/workflows/deploy-pages.yml`, which no ruleset requires, skip + by a ``pull_request`` path filter. No workflow emitting a required context + has one, as a workflow a filter skips leaves its checks pending. +* A push to ``main``, and every other event, takes none of these paths. + +""" + +from __future__ import annotations + +import re +import unittest +from typing import TYPE_CHECKING + +from tests._workflows import WORKFLOWS +from tests.project.test_python_versions_1660 import REQUIRED_AT_MAIN, legs, live_outputs, matrix, render, workflow +from tests.project.test_release_gates import STATUS, calls_status_check, evaluate_condition, evaluate_expression + +if TYPE_CHECKING: + from typing import Any, Optional + +#: Test tier (#1958): reads the workflows and the tier file, a question about the tree. +__tier__ = 'LATEST' + +UNIT = 'unit-tests.yml' +COMPAT = 'python-compatibility.yml' +EXPERIMENTAL = 'experimental.yml' + +#: ``changes``' outputs for each kind of diff, and for a ``changes`` that wrote none. +CLASSES = { + 'code': {'code': 'true', 'test-only': 'false'}, + 'docs': {'code': 'false', 'test-only': 'false'}, + 'tests': {'code': 'true', 'test-only': 'true'}, + 'unclassified': {}, +} + +#: unit-tests.yml's jobs that run on a test-only diff, on their ``latest`` leg alone. +LATEST_ONLY = ('test', 'integration') +#: unit-tests.yml's jobs that skip both light diffs. +HEAVY = ('test-ktlo', 'integration-ktlo', 'engine-tests', 'pypcap-parity', 'pypcap-parity-ktlo') + +#: The workflows emitting a required context of ruleset 23497679, and the job that does. +REQUIRED_WORKFLOWS = {'lint.yml': 'lint', UNIT: 'required-checks', COMPAT: 'compatibility'} +#: The workflows a ``pull_request`` path filter gates instead, each with its filter. +FILTERED = {'codeql-analysis.yml': ['docs/**', 'tests/**', '**.md'], 'deploy-pages.yml': ['tests/**']} + + +def needs_context(name: 'str', job_id: 'str', outputs: 'dict[str, str]', + results: 'Optional[dict[str, str]]' = None) -> 'dict[str, Any]': + """``needs`` for job ``job_id``: ``changes`` with ``outputs``, ``python-versions`` with the tier file's.""" + results = results or {} + spec = workflow(name)['jobs'][job_id] + found = {} # type: dict[str, Any] + needs = spec.get('needs', []) + for target in [needs] if isinstance(needs, str) else needs: + found[target] = {'result': results.get(target, 'success'), 'outputs': {}} + if 'changes' in found: + found['changes']['outputs'] = dict(outputs) + if 'python-versions' in found: + found['python-versions']['outputs'] = live_outputs() + if 'current' in found: + found['current']['outputs'] = {'current': 'true'} + return found + + +def job_runs(name: 'str', job_id: 'str', outputs: 'dict[str, str]', + results: 'Optional[dict[str, str]]' = None) -> 'bool': + """Whether job ``job_id`` runs, GitHub's implicit ``success()`` applied where its ``if:`` calls none.""" + spec = workflow(name)['jobs'][job_id] + needs = needs_context(name, job_id, outputs, results) + succeeded = all(entry['result'] == 'success' for entry in needs.values()) + context = {'needs': needs, 'inputs': {}, 'github': {'event_name': 'pull_request'}, + STATUS: {'cancelled': False, 'always': True, 'success': succeeded, + 'failure': any(entry['result'] == 'failure' for entry in needs.values())}} + condition = str(spec.get('if', '')) + runs = evaluate_condition(condition, context) if condition else True + return runs if calls_status_check(condition) else runs and succeeded + + +def matrix_legs(name: 'str', job_id: 'str', outputs: 'dict[str, str]') -> 'list[dict[str, Any]]': + """The legs job ``job_id``'s matrix expression expands to, evaluated as GitHub would.""" + raw = workflow(name)['jobs'][job_id]['strategy']['matrix'] + found = evaluate_expression(raw, {'needs': needs_context(name, job_id, outputs)}) + assert isinstance(found, dict), f'{name} {job_id} matrix evaluates to {found!r}' + return legs(found) + + +class TestUnitTests(unittest.TestCase): + """unit-tests.yml's jobs on each kind of diff.""" + + def test_the_heavy_jobs_skip_both_light_diffs(self) -> None: + for job_id in HEAVY: + for kind, outputs in CLASSES.items(): + with self.subTest(job=job_id, diff=kind): + self.assertEqual(job_runs(UNIT, job_id, outputs), kind in ('code', 'unclassified')) + + def test_test_and_integration_skip_only_a_docs_only_diff(self) -> None: + for job_id in LATEST_ONLY: + for kind, outputs in CLASSES.items(): + with self.subTest(job=job_id, diff=kind): + self.assertEqual(job_runs(UNIT, job_id, outputs), kind != 'docs') + + def test_a_test_only_diff_runs_their_latest_leg_alone(self) -> None: + """The coverage leg on ``latest``, as the full matrix spells it, and no other.""" + latest = live_outputs()['latest'] + for job_id in LATEST_ONLY: + with self.subTest(job=job_id): + full = matrix_legs(UNIT, job_id, CLASSES['code']) + self.assertEqual(matrix_legs(UNIT, job_id, CLASSES['tests']), + [leg for leg in full if leg['python-version'] == latest]) + self.assertEqual(len(matrix_legs(UNIT, job_id, CLASSES['tests'])), 1) + self.assertEqual(matrix_legs(UNIT, job_id, CLASSES['tests']), legs(matrix(UNIT, job_id, light=True))) + self.assertGreater(len(full), 1) + self.assertEqual(matrix_legs(UNIT, 'test', CLASSES['tests']), + [{'python-version': latest, 'coverage': True}]) + + def test_every_other_diff_runs_the_full_matrix(self) -> None: + for job_id in LATEST_ONLY: + for kind in ('code', 'unclassified'): + with self.subTest(job=job_id, diff=kind): + self.assertEqual(matrix_legs(UNIT, job_id, CLASSES[kind]), legs(matrix(UNIT, job_id))) + + def test_project_tests_runs_on_every_diff(self) -> None: + for kind, outputs in CLASSES.items(): + with self.subTest(diff=kind): + self.assertTrue(job_runs(UNIT, 'project-tests', outputs)) + + def test_the_gate_still_runs_whatever_its_needs_did(self) -> None: + """``required-checks`` runs on ``always()``, so a skipped or failed leg is judged, not inherited.""" + for kind, outputs in CLASSES.items(): + for result in ('success', 'skipped', 'failure'): + with self.subTest(diff=kind, result=result): + self.assertTrue(job_runs(UNIT, 'required-checks', outputs, {'test': result})) + + +class TestExperimental(unittest.TestCase): + """experimental.yml's legs skip both light diffs.""" + + def test_the_legs_skip_both_light_diffs(self) -> None: + for job_id in ('experimental', 'windows'): + for kind, outputs in CLASSES.items(): + with self.subTest(job=job_id, diff=kind): + self.assertEqual(job_runs(EXPERIMENTAL, job_id, outputs), kind in ('code', 'unclassified')) + + def test_changes_writes_test_only(self) -> None: + self.assertEqual(workflow(EXPERIMENTAL)['jobs']['changes']['outputs']['test-only'], + '${{ steps.classify.outputs.test-only }}') + + +def light(outputs: 'dict[str, str]') -> 'str': + """python-compatibility.yml's ``changes`` output ``light``, as its expression evaluates on ``outputs``.""" + expression = workflow(COMPAT)['jobs']['changes']['outputs']['light'] + value = evaluate_expression(expression, {'steps': {'classify': {'outputs': dict(outputs)}}}) + return 'true' if value is True else 'false' if value is False else str(value) + + +def run_steps(leg: 'dict[str, Any]', light_output: 'str') -> 'list[str]': + """The names of the ``compatibility`` steps that run on ``leg`` with ``light`` set, none failing.""" + ran = [] + steps = {} # type: dict[str, dict[str, Any]] + for step in workflow(COMPAT)['jobs']['compatibility']['steps']: + context = {'matrix': leg, 'steps': steps, 'env': {}, + 'needs': {'changes': {'outputs': {'light': light_output}}}, + STATUS: {'success': True, 'failure': False, 'cancelled': False, 'always': True}} + condition = str(step.get('if', '')) + runs = evaluate_condition(condition, context) if condition else True + label = str(step.get('name') or step['uses'].split('@', 1)[0]) + if runs: + ran.append(label) + if 'id' in step: + steps[step['id']] = {'outcome': 'success' if runs else 'skipped', 'outputs': {}} + return ran + + +#: The steps a light diff's ``Compat`` leg runs: a notice, and the job summary's pair. +LIGHT_STEPS = ['Skip on a docs-only or test-only diff', 'Name the job summary', 'Upload the job summary'] + + +class TestCompatibility(unittest.TestCase): + """python-compatibility.yml's legs on each kind of diff.""" + + def test_light_is_true_on_either_light_diff_alone(self) -> None: + expected = {'code': 'false', 'docs': 'true', 'tests': 'true', 'unclassified': 'false'} + for kind, outputs in CLASSES.items(): + with self.subTest(diff=kind): + self.assertEqual(light(outputs), expected[kind]) + + def test_a_light_diff_still_runs_every_required_leg_by_name(self) -> None: + """Each required ``Compat`` context is a leg that runs; a skipped matrix job would report none.""" + template = workflow(COMPAT)['jobs']['compatibility']['name'] + required = REQUIRED_AT_MAIN - {'Required checks passed'} + for kind in ('docs', 'tests'): + with self.subTest(diff=kind): + found = matrix_legs(COMPAT, 'compatibility', {'light': light(CLASSES[kind])}) + self.assertEqual({render(template, leg) for leg in found}, required) + self.assertFalse([leg for leg in found if leg.get('ktlo')]) + self.assertTrue(job_runs(COMPAT, 'compatibility', {'light': light(CLASSES[kind])})) + + def test_every_other_diff_runs_both_tiers(self) -> None: + for kind in ('code', 'unclassified'): + with self.subTest(diff=kind): + found = matrix_legs(COMPAT, 'compatibility', {'light': light(CLASSES[kind])}) + self.assertEqual(found, legs(matrix(COMPAT, 'compatibility'))) + self.assertTrue([leg for leg in found if leg.get('ktlo')]) + + def test_a_light_leg_checks_nothing_and_passes(self) -> None: + for leg in matrix_legs(COMPAT, 'compatibility', {'light': 'true'}): + with self.subTest(leg=leg['python-version']): + self.assertEqual(run_steps(leg, 'true'), LIGHT_STEPS) + + def test_every_other_leg_checks_and_never_takes_the_notice(self) -> None: + for value in ('false', ''): + for leg in matrix_legs(COMPAT, 'compatibility', {'light': value}): + with self.subTest(light=value, leg=leg['python-version']): + ran = run_steps(leg, value) + self.assertNotIn(LIGHT_STEPS[0], ran) + self.assertIn('Verify package', ran) + + def test_a_failed_classification_runs_every_leg_in_full(self) -> None: + """``!cancelled()`` replaces the implicit ``success()``: a failed ``changes`` skips nothing.""" + self.assertTrue(job_runs(COMPAT, 'compatibility', {}, {'changes': 'failure'})) + self.assertEqual(matrix_legs(COMPAT, 'compatibility', {}), legs(matrix(COMPAT, 'compatibility'))) + # And without the tier file's matrix there is nothing to expand. + self.assertFalse(job_runs(COMPAT, 'compatibility', {}, {'python-versions': 'failure'})) + + def test_changes_classifies_this_workflows_own_pull_request(self) -> None: + step = workflow(COMPAT)['jobs']['changes']['steps'][1] + self.assertEqual(step['run'], 'python3 util/classify_changes.py') + self.assertEqual(step['env']['OWN_PR'], "${{ github.event_name == 'pull_request' }}") + + +class TestPathFilters(unittest.TestCase): + """Path filters gate only workflows no ruleset requires, and only on ``pull_request``.""" + + @staticmethod + def triggers(name: 'str') -> 'dict[str, Any]': + found = workflow(name)['on'] + return {key: value or {} for key, value in found.items()} if isinstance(found, dict) else {} + + def test_no_required_workflow_has_a_path_filter(self) -> None: + """docs.github.com, "Handling skipped but required checks": a workflow a path filter skips leaves + its checks pending, so a pull request that requires them cannot merge.""" + names = {'lint': 'Lint', 'required-checks': 'Required checks passed'} + for name, job_id in REQUIRED_WORKFLOWS.items(): + spec = workflow(name)['jobs'][job_id] + self.assertTrue(spec['name'].startswith(names.get(job_id, 'Compat Python '))) + for event, body in self.triggers(name).items(): + with self.subTest(workflow=name, event=event): + self.assertFalse({'paths', 'paths-ignore'} & set(body if isinstance(body, dict) else ())) + + def test_the_filtered_workflows_filter_pull_requests_alone(self) -> None: + for name, expected in FILTERED.items(): + for event, body in self.triggers(name).items(): + with self.subTest(workflow=name, event=event): + keys = set(body) if isinstance(body, dict) else set() + if event == 'pull_request': + self.assertEqual(body.get('paths-ignore'), expected) + self.assertNotIn('paths', keys) + else: + self.assertFalse({'paths', 'paths-ignore'} & keys) + + def test_no_other_workflow_has_a_path_filter(self) -> None: + for path in sorted(WORKFLOWS.glob('*.yml')): + if path.name in FILTERED: + continue + for event, body in self.triggers(path.name).items(): + with self.subTest(workflow=path.name, event=event): + self.assertFalse({'paths', 'paths-ignore'} & set(body if isinstance(body, dict) else ())) + + def test_the_filters_match_light_paths_and_spare_code(self) -> None: + """GitHub's glob: ``**`` crosses ``/``, ``*`` does not (Workflow syntax, "Filter pattern cheat sheet").""" + def matches(pattern: 'str', path: 'str') -> 'bool': + body = re.escape(pattern).replace(r'\*\*', '.*').replace(r'\*', '[^/]*') + return re.fullmatch(body, path) is not None + + light_paths = {'codeql-analysis.yml': ('docs/source/index.rst', 'README.md', 'tests/test_x.py', + 'tests/project/fixtures/x.yml'), + 'deploy-pages.yml': ('tests/test_x.py', 'tests/conftest.py')} + code_paths = ('pcapkit/__init__.py', 'util/classify_changes.py', '.github/workflows/unit-tests.yml', + 'pyproject.toml', 'examples/captures/in.pcap') + for name, patterns in FILTERED.items(): + for path in light_paths[name]: + with self.subTest(workflow=name, path=path): + self.assertTrue(any(matches(pattern, path) for pattern in patterns)) + for path in code_paths + (('docs/source/index.rst', 'README.md') if name == 'deploy-pages.yml' else ()): + with self.subTest(workflow=name, path=path): + self.assertFalse(any(matches(pattern, path) for pattern in patterns)) + + +class TestPushesTakeNoLightPath(unittest.TestCase): + """A push to ``main``, and every event but the workflow's own pull request, classifies as code.""" + + def test_every_changes_job_classifies_only_its_own_pull_request(self) -> None: + for name in (UNIT, EXPERIMENTAL, COMPAT): + step = next(entry for entry in workflow(name)['jobs']['changes']['steps'] if entry.get('id') == 'classify') + expression = step['env']['OWN_PR'] + workflow_name = workflow(name)['name'] + for event in ('push', 'schedule', 'workflow_dispatch', 'workflow_run', 'pull_request_target'): + with self.subTest(workflow=name, event=event): + context = {'github': {'event_name': event, 'workflow': workflow_name}} + self.assertFalse(evaluate_expression(expression, context)) + with self.subTest(workflow=name, event='pull_request'): + context = {'github': {'event_name': 'pull_request', 'workflow': workflow_name}} + self.assertTrue(evaluate_expression(expression, context)) + + def test_a_push_runs_every_leg(self) -> None: + """Pushes classify as code, which is the full matrix of every job here.""" + for job_id in LATEST_ONLY + HEAVY + ('project-tests',): + with self.subTest(job=job_id): + self.assertTrue(job_runs(UNIT, job_id, CLASSES['code'])) + self.assertEqual(light(CLASSES['code']), 'false') + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/project/test_python_versions_1660.py b/tests/project/test_python_versions_1660.py index 40acdbf032..98dc1bb049 100644 --- a/tests/project/test_python_versions_1660.py +++ b/tests/project/test_python_versions_1660.py @@ -177,9 +177,31 @@ def resolve(name: 'str', job_id: 'str', text: 'Any', values: 'Optional[dict[str, return json.loads(values[output]) if match.group(1) else values[output] -def matrix(name: 'str', job_id: 'str', values: 'Optional[dict[str, str]]' = None) -> 'dict[str, Any]': - """Job ``job_id``'s ``strategy.matrix``, each expression read from the script.""" +#: #2243: a matrix that a light pull request cuts, ``fromJSON( && || )``. +LIGHT = re.compile(r'\$\{\{ fromJSON\((?Pneeds\.changes\.outputs\.[\w-]+ == \'true\') && ' + r'(?Pneeds\.[\w-]+\.outputs\.[\w-]+) \|\| (?Pneeds\.[\w-]+\.outputs\.[\w-]+)\) \}\}') + + +def light_matrix(name: 'str', job_id: 'str') -> 'Optional[re.Match[str]]': + """Job ``job_id``'s matrix as :data:`LIGHT` reads it, or :data:`None` for a matrix every run expands alike.""" + raw = workflow(name)['jobs'][job_id]['strategy']['matrix'] + return LIGHT.fullmatch(raw) if isinstance(raw, str) else None + + +def matrix(name: 'str', job_id: 'str', values: 'Optional[dict[str, str]]' = None, + light: 'bool' = False) -> 'dict[str, Any]': + """Job ``job_id``'s ``strategy.matrix``, each expression read from the script. + + A :data:`LIGHT` matrix is read as a push to ``main`` expands it, or with + ``light`` as a docs-only or test-only pull request does (#2243). + + """ raw = workflow(name)['jobs'][job_id]['strategy']['matrix'] + cut = light_matrix(name, job_id) + if cut is not None: + raw = '${{ fromJSON(%s) }}' % cut.group('cut' if light else 'full') + elif light: + raise AssertionError(f'{name} {job_id} has no light matrix') if isinstance(raw, str): found = resolve(name, job_id, raw, values) assert isinstance(found, dict), f'{name} {job_id} matrix is {raw!r}' diff --git a/tests/project/test_sweeps_on_latest_only_1732.py b/tests/project/test_sweeps_on_latest_only_1732.py index dac2779e5b..e9301f985b 100644 --- a/tests/project/test_sweeps_on_latest_only_1732.py +++ b/tests/project/test_sweeps_on_latest_only_1732.py @@ -468,7 +468,11 @@ def test_the_same_events_and_no_concurrency_group(self) -> None: self.assertNotIn('concurrency', self.own) def test_it_skips_a_docs_only_diff_as_test_does(self) -> None: - """unit-tests.yml's ``changes`` script, byte for byte, gating the job as it gates ``test``.""" + """unit-tests.yml's ``changes`` script, byte for byte, gating the job as it gates ``test``. + + #2243: and skipping a test-only diff, as ``test-ktlo`` does. + + """ def classify(spec: 'dict[str, Any]') -> 'str': return next(entry['run'] for entry in spec['jobs']['changes']['steps'] if entry.get('id') == 'classify') self.assertEqual(classify(self.own), classify(self.unit)) @@ -479,8 +483,9 @@ def classify(spec: 'dict[str, Any]') -> 'str': self.assertEqual(spec['if'], "${{ !cancelled() && needs.changes.result == 'success' && " "needs.python-versions.result == 'success' && needs.current.outputs.current != 'false' && " - "needs.changes.outputs.code != 'false' }}") + "needs.changes.outputs.code != 'false' && needs.changes.outputs.test-only != 'true' }}") self.assertIn("needs.changes.outputs.code != 'false'", self.unit['jobs']['test']['if']) + self.assertIn("needs.changes.outputs.test-only != 'true'", self.unit['jobs']['test-ktlo']['if']) def test_nothing_waits_on_it(self) -> None: """No ``workflow_run`` names it but a reporter's, nothing calls it, and no ``needs:`` anywhere names its job. diff --git a/tests/project/test_unit_tests_queue.py b/tests/project/test_unit_tests_queue.py index c9909f2845..25de9f3c20 100644 --- a/tests/project/test_unit_tests_queue.py +++ b/tests/project/test_unit_tests_queue.py @@ -4,12 +4,16 @@ These fail silently if they drift, so they are pinned here: * ``required-checks`` may accept a ``skipped`` leg **only** when ``changes`` - classified the diff as docs-only. Its shell step is executed below against - every combination that matters, rather than read, because the one way it goes - wrong -- a skip accepted on a code change -- is a green gate over untested code. - It needs ``project-tests`` on every diff, as the project pins' one run (#1954). + classified the diff as docs-only, or a skipped ``engine-tests`` and + ``pypcap-parity`` only when it classified it as test-only (#2243). Its shell + step is executed below against every combination that matters, rather than + read, because the one way it goes wrong -- a skip accepted on a code change -- + is a green gate over untested code. It needs ``project-tests`` on every diff, + as the project pins' one run (#1954). * The ``changes`` classifier must call anything outside ``docs/`` and ``*.md`` - code, and a Markdown file under a code directory code too. + not docs, and a Markdown file under a code directory not docs either. A diff of + ``tests/`` and docs alone is test-only, and any other file makes it code (#2243). + The jobs each class skips are :mod:`tests.project.test_light_pull_requests_2243`'s. * ``unittest-ordering`` stays off pull requests and out of the required set, and queues behind ``test`` without needing it to pass (#1538); ``engine-tests``'s KTLO cells stay non-blocking through a step-level @@ -58,6 +62,11 @@ #: The legs a docs-only pull request may skip. LEGS = ('test', 'integration', 'engine-tests', 'pypcap-parity') +#: The legs a test-only pull request may skip (#2243); the others run ``latest`` alone. +TEST_ONLY_SKIPS = ('engine-tests', 'pypcap-parity') +#: The variable each leg's result reaches the gate's step in. +LEG_ENV = {'test': 'TEST', 'integration': 'INTEGRATION', 'engine-tests': 'ENGINE_TESTS', + 'pypcap-parity': 'PYPCAP_PARITY'} def job(name: 'str') -> 'str': @@ -106,11 +115,12 @@ def setUp(self) -> None: self.script = run_block(job('required-checks')) def gate(self, code: 'str', legs: 'str', project: 'str', changes: 'str' = 'success', - **override: 'str') -> 'int': + test_only: 'str' = 'false', **override: 'str') -> 'int': """The gate's exit status with every leg reporting ``legs`` unless overridden.""" results = {'TEST': legs, 'INTEGRATION': legs, 'ENGINE_TESTS': legs, 'PYPCAP_PARITY': legs} results.update(override) - env = dict(os.environ, CHANGES=changes, CODE=code, PROJECT_TESTS=project, **results) + env = dict(os.environ, CHANGES=changes, CODE=code, TEST_ONLY=test_only, PROJECT_TESTS=project, + **results) return subprocess.run(['bash', '-c', self.script], env=env, check=False, capture_output=True, text=True).returncode @@ -157,10 +167,58 @@ def test_every_leg_is_checked_on_its_own(self) -> None: def test_an_unclassified_diff_skips_nothing(self) -> None: """A failed, skipped or garbled ``changes`` makes every skip a failure.""" - for changes, code in (('failure', ''), ('skipped', ''), ('success', ''), - ('success', 'maybe'), ('failure', 'false')): - with self.subTest(changes=changes, code=code): - self.assertNotEqual(self.gate(code, 'skipped', 'success', changes), 0) + for changes, code, test_only in (('failure', '', ''), ('skipped', '', ''), ('success', '', ''), + ('success', 'maybe', 'false'), ('failure', 'false', 'false'), + ('failure', 'true', 'true'), ('success', 'true', ''), + ('success', 'true', 'maybe'), ('success', 'false', ''), + ('success', 'false', 'true')): + with self.subTest(changes=changes, code=code, test_only=test_only): + self.assertNotEqual(self.gate(code, 'skipped', 'success', changes, test_only), 0) + skips = {LEG_ENV[name]: 'skipped' for name in TEST_ONLY_SKIPS} + self.assertNotEqual(self.gate(code, 'success', 'success', changes, test_only, **skips), 0) + + def test_test_only_accepts_skipped_engine_and_parity_legs(self) -> None: + """#2243: a test-only diff may skip ``engine-tests`` and ``pypcap-parity``, or pass them.""" + skips = {LEG_ENV[name]: 'skipped' for name in TEST_ONLY_SKIPS} + self.assertEqual(self.gate('true', 'success', 'success', test_only='true', **skips), 0) + self.assertEqual(self.gate('true', 'success', 'success', test_only='true'), 0) + + def test_test_only_still_needs_test_and_integration(self) -> None: + """#2243: their ``latest`` legs are the test-only path's tests, so a skip there fails.""" + skips = {LEG_ENV[name]: 'skipped' for name in TEST_ONLY_SKIPS} + for name in sorted(set(LEGS) - set(TEST_ONLY_SKIPS)): + for result in ('skipped', 'failure', 'cancelled'): + with self.subTest(leg=name, result=result): + self.assertNotEqual(self.gate('true', 'success', 'success', test_only='true', + **dict(skips, **{LEG_ENV[name]: result})), 0) + + def test_test_only_still_rejects_a_failed_or_cancelled_skippable_leg(self) -> None: + """#2243: ``skipped`` is accepted on a test-only diff; ``failure`` and ``cancelled`` are not.""" + for name in TEST_ONLY_SKIPS: + for result in ('failure', 'cancelled'): + with self.subTest(leg=name, result=result): + self.assertNotEqual(self.gate('true', 'success', 'success', test_only='true', + **{LEG_ENV[name]: result}), 0) + + def test_test_only_still_needs_project_tests(self) -> None: + """#2243: ``project-tests`` runs the pins on a test-only diff too.""" + skips = {LEG_ENV[name]: 'skipped' for name in TEST_ONLY_SKIPS} + for result in ('skipped', 'failure', 'cancelled'): + with self.subTest(result=result): + self.assertNotEqual(self.gate('true', 'success', result, test_only='true', **skips), 0) + + def test_a_code_change_skips_nothing_the_test_only_path_may(self) -> None: + """#2243: without ``test-only``, a skipped ``engine-tests`` or ``pypcap-parity`` fails.""" + for name in TEST_ONLY_SKIPS: + with self.subTest(leg=name): + self.assertNotEqual(self.gate('true', 'success', 'success', test_only='false', + **{LEG_ENV[name]: 'skipped'}), 0) + + def test_the_gate_reads_test_only_from_changes(self) -> None: + """#2243: ``TEST_ONLY`` is the ``changes`` job's output, set nowhere else.""" + self.assertEqual(job('required-checks').count('TEST_ONLY: '), 1) + self.assertIn(' TEST_ONLY: ${{ needs.changes.outputs.test-only }}\n', job('required-checks')) + self.assertIn(' test-only: ${{ steps.classify.outputs.test-only }}\n', job('changes')) class TestChangesClassifier(unittest.TestCase): @@ -172,6 +230,7 @@ def setUp(self) -> None: module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) self.is_docs = module.is_docs # type: Callable[[str], bool] + self.kind = module.kind # type: Callable[[str], str] def test_docs_and_markdown_are_docs(self) -> None: """``docs/**`` and top-level Markdown are docs.""" @@ -180,8 +239,8 @@ def test_docs_and_markdown_are_docs(self) -> None: with self.subTest(path=path): self.assertTrue(self.is_docs(path)) - def test_everything_else_is_code(self) -> None: - """Anything else, and Markdown under a code directory, is code.""" + def test_everything_else_is_not_docs(self) -> None: + """Anything else, and Markdown under a code directory, is not docs.""" for path in ('pyproject.toml', 'setup.py', 'pcapkit/__init__.py', 'Makefile', '.github/workflows/unit-tests.yml', '.github/PULL_REQUEST_TEMPLATE.md', 'tests/project/README.md', 'util/notes.md', 'examples/README.md', @@ -189,6 +248,27 @@ def test_everything_else_is_code(self) -> None: with self.subTest(path=path): self.assertFalse(self.is_docs(path)) + def test_tests_and_their_support_files_are_tests(self) -> None: + """#2243: everything under ``tests/``, the support files every test runs under included.""" + for path in ('tests/project/test_unit_tests_queue.py', 'tests/conftest.py', 'tests/_tiers.py', + 'tests/__init__.py', 'tests/project/README.md', 'tests/project/ci_timing_fixtures/x.yml'): + with self.subTest(path=path): + self.assertEqual(self.kind(path), 'test') + + def test_workflows_util_and_examples_are_never_tests(self) -> None: + """#2243: a workflow, a ``util/`` script or a capture fixture changes more than the tests.""" + for path in ('.github/workflows/unit-tests.yml', '.github/python-versions.toml', 'util/classify_changes.py', + 'examples/captures/in.pcap', 'examples/generators/make_samples.py', 'pyproject.toml', + 'pcapkit/__init__.py', 'conftest.py', 'test_x.py', 'testsuite/x.py'): + with self.subTest(path=path): + self.assertEqual(self.kind(path), 'code') + + def test_docs_stay_docs(self) -> None: + """``kind`` agrees with ``is_docs``.""" + for path in ('docs/source/index.rst', 'README.md'): + with self.subTest(path=path): + self.assertEqual(self.kind(path), 'docs') + def test_only_this_workflows_own_pull_request_is_classified(self) -> None: """Other events, and callers, always run everything.""" section = job('changes') @@ -351,6 +431,12 @@ def setUp(self) -> None: self.tmp = pathlib.Path(tmp.name) +#: What the classifier writes for each kind of diff (#2243). +DOCS_ONLY = 'code=false\ntest-only=false' +TEST_ONLY = 'code=true\ntest-only=true' +CODE = 'code=true\ntest-only=false' + + class TestClassifierAgainstARealMerge(_Scratch): """The whole ``Classify the diff`` script, run in a scratch repository. @@ -366,7 +452,11 @@ def setUp(self) -> None: if shutil.which('git') is None: # pragma: no cover self.skipTest('git is not available') self.assertIn(CLASSIFY_RUN, job('changes')) - self.repo = self.tmp / 'repo' + self.make_repo('repo') + + def make_repo(self, name: 'str') -> None: + """A fresh scratch repository ``name``, on the PR branch, with main a code change ahead.""" + self.repo = self.tmp / name self.repo.mkdir() self.git('init', '-q', '-b', 'main') self.write('README.md', 'base\n') @@ -383,7 +473,10 @@ def setUp(self) -> None: def git(self, *args: 'str') -> None: """Run git in the scratch repository, isolated from user config.""" env = dict(os.environ, GIT_CONFIG_GLOBAL=os.devnull, GIT_CONFIG_NOSYSTEM='1') - config = ('-c', 'user.name=t', '-c', 'user.email=t@t', '-c', 'commit.gpgsign=false') + # #2243: no directory renames, or moving pcapkit/'s one file in the PR + # would carry main's new pcapkit/y.py along, as a merge conflict. + config = ('-c', 'user.name=t', '-c', 'user.email=t@t', '-c', 'commit.gpgsign=false', + '-c', 'merge.directoryRenames=false') subprocess.run(['git', *config, *args], cwd=self.repo, env=env, check=True, capture_output=True) @@ -403,8 +496,8 @@ def pr(self, *files: 'str') -> None: self.git('merge', '-q', '--no-ff', '--no-edit', 'pr') def classify(self, own_pr: 'str' = 'true', cwd: 'Optional[pathlib.Path]' = None) -> 'str': - """Run the classifier and return the line it wrote to ``GITHUB_OUTPUT``.""" - output = self.tmp / 'output' + """Run the classifier and return the lines it wrote to ``GITHUB_OUTPUT``.""" + output = self.tmp / f'output-{self.repo.name}' status = subprocess.run([sys.executable, str(CLASSIFIER)], env=dict(os.environ, OWN_PR=own_pr, GITHUB_OUTPUT=str(output)), cwd=str(cwd or self.repo), check=False, capture_output=True, text=True).returncode @@ -414,33 +507,64 @@ def classify(self, own_pr: 'str' = 'true', cwd: 'Optional[pathlib.Path]' = None) def test_a_docs_only_pull_request_is_not_code(self) -> None: """Only ``HEAD^1`` sees the PR's own diff; ``HEAD^2`` would see main's code.""" self.pr('docs/source/index.rst', 'CONTRIBUTING.md') - self.assertEqual(self.classify(), 'code=false') + self.assertEqual(self.classify(), DOCS_ONLY) def test_a_code_file_makes_it_code(self) -> None: """One code file among docs is enough.""" self.pr('docs/source/index.rst', 'pcapkit/x.py') - self.assertEqual(self.classify(), 'code=true') + self.assertEqual(self.classify(), CODE) def test_a_name_holding_whitespace_is_one_path(self) -> None: """``notes.md docs/y`` is a root file, not ``notes.md`` and ``docs/y``.""" self.pr('docs/source/index.rst', 'notes.md docs/y') - self.assertEqual(self.classify(), 'code=true') + self.assertEqual(self.classify(), CODE) def test_another_event_always_runs_everything(self) -> None: """``OWN_PR`` false (a push, or a caller's pull request) is code.""" self.pr('docs/source/index.rst') - self.assertEqual(self.classify(own_pr='false'), 'code=true') + self.assertEqual(self.classify(own_pr='false'), CODE) + + def test_a_push_of_tests_alone_is_code(self) -> None: + """#2243: a push to ``main`` never takes the test-only path, however little it changed.""" + self.pr('tests/test_x.py') + self.assertEqual(self.classify(own_pr='false'), CODE) def test_an_empty_diff_is_code(self) -> None: """Nothing to classify is not evidence of a docs-only change.""" self.pr() - self.assertEqual(self.classify(), 'code=true') + self.assertEqual(self.classify(), CODE) def test_a_failed_diff_is_code(self) -> None: """Outside a repository ``git diff`` fails, and that must not skip anything.""" elsewhere = self.tmp / 'not-a-repo' elsewhere.mkdir() - self.assertEqual(self.classify(cwd=elsewhere), 'code=true') + self.assertEqual(self.classify(cwd=elsewhere), CODE) + + def test_tests_alone_are_test_only(self) -> None: + """#2243: a diff under ``tests/`` alone, its support files included, is test-only.""" + self.pr('tests/test_x.py', 'tests/conftest.py', 'tests/_tiers.py') + self.assertEqual(self.classify(), TEST_ONLY) + + def test_tests_and_docs_are_test_only(self) -> None: + """#2243: docs alongside tests do not make it code.""" + self.pr('tests/test_x.py', 'docs/source/index.rst', 'README.md') + self.assertEqual(self.classify(), TEST_ONLY) + + def test_tests_and_anything_else_are_code(self) -> None: + """#2243: one file outside ``tests/`` and the docs makes it code.""" + for index, other in enumerate(('pcapkit/x.py', 'util/classify_changes.py', '.github/workflows/unit-tests.yml', + 'examples/captures/in.pcap', 'pyproject.toml')): + with self.subTest(other=other): + self.make_repo(f'repo-{index}') + self.pr('tests/test_x.py', other) + self.assertEqual(self.classify(), CODE) + + def test_moving_code_under_tests_is_code(self) -> None: + """#2243: ``--no-renames`` lists the old path too, so a move out of ``pcapkit/`` is code.""" + (self.repo / 'tests').mkdir() + self.git('mv', 'pcapkit/x.py', 'tests/x.py') + self.pr() + self.assertEqual(self.classify(), CODE) class TestEngineCellLoops(_Scratch): diff --git a/tests/project/test_workflow_scripts_1967.py b/tests/project/test_workflow_scripts_1967.py index a7e85d7561..9b1bbe5201 100644 --- a/tests/project/test_workflow_scripts_1967.py +++ b/tests/project/test_workflow_scripts_1967.py @@ -4,7 +4,7 @@ * :file:`util/leg_summary.py`, every ``KTLO summary`` and the ``Experimental summary``: :class:`TestLegSummary`. The steps themselves are pinned in :mod:`tests.project.test_ktlo_legs_stay_green_1672`. -* :file:`util/classify_changes.py`, ``Classify the diff`` in both ``changes`` jobs: +* :file:`util/classify_changes.py`, ``Classify the diff`` in every ``changes`` job: :class:`TestClassifyChanges`. Its classification against a real merge commit is :mod:`tests.project.test_unit_tests_queue`'s. * :file:`util/coverage_report.py`, ``Report coverage``: :class:`TestCoverageReport`. @@ -108,8 +108,9 @@ def test_another_event_is_code(self) -> None: output.write_text('earlier=1\n', encoding='utf-8') result = run('classify_changes.py', env={'OWN_PR': 'false', 'GITHUB_OUTPUT': str(output)}, cwd=self.tmp) self.assertEqual(result.returncode, 0, result.stderr) - self.assertEqual(result.stdout, "Not this workflow's own pull_request run: everything runs.\ncode=true\n") - self.assertEqual(output.read_text(encoding='utf-8'), 'earlier=1\ncode=true\n') + self.assertEqual(result.stdout, + "Not this workflow's own pull_request run: everything runs.\ncode=true\ntest-only=false\n") + self.assertEqual(output.read_text(encoding='utf-8'), 'earlier=1\ncode=true\ntest-only=false\n') def test_an_unreadable_diff_warns_and_is_code(self) -> None: if shutil.which('git') is None: # pragma: no cover @@ -119,11 +120,12 @@ def test_an_unreadable_diff_warns_and_is_code(self) -> None: result = run('classify_changes.py', env=env, cwd=self.tmp) self.assertEqual(result.returncode, 0, result.stderr) self.assertRegex(result.stdout, r'^::warning title=Could not list the changed files::git diff exited ' - r'[1-9]\d* with 0 files: .*Treating the diff as code\.\ncode=true\n$') - self.assertEqual(output.read_text(encoding='utf-8'), 'code=true\n') + r'[1-9]\d* with 0 files: .*Treating the diff as code\.\n' + r'code=true\ntest-only=false\n$') + self.assertEqual(output.read_text(encoding='utf-8'), 'code=true\ntest-only=false\n') - def test_both_workflows_run_it_from_a_checkout(self) -> None: - for name in ('unit-tests.yml', 'experimental.yml'): + def test_every_workflow_runs_it_from_a_checkout(self) -> None: + for name in ('unit-tests.yml', 'experimental.yml', 'python-compatibility.yml'): found = steps(name, 'changes') with self.subTest(workflow=name): self.assertNotIn('if', found[0]) diff --git a/util/classify_changes.py b/util/classify_changes.py index ce6548ec79..86117d1004 100644 --- a/util/classify_changes.py +++ b/util/classify_changes.py @@ -1,24 +1,40 @@ # -*- coding: utf-8 -*- -"""Whether a pull request's changed files include code (#1052). +"""Whether a pull request's changed files include code (#1052), or only tests (#2243). The ``Classify the diff`` step of the ``changes`` job in -:file:`.github/workflows/unit-tests.yml` and in :file:`.github/workflows/experimental.yml` -runs ``python3 util/classify_changes.py`` in the checkout and writes ``code=true`` -or ``code=false`` to ``$GITHUB_OUTPUT``. That output decides whether the -expensive legs run at all, so the script fails safe: anything it cannot classify -is code. +:file:`.github/workflows/unit-tests.yml`, :file:`.github/workflows/experimental.yml` +and :file:`.github/workflows/python-compatibility.yml` runs +``python3 util/classify_changes.py`` in the checkout and writes two outputs to +``$GITHUB_OUTPUT``: + +``code`` + ``false`` on a docs-only diff, ``true`` otherwise. +``test-only`` + ``true`` on a test-only diff, ``false`` otherwise. + +Those outputs decide which expensive legs run at all, so the script fails safe: +anything it cannot classify is code, ``code=true`` and ``test-only=false``. A file is docs iff :func:`is_docs` holds: it is under ``docs/`` or ends in ``.md``, and is not under one of :data:`CODE_DIRS` -- so a Markdown file in one -of those (a PR template, a test fixture) is code, and so is every other file -(``pyproject.toml``, ``Makefile``, ...). The diff is ``HEAD^1..HEAD``, a pull -request's merge commit against its first parent, which is exactly what merging -would add; ``--no-renames`` lists both sides of a rename, so moving a code file -under ``docs/`` is still a code change. +of those (a PR template, a test fixture) is not docs, and nor is any other file +(``pyproject.toml``, ``Makefile``, ...). A file is a test iff :func:`is_test` +holds: it is under :data:`TEST_DIR`. That takes in the support files every test +runs under, such as ``tests/conftest.py`` and ``tests/_tiers.py``, and the +fixtures there, since a test-only diff still runs the whole suite on ``latest``. +Every other file is code, so a workflow, a ``util/`` script or an ``examples/`` +file is never a test. A diff of docs and tests alone, with at least one test, is +test-only. + +The diff is ``HEAD^1..HEAD``, a pull request's merge commit against its first +parent, which is exactly what merging would add; ``--no-renames`` lists both +sides of a rename, so moving a code file under ``docs/`` or ``tests/`` is still +a code change. Only the workflow's own ``pull_request`` run is classified, which the step -passes as ``OWN_PR=true``. Anything else reports ``code=true`` without reading -the diff. A failed or empty ``git diff`` is code, with a warning. +passes as ``OWN_PR=true``. Anything else, a push to ``main`` included, reports +code without reading the diff. A failed or empty ``git diff`` is code, with a +warning. The step runs on the runner's own ``python3``, as no setup-python precedes it, so the script is standard library only. @@ -37,10 +53,12 @@ except ImportError: # imported as util.