test: auto-fix pull-request e2e (do not merge)

.github/workflows/auto-fix-pr-e2e.yml

@@ -0,0 +1,289 @@
+name: auto-fix PR e2e
+
+# Phase 2 of the auto-fix end-to-end test: the pull-request path.
+#
+# Phase 1 (auto-fix-e2e.yml) covers the push path, where the action derives the
+# branch from GITHUB_REF. This workflow covers what only a real PR can exercise:
+#
+#   - the default pull_request checkout (the merge ref), which is what users get
+#   - the same-repository gating of the commit step
+#   - pushing the fix back onto the PR head branch, so the PR updates itself
+#   - that applying fixes does not disturb what clang-tidy reports
+#
+# The action under test is the ./cpp-linter-action submodule, pinned on this
+# branch to the head of cpp-linter-action's feature/auto-fix.
+#
+# Workflows added by a pull request do run for `pull_request` events, so this
+# file takes effect from the PR branch itself.
+
+on:
+  pull_request:
+    types: [opened, synchronize, reopened]
+
+permissions:
+  contents: write
+  # `format-review` on the auto-fix run below needs this.
+  pull-requests: write
+
+env:
+  TEST_FILE: src/e2e_autofix_pr_demo.cpp
+  CLANG_VERSION: '18'
+  COMMIT_MSG: 'style: apply clang-format fixes'
+  # Two long-standing checks with stable behaviour across clang releases. The
+  # fixture triggers both, and `-*` keeps the repo's .clang-tidy out of it so
+  # the counts below don't drift when that file changes.
+  TIDY_CHECKS: '-*,readability-magic-numbers,modernize-use-nullptr'
+
+jobs:
+  auto-fix-pr:
+    # Only ever run for the dedicated e2e branch, never for real pull requests.
+    if: startsWith(github.head_ref, 'test/auto-fix-pr-e2e')
+    runs-on: ubuntu-latest
+    steps:
+      # `persist-credentials` stays enabled (the default) so the action can fetch
+      # the head branch and push the fix back to it.
+      - uses: actions/checkout@v7
+        with:
+          submodules: true
+          fetch-depth: 0
+
+      - name: Record the pre-fix state
+        run: |
+          set -euo pipefail
+          echo "::group::fixture before"
+          cat -n "$TEST_FILE"
+          echo "::endgroup::"
+          # This run has to start from the default checkout (the merge commit),
+          # otherwise it would not cover the action's own checkout of the PR head.
+          echo "HEAD=$(git rev-parse HEAD) PR head=${{ github.event.pull_request.head.sha }}"
+          if [ "$(git rev-parse HEAD)" = "${{ github.event.pull_request.head.sha }}" ]; then
+            echo "::error title=Not the default checkout::HEAD is already the PR head."
+            exit 1
+          fi
+          {
+            echo "MALFORMED_BLOB=$(git hash-object "$TEST_FILE")"
+            echo "PR_HEAD_SHA=${{ github.event.pull_request.head.sha }}"
+          } >> "$GITHUB_ENV"
+
+      # Baseline: the same lint, with auto-fix off. This measures what
+      # clang-tidy reports against the file as the diff describes it, and is
+      # the reference the auto-fix run below has to reproduce.
+      #
+      # `files-changed-only: true` is deliberate and load-bearing: src/demo.cpp
+      # and src/demo.hpp are intentionally unformatted fixtures of this repo, and
+      # scanning everything would make auto-fix "repair" and commit them too.
+      - name: Lint without auto-fix (baseline)
+        uses: ./cpp-linter-action
+        id: baseline
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        with:
+          style: file
+          tidy-checks: ${{ env.TIDY_CHECKS }}
+          extra-args: '-std=c++17'
+          files-changed-only: true
+          lines-changed-only: true
+          ignore: build
+          version: '18'
+          verbosity: debug
+          thread-comments: false
+          step-summary: false
+          auto-fix: false
+
+      - name: The baseline must not have touched the fixture
+        run: |
+          set -euo pipefail
+          if [ "$(git hash-object "$TEST_FILE")" != "$MALFORMED_BLOB" ]; then
+            echo "::error title=Baseline mutated the fixture::auto-fix was off, but $TEST_FILE changed."
+            exit 1
+          fi
+          echo "PASS: baseline left the fixture malformed, as expected"
+
+      # The real run. Same inputs, auto-fix on.
+      - name: Run cpp-linter with auto-fix
+        uses: ./cpp-linter-action
+        id: linter
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        with:
+          style: file
+          tidy-checks: ${{ env.TIDY_CHECKS }}
+          extra-args: '-std=c++17'
+          files-changed-only: true
+          lines-changed-only: true
+          ignore: build
+          version: '18'
+          verbosity: debug
+          thread-comments: false
+          step-summary: false
+          auto-fix: true
+          auto-fix-commit-msg: 'style: apply clang-format fixes'
+          # Only this run asks for a clang-format review, and only this run can
+          # hit the bug it covers: a fixed file has no advice left to report,
+          # but the review pass still walks it and wants a patch to diff
+          # against. Handing it an empty one used to abort cpp-linter with
+          # `AssertionError: FormatAdvice has no suggestions for <file>`, so
+          # this step failing at all is the assertion. Nothing is posted --
+          # the file comes out clean, and `no-lgtm` defaults to true.
+          format-review: true
+          passive-reviews: true
+
+      - name: Verify the PR branch was fixed and updated
+        env:
+          BASE_FORMAT: ${{ steps.baseline.outputs.clang-format-checks-failed }}
+          BASE_TIDY: ${{ steps.baseline.outputs.clang-tidy-checks-failed }}
+          FIX_FORMAT: ${{ steps.linter.outputs.clang-format-checks-failed }}
+          FIX_TIDY: ${{ steps.linter.outputs.clang-tidy-checks-failed }}
+        run: |
+          set -uo pipefail
+          failed=0
+          head_ref='${{ github.event.pull_request.head.ref }}'
+
+          echo "::group::fixture after"
+          cat -n "$TEST_FILE"
+          echo "::endgroup::"
+
+          echo "baseline: clang-format=$BASE_FORMAT clang-tidy=$BASE_TIDY"
+          echo "auto-fix: clang-format=$FIX_FORMAT clang-tidy=$FIX_TIDY"
+
+          # --- which of the two states is this run in? ---
+          # Pushing the fix raises a `synchronize` event, so this workflow runs a
+          # second time against its own auto-fix commit. The fixture is clean by
+          # then and doing nothing is the correct behaviour, so that run asserts
+          # idempotency instead of demanding another fix. The baseline's own
+          # clang-format count says which state we are in, so this stays true
+          # whatever the fixture is later rewritten to contain.
+          if [ "${BASE_FORMAT:-0}" -gt 0 ]; then
+            expect_fix=true
+            echo "PASS: baseline saw $BASE_FORMAT clang-format issue(s)"
+          else
+            expect_fix=false
+            echo "NOTE: the fixture is already formatted, so this is the re-run"
+            echo "      that auto-fix's own push triggered. Asserting that it is"
+            echo "      a no-op rather than expecting another fix."
+          fi
+
+          if [ "${BASE_TIDY:-0}" -le 0 ]; then
+            echo "::error title=No tidy coverage::the baseline found no clang-tidy issues, so the comparison below proves nothing."
+            failed=1
+          else
+            echo "PASS: baseline saw $BASE_TIDY clang-tidy diagnostic(s)"
+          fi
+
+          # --- auto-fix cleared the format issues ---
+          if [ "${FIX_FORMAT:-1}" -ne 0 ]; then
+            echo "::error title=Format issues remain::auto-fix ran but still reports $FIX_FORMAT clang-format issue(s)."
+            failed=1
+          else
+            echo "PASS: auto-fix cleared all clang-format issues"
+          fi
+
+          # --- the point of the tidy coverage ---
+          # clang-format's `-i` rewrites the file. clang-tidy reports line
+          # numbers from the file on disk, but its --line-filter and the review
+          # comments built from its output are keyed to the diff. If the tools
+          # run in the wrong order, diagnostics past a reflowed line drift out
+          # of the filter and silently disappear.
+          if [ "${FIX_TIDY:-0}" -ne "${BASE_TIDY:-0}" ]; then
+            echo "::error title=Tidy diagnostics drifted::auto-fix changed the clang-tidy count from $BASE_TIDY to $FIX_TIDY. Applying format fixes must not affect what clang-tidy reports."
+            failed=1
+          else
+            echo "PASS: clang-tidy still reports $FIX_TIDY diagnostic(s); auto-fix did not shift them"
+          fi
+
+          # --- the pull-request plumbing ---
+          head_sha="$(git rev-parse HEAD)"
+          subject="$(git log -1 --pretty=%s)"
+
+          if [ "$expect_fix" = "true" ]; then
+            if [ "$(git hash-object "$TEST_FILE")" = "$MALFORMED_BLOB" ]; then
+              echo "::error title=Not reformatted::$TEST_FILE is unchanged; --fix did not rewrite it."
+              failed=1
+            else
+              echo "PASS: fixture was reformatted"
+            fi
+
+            if [ "$head_sha" = "$PR_HEAD_SHA" ]; then
+              echo "::error title=No commit::auto-fix produced no commit on the PR branch."
+              failed=1
+            else
+              echo "PASS: auto-fix commit $head_sha"
+              echo "      subject: $subject"
+              echo "      author:  $(git log -1 --pretty='%an <%ae>')"
+            fi
+
+            if [ "$subject" != "$COMMIT_MSG" ]; then
+              echo "::error title=Wrong commit message::expected '$COMMIT_MSG', got '$subject'"
+              failed=1
+            else
+              echo "PASS: commit message matches auto-fix-commit-msg"
+            fi
+
+            # Only the fixture may appear in the auto-fix commit.
+            touched="$(git diff --name-only "$PR_HEAD_SHA" HEAD)"
+            if [ "$touched" != "$TEST_FILE" ]; then
+              echo "::error title=Unexpected files committed::auto-fix committed more than the fixture:"
+              echo "$touched"
+              failed=1
+            else
+              echo "PASS: the auto-fix commit contains only $TEST_FILE"
+            fi
+          else
+            # Idempotency: with nothing left to fix, auto-fix must not rewrite
+            # the file and must not manufacture an empty commit.
+            if [ "$(git hash-object "$TEST_FILE")" != "$MALFORMED_BLOB" ]; then
+              echo "::error title=Needless rewrite::auto-fix modified an already-formatted file."
+              failed=1
+            else
+              echo "PASS: already-clean fixture was left byte-identical"
+            fi
+
+            if [ "$head_sha" != "$PR_HEAD_SHA" ]; then
+              echo "::error title=Empty commit::auto-fix committed $head_sha despite having nothing to fix."
+              failed=1
+            else
+              echo "PASS: no commit was created when there was nothing to fix"
+            fi
+          fi
+
+          fmt="$(command -v "clang-format-${CLANG_VERSION}" || command -v clang-format || true)"
+          if [ -n "$fmt" ]; then
+            if "$fmt" --style=file --dry-run --Werror "$TEST_FILE"; then
+              echo "PASS: committed fixture satisfies .clang-format"
+            else
+              echo "::error title=Still unformatted::the committed fixture still violates .clang-format"
+              failed=1
+            fi
+          else
+            echo "note: clang-format not on PATH here; skipped the re-check"
+          fi
+
+          # The repo's intentionally-unformatted fixtures must be left alone.
+          if ! git diff --quiet "$PR_HEAD_SHA" HEAD -- src/demo.cpp src/demo.hpp; then
+            echo "::error title=Collateral damage::auto-fix also rewrote this repo's intentional demo fixtures."
+            failed=1
+          else
+            echo "PASS: src/demo.cpp and src/demo.hpp were left untouched"
+          fi
+
+          # The checkout above is a full clone; the action must leave it that way.
+          if [ "$(git rev-parse --is-shallow-repository)" != "false" ]; then
+            echo "::error title=History truncated::the action turned the full clone into a shallow one."
+            failed=1
+          else
+            echo "PASS: the full clone was left intact"
+          fi
+
+          # The fix has to be on the PR branch at the remote, not just locally.
+          git fetch -q origin "$head_ref"
+          if [ "$(git rev-parse FETCH_HEAD)" != "$head_sha" ]; then
+            echo "::error title=Not pushed::the auto-fix commit is not on origin/$head_ref"
+            failed=1
+          else
+            echo "PASS: auto-fix commit is present on origin/$head_ref"
+          fi
+
+          if [ "$failed" -eq 0 ]; then
+            echo "auto-fix PR e2e PASSED"
+          fi
+          exit "$failed"

src/e2e_autofix_pr_demo.cpp

@@ -0,0 +1,28 @@
+// Deliberately malformed fixture for the auto-fix e2e test.
+//
+// Two properties matter here:
+//   1. clang-format has plenty to fix, so auto-fix has something to commit.
+//   2. Formatting it changes its line count (the one-liners below expand),
+//      which is what makes it able to catch clang-tidy diagnostics drifting
+//      off the diff when the tools run in the wrong order.
+int accumulate()
+{
+    int a = 10;
+    int b = 20;
+    int c = 30;
+    int d = 40;
+    int e = 50;
+    return a + b + c + d + e;
+}
+
+int magic_user()
+{
+    int v = 42;
+    return v;
+}
+
+int* null_user()
+{
+    int* p = 0;
+    return p;
+}