diff --git a/.github/workflows/changed-paths.yml b/.github/workflows/changed-paths.yml new file mode 100644 index 0000000..2ccb173 --- /dev/null +++ b/.github/workflows/changed-paths.yml @@ -0,0 +1,236 @@ +name: Changed paths + +# description: | +# Tells callers whether a pull request touches anything relevant to a given concern: +# go sources for the 'go' preset, documentation sources for the 'doc' preset. +# +# The point is to let a workflow skip its expensive jobs on a pull request that cannot +# affect them, while still reporting a status: a job skipped by an "if:" costs no runner +# minute, but the workflow -- and therefore its gate -- keeps reporting to the branch +# protection rules. Filtering with "on.paths-ignore" instead would prevent the workflow +# from starting at all, leaving a required check hanging at "Expected -- waiting for +# status to be reported". +# +# Detection is ADVISORY and FAILS OPEN. Every uncertain situation resolves to +# "changed=true", i.e. do the work. Jobs must never be skipped as the side effect of a +# rate limit, a missing token scope or an API quirk. See the "Decide" step below for the +# full list of cases. + +permissions: + contents: read + pull-requests: read + +on: + workflow_call: + inputs: + preset: + description: | + Which set of paths to watch: 'go' or 'doc'. + + 'go' watches go sources and anything that alters a build or a test outcome. + 'doc' watches markdown, the hugo doc site and the markdown/spellcheck linter + configurations. + + Both presets are deliberately generous: one pattern too many only means the work + runs when it need not, one pattern too few means it is silently skipped when it + should have run. + type: string + required: true + extra-paths: + description: | + Extra glob patterns to watch on top of the preset, one per line. + + Repositories that do not follow the go-openapi layout declare the difference here, + e.g. for a doc site whose content sits at the repository root: + + extra-paths: | + docs/** + + Patterns are picomatch globs, relative to the repository root. + type: string + required: false + default: '' + force: + description: | + Set to 'true' to bypass detection altogether and always report 'changed=true'. + + Use it for scheduled or manually dispatched runs that must do the work whatever + changed, and as the opt-out for repositories that do not want path-based skipping. + type: string + required: false + default: 'false' + outputs: + changed: + description: | + 'true' when a watched path changed, or when detection could not be trusted. + 'false' only when we positively established that nothing relevant changed. + value: ${{ jobs.changed-paths.outputs.changed }} + reason: + description: 'Plain English explanation of how the decision was reached.' + value: ${{ jobs.changed-paths.outputs.reason }} + +defaults: + run: + shell: bash + +jobs: + changed-paths: + name: detect changed paths + runs-on: ubuntu-latest + outputs: + changed: ${{ steps.decide.outputs.changed }} + reason: ${{ steps.decide.outputs.reason }} + steps: + - + name: Build the filter spec + id: spec + env: + PRESET: ${{ inputs.preset }} + EXTRA_PATHS: ${{ inputs.extra-paths }} + run: | + # Watched paths per preset. Leading blanks are trimmed when the lists are + # emitted, so these stay aligned with the surrounding script. + GO_PATHS='**/*.go + **/*.gotmpl + **/go.mod + **/go.sum + **/go.work + **/go.work.sum + **/testdata/** + .golangci.yml + .golangci.yaml + .codecov.yml + .github/workflows/**' + + # The go-openapi layout is: markdown content under docs/doc-site, hugo + # configuration, layouts and themes under hack/doc-site/hugo. Repositories that + # keep their content elsewhere add it through extra-paths. + DOC_PATHS='**/*.md + **/*.markdown + docs/doc-site/** + hack/doc-site/** + .markdownlint.yml + .markdownlint.yaml + .spellcheck.yml + .spellcheck.yaml + .wordlist.txt' + + case "${PRESET}" in + go) + base="${GO_PATHS}" + ;; + doc) + base="${DOC_PATHS}" + ;; + *) + # A bad preset is a caller mistake, not an uncertain detection: fail loudly + # rather than fall back on watching nothing. + echo "::error title=changed-paths::unknown preset '${PRESET}': expected 'go' or 'doc'" + exit 1 + ;; + esac + + spec="${RUNNER_TEMP}/paths-filter.yml" + echo "changed:" > "${spec}" + + # Single quotes are dropped: they would break the YAML scalar and no legitimate + # glob needs them. The "|| [[ -n ... ]]" tail keeps the last line when the input + # does not end with a newline. + emit_globs() { + local glob + + while IFS=$' \t' read -r glob || [[ -n "${glob}" ]] ; do + glob="${glob//\'/}" + if [[ -z "${glob}" ]] ; then + continue + fi + printf " - '%s'\n" "${glob}" >> "${spec}" + done + } + + printf '%s\n' "${base}" | emit_globs + printenv EXTRA_PATHS | emit_globs + + echo "::group::paths filter spec (preset: ${PRESET})" + cat "${spec}" + echo "::endgroup::" + + { + echo "filters<> "${GITHUB_OUTPUT}" + - + name: Detect changed paths + id: filter + # Only pull requests carry a list of changed files we can rely on: the action + # reads it from the pull request files API of the BASE repository, so it works + # for fork pull requests (the read-only token is still a token on the base repo) + # and needs no checkout. On any other event -- push, schedule, workflow_dispatch, + # merge_group -- there is no such list and we do the work. + # + # continue-on-error is what makes the failure open rather than closed: a rate + # limit or a caller that forgot "pull-requests: read" leaves the decision below + # free to fall back on doing the work. + if: >- + ${{ + inputs.force != 'true' && + (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') + }} + continue-on-error: true + uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4.0.3 + with: + filters: ${{ steps.spec.outputs.filters }} + - + name: Decide + id: decide + env: + FORCE: ${{ inputs.force }} + EVENT_NAME: ${{ github.event_name }} + # Empty outside of a pull request context. + CHANGED_FILES: ${{ github.event.pull_request.changed_files }} + FILTER_OUTCOME: ${{ steps.filter.outcome }} + FILTER_CHANGED: ${{ steps.filter.outputs.changed }} + run: | + # The pull request files API returns at most this many entries. At or above the + # cap the list is truncated, so "nothing relevant changed" cannot be concluded + # from it. Large regeneration pull requests do reach this. + API_FILE_CAP=3000 + + decision() { + local changed="$1" reason="$2" + + printf 'changed=%s\n' "${changed}" >> "${GITHUB_OUTPUT}" + printf 'reason=%s\n' "${reason}" >> "${GITHUB_OUTPUT}" + + if [[ "${changed}" == "true" ]] ; then + echo "::notice title=changed-paths::proceeding: ${reason}" + else + echo "::notice title=changed-paths::skipping: ${reason}" + fi + + exit 0 + } + + if [[ "${FORCE}" == "true" ]] ; then + decision true "detection bypassed by the 'force' input" + fi + + if [[ "${EVENT_NAME}" != "pull_request" && "${EVENT_NAME}" != "pull_request_target" ]] ; then + decision true "event '${EVENT_NAME}' carries no reliable list of changed files" + fi + + if [[ "${CHANGED_FILES}" =~ ^[0-9]+$ ]] && (( CHANGED_FILES >= API_FILE_CAP )) ; then + decision true \ + "this pull request reports ${CHANGED_FILES} changed files, at or over the ${API_FILE_CAP}-file cap of the pull request files API: the list is truncated" + fi + + if [[ "${FILTER_OUTCOME}" != "success" ]] ; then + decision true "changed paths could not be determined (detection outcome: '${FILTER_OUTCOME}')" + fi + + if [[ "${FILTER_CHANGED}" == "true" ]] ; then + decision true "a watched path changed" + fi + + decision false "no watched path changed" diff --git a/.github/workflows/doc-changed.yml b/.github/workflows/doc-changed.yml new file mode 100644 index 0000000..ae356dd --- /dev/null +++ b/.github/workflows/doc-changed.yml @@ -0,0 +1,58 @@ +name: Doc changed + +# description: | +# Tells a documentation workflow whether a pull request touches the doc site, so it can +# skip its build (or its markdown/spellcheck lint) while still reporting a status. +# +# Replaces markdown-changed.yml, which reported only "some markdown changed", relied on +# tj-actions/changed-files, needed a checkout, and had no fallback when detection failed. +# +# Detection FAILS OPEN: see changed-paths.yml, which does the actual work. + +permissions: + contents: read + pull-requests: read + +on: + workflow_call: + inputs: + extra-paths: + description: | + Extra glob patterns to watch on top of the doc preset, one per line. + + The preset follows the go-openapi layout: markdown anywhere, content under + docs/doc-site, hugo configuration and themes under hack/doc-site. A repository + that keeps its content elsewhere declares it here, e.g. go-swagger: + + extra-paths: | + docs/** + type: string + required: false + default: '' + force: + description: | + Set to 'true' to bypass detection and always report 'changed=true'. + + Use it for scheduled or manually dispatched runs, and as the opt-out for + repositories that do not want path-based skipping. + type: string + required: false + default: 'false' + outputs: + changed: + description: | + 'true' when a documentation path changed, or when detection could not be trusted. + 'false' only when we positively established that no doc path changed. + value: ${{ jobs.doc-changed.outputs.changed }} + reason: + description: 'Plain English explanation of how the decision was reached.' + value: ${{ jobs.doc-changed.outputs.reason }} + +jobs: + doc-changed: + name: doc changed + uses: ./.github/workflows/changed-paths.yml + with: + preset: doc + extra-paths: ${{ inputs.extra-paths }} + force: ${{ inputs.force }} diff --git a/.github/workflows/go-test-monorepo.yml b/.github/workflows/go-test-monorepo.yml index 1594e81..a8daf3d 100644 --- a/.github/workflows/go-test-monorepo.yml +++ b/.github/workflows/go-test-monorepo.yml @@ -33,14 +33,46 @@ on: type: string required: false default: '30m' + extra-paths: + description: | + Extra glob patterns, one per line, that mark a change as relevant to the tests. + + See the 'extra-paths' input of changed-paths.yml. Repositories with fixtures or a + codegen configuration outside of the go sources want to declare them here. + type: string + required: false + default: '' + force-run: + description: | + Set to 'true' to run the full matrix whatever changed, bypassing changed-paths detection. + + Use it for scheduled or manually dispatched runs, and as the opt-out for repositories + that do not want path-based skipping. + type: string + required: false + default: 'false' defaults: run: shell: bash jobs: + changes: + # description: | + # Decide whether this run has anything to test. Fails open: anything unclear runs + # the whole suite. Skipped jobs cost no runner minute, but the workflow keeps + # reporting, so "tests completed" stays usable as a required status check. + name: Changes + uses: ./.github/workflows/changed-paths.yml + with: + preset: go + extra-paths: ${{ inputs.extra-paths }} + force: ${{ inputs.force-run }} + lint: name: Lint + needs: [changes] + if: ${{ needs.changes.outputs.changed == 'true' }} runs-on: ubuntu-latest outputs: is-monorepo: ${{ steps.detect-monorepo.outputs.is-monorepo }} @@ -49,7 +81,6 @@ jobs: module-names: ${{ steps.detect-monorepo.outputs.names }} coverpkg: ${{ steps.prepare-tests.outputs.coverpkg }} all-modules: ${{ steps.prepare-tests.outputs.all-modules }} - test-matrix: ${{ steps.test-matrix.outputs.test-matrix }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -111,23 +142,19 @@ jobs: # golangci-lint run --new-from-rev origin/master # popd # done < <(echo ${{ steps.detect-monorepo.outputs.bash-paths }}) - - - name: Test matrix - id: test-matrix - env: - MATRIX: | - ${{ inputs.test-matrix || '{"os":["ubuntu-latest","macos-latest","windows-latest"],"go": ["oldstable","stable"]}' }} - run: | - echo "test-matrix<> "${GITHUB_OUTPUT}" - printenv MATRIX >> "${GITHUB_OUTPUT}" - echo "EOF" >> "${GITHUB_OUTPUT}" test: name: Unit tests mono-repo - needs: [ lint ] + needs: [changes, lint] + if: ${{ needs.changes.outputs.changed == 'true' }} runs-on: ${{ matrix.os }} strategy: - matrix: ${{ fromJSON(needs.lint.outputs.test-matrix) }} + # Read straight from the inputs: routing it through a job output would make the + # matrix depend on that job having run, and lint is now skippable. + # + # Mind the absence of a space after '"go":': a ": " sequence would end the plain + # YAML scalar and the whole expression would fail to parse. + matrix: ${{ fromJSON(inputs.test-matrix || '{"os":["ubuntu-latest","macos-latest","windows-latest"],"go":["oldstable","stable"]}') }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -255,23 +282,54 @@ jobs: fuzz-test: # fuzz-test supports go monorepos + needs: [changes] + if: ${{ needs.changes.outputs.changed == 'true' }} uses: ./.github/workflows/fuzz-test.yml test-complete: # description: | # Be explicit about all tests being passed. This allows for setting up only a few status checks on PRs. + # + # This is the gate: it is the one job callers pin in their branch protection rules, so it + # MUST report on every run, including the runs where everything else was skipped. + # + # Hence "always()" -- with the default condition the job would inherit the skip of its + # dependencies and never report, which is the very hole we are closing -- and an assertion + # on the negative: fail on 'failure' and 'cancelled', pass on 'skipped'. A job correctly + # skipped is then indistinguishable from a job that had nothing to do. + # + # Every job belongs in "needs", "changes" included: a job skipped because ITS dependency + # failed reports 'skipped', so leaving the failing one out would turn the gate green over + # an untested pull request. name: tests completed - needs: [test,fuzz-test] + needs: [changes, lint, test, fuzz-test] + if: ${{ always() }} runs-on: ubuntu-latest steps: + - + name: Report upstream job results + env: + RESULTS: ${{ toJSON(needs) }} + run: | + echo "::group::upstream job results" + printenv RESULTS + echo "::endgroup::" + - + name: Fail on any unsuccessful upstream job + if: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }} + run: | + echo "::error title=tests::at least one job failed or was cancelled" + exit 1 - name: Tests completed run: | echo "::notice title=Success::All tests passed" collect-coverage: - needs: [test-complete] - if: ${{ !cancelled() && needs.test-complete.result == 'success' }} + # Guarded on the tests themselves: "tests completed" is now green on runs where the + # matrix never ran, and there would be no coverage artifact to collect. + needs: [test] + if: ${{ !cancelled() && needs.test.result == 'success' }} uses: ./.github/workflows/collect-coverage.yml collect-reports: diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 8f15cd0..c50a299 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -33,17 +33,47 @@ on: type: string required: false default: '30m' + extra-paths: + description: | + Extra glob patterns, one per line, that mark a change as relevant to the tests. + + See the 'extra-paths' input of changed-paths.yml. Repositories with fixtures or a + codegen configuration outside of the go sources want to declare them here. + type: string + required: false + default: '' + force-run: + description: | + Set to 'true' to run the full matrix whatever changed, bypassing changed-paths detection. + + Use it for scheduled or manually dispatched runs, and as the opt-out for repositories + that do not want path-based skipping. + type: string + required: false + default: 'false' defaults: run: shell: bash jobs: + changes: + # description: | + # Decide whether this run has anything to test. Fails open: anything unclear runs + # the whole suite. Skipped jobs cost no runner minute, but the workflow keeps + # reporting, so "tests completed" stays usable as a required status check. + name: Changes + uses: ./.github/workflows/changed-paths.yml + with: + preset: go + extra-paths: ${{ inputs.extra-paths }} + force: ${{ inputs.force-run }} + lint: name: Lint + needs: [changes] + if: ${{ needs.changes.outputs.changed == 'true' }} runs-on: ubuntu-latest - outputs: - test-matrix: ${{ steps.test-matrix.outputs.test-matrix }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -60,24 +90,20 @@ jobs: version: latest only-new-issues: true skip-cache: true - - - name: Test matrix - id: test-matrix - env: - MATRIX: | - ${{ inputs.test-matrix || '{"os":["ubuntu-latest","macos-latest","windows-latest"],"go": ["oldstable","stable"]}' }} - run: | - echo "test-matrix<> "${GITHUB_OUTPUT}" - printenv MATRIX >> "${GITHUB_OUTPUT}" - echo "EOF" >> "${GITHUB_OUTPUT}" test: name: Unit tests runs-on: ${{ matrix.os }} - needs: [lint] + needs: [changes, lint] + if: ${{ needs.changes.outputs.changed == 'true' }} strategy: - matrix: ${{ fromJSON(needs.lint.outputs.test-matrix) }} + # Read straight from the inputs: routing it through a job output would make the + # matrix depend on that job having run, and lint is now skippable. + # + # Mind the absence of a space after '"go":': a ": " sequence would end the plain + # YAML scalar and the whole expression would fail to parse. + matrix: ${{ fromJSON(inputs.test-matrix || '{"os":["ubuntu-latest","macos-latest","windows-latest"],"go":["oldstable","stable"]}') }} steps: - @@ -143,23 +169,54 @@ jobs: retention-days: 1 fuzz-test: + needs: [changes] + if: ${{ needs.changes.outputs.changed == 'true' }} uses: ./.github/workflows/fuzz-test.yml test-complete: # description: | # Be explicit about all tests being passed. This allows for setting up only a few status checks on PRs. + # + # This is the gate: it is the one job callers pin in their branch protection rules, so it + # MUST report on every run, including the runs where everything else was skipped. + # + # Hence "always()" -- with the default condition the job would inherit the skip of its + # dependencies and never report, which is the very hole we are closing -- and an assertion + # on the negative: fail on 'failure' and 'cancelled', pass on 'skipped'. A job correctly + # skipped is then indistinguishable from a job that had nothing to do. + # + # Every job belongs in "needs", "changes" included: a job skipped because ITS dependency + # failed reports 'skipped', so leaving the failing one out would turn the gate green over + # an untested pull request. name: tests completed - needs: [test,fuzz-test] + needs: [changes, lint, test, fuzz-test] + if: ${{ always() }} runs-on: ubuntu-latest steps: + - + name: Report upstream job results + env: + RESULTS: ${{ toJSON(needs) }} + run: | + echo "::group::upstream job results" + printenv RESULTS + echo "::endgroup::" + - + name: Fail on any unsuccessful upstream job + if: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }} + run: | + echo "::error title=tests::at least one job failed or was cancelled" + exit 1 - name: Tests completed run: | echo "::notice title=Success::All tests passed" collect-coverage: - needs: [test-complete] - if: ${{ !cancelled() && needs.test-complete.result == 'success' }} + # Guarded on the tests themselves: "tests completed" is now green on runs where the + # matrix never ran, and there would be no coverage artifact to collect. + needs: [test] + if: ${{ !cancelled() && needs.test.result == 'success' }} uses: ./.github/workflows/collect-coverage.yml collect-reports: diff --git a/.github/workflows/local-go-test.yml b/.github/workflows/local-go-test.yml index 4eeb91f..8761b56 100644 --- a/.github/workflows/local-go-test.yml +++ b/.github/workflows/local-go-test.yml @@ -14,7 +14,28 @@ on: pull_request: + # Manual trigger, to exercise changed-paths detection by hand. + # + # A dispatch run carries no list of changed files, so detection resolves to "run + # everything" whatever is chosen here. What the input tells apart is WHY: with + # 'false' the run reports "event 'workflow_dispatch' carries no reliable list of + # changed files", with 'true' it reports "detection bypassed by the 'force' input" + # -- which is how we check that the input actually reaches changed-paths.yml. + workflow_dispatch: + inputs: + force-run: + description: 'bypass changed-paths detection' + type: choice + required: false + default: 'false' + options: + - 'false' + - 'true' + jobs: test: uses: ./.github/workflows/go-test.yml + with: + # Empty on every trigger but workflow_dispatch, where the choice input is a string. + force-run: ${{ inputs.force-run || 'false' }} secrets: inherit diff --git a/.github/workflows/markdown-changed.yml b/.github/workflows/markdown-changed.yml deleted file mode 100644 index 7a66c42..0000000 --- a/.github/workflows/markdown-changed.yml +++ /dev/null @@ -1,33 +0,0 @@ -name: Markdown changed - -on: - workflow_call: - -permissions: - contents: read - -jobs: - markdown-changed: - # description: | - # This triggers a markdown and spellcheck lint whenever documentation files change. - runs-on: ubuntu-latest - outputs: - proceed: ${{ steps.changed-markdown-files.outputs.any_changed }} - all_changed_files: ${{ steps.changed-markdown-files.outputs.all_changed_files }} - steps: - - - name: Originating repo checkout (e.g. public fork) - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - ref: ${{ github.event.pull_request.head.sha }} - - - name: Get changed markdown files - uses: tj-actions/changed-files@9426d40962ed5378910ee2e21d5f8c6fcbf2dd96 # v4.7.0 - id: changed-markdown-files - with: - files: '**/*.md' - - - name: Notify - run: | - echo "::notice::Detected some changed markdown files" - echo "${{ steps.changed-markdown-files.outputs.all_changed_files }}"