Skip to content

Fail CI if Git is dirty after running tests#27084

Open
EvanHahn wants to merge 3 commits intomainfrom
git-status-must-be-clean-after-running-tests
Open

Fail CI if Git is dirty after running tests#27084
EvanHahn wants to merge 3 commits intomainfrom
git-status-must-be-clean-after-running-tests

Conversation

@EvanHahn
Copy link
Copy Markdown
Contributor

@EvanHahn EvanHahn commented Apr 2, 2026

ref #27083
ref 353552f

Here's the prompt I gave to Claude Sonnet 4.6:

When I ran cd ghost/core && yarn test:e2e on a fresh clone, new snapshots were generated, and git status wasn't clean. That's not what I want!

Update the GitHub CI scripts so that, if git diff/git status isn't empty after running tests, CI fails.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 2, 2026

Walkthrough

A new CI validation step, "Check for unexpected file changes", was added to the workflow and inserted after the unit, acceptance, and legacy test jobs. The step runs git status --porcelain; if output is non-empty it prints git status and git diff and exits with code 1 to fail the job. Net change: +27/-0 lines.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding a CI check to fail tests when Git has uncommitted changes, which directly matches the raw summary of adding file change validation steps.
Description check ✅ Passed The description is directly related to the changeset, explaining the motivation for the changes (preventing snapshot generation from leaving Git dirty) and referencing the related issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch git-status-must-be-clean-after-running-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

394-402: Extract the dirty-worktree check into a shared script to avoid drift.

This block is now repeated three times (Line [394], Line [489], Line [579]). Consider centralizing it in one script (e.g., .github/scripts/check-clean-worktree.sh) and calling that script from each job.

♻️ Suggested refactor
-      - name: Check for unexpected file changes
-        run: |
-          if [ -n "$(git status --porcelain)" ]; then
-            echo "Tests generated unexpected file changes. Commit them before merging:"
-            git status
-            git diff
-            exit 1
-          fi
+      - name: Check for unexpected file changes
+        run: bash .github/scripts/check-clean-worktree.sh
#!/usr/bin/env bash
set -euo pipefail

if [ -n "$(git status --porcelain)" ]; then
  echo "Tests generated unexpected file changes. Commit them before merging:"
  git status
  git diff
  exit 1
fi

Also applies to: 489-497, 579-587

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 394 - 402, The "Check for unexpected
file changes" step is duplicated across multiple CI jobs; extract the logic into
a single executable script named check-clean-worktree.sh, add the exact content
shown (#!/usr/bin/env bash with set -euo pipefail and the git status --porcelain
check that prints git status/git diff and exits 1), commit that script, and
replace each duplicated step in the workflow (the steps titled "Check for
unexpected file changes") with a simple invocation of that script (e.g., run:
./check-clean-worktree.sh) so all jobs reuse the same implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 394-402: The "Check for unexpected file changes" step is
duplicated across multiple CI jobs; extract the logic into a single executable
script named check-clean-worktree.sh, add the exact content shown
(#!/usr/bin/env bash with set -euo pipefail and the git status --porcelain check
that prints git status/git diff and exits 1), commit that script, and replace
each duplicated step in the workflow (the steps titled "Check for unexpected
file changes") with a simple invocation of that script (e.g., run:
./check-clean-worktree.sh) so all jobs reuse the same implementation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: df793ea7-68fa-4ecb-8583-3440a3a868cc

📥 Commits

Reviewing files that changed from the base of the PR and between 93f5831 and af6f1c9.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml

@EvanHahn EvanHahn changed the base branch from main to yarn-test-e2e-adding-snapshots April 2, 2026 16:34
@EvanHahn EvanHahn force-pushed the git-status-must-be-clean-after-running-tests branch from af6f1c9 to 2c65ff4 Compare April 2, 2026 16:35
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.20%. Comparing base (93f5831) to head (49a8f20).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #27084      +/-   ##
==========================================
- Coverage   73.20%   73.20%   -0.01%     
==========================================
  Files        1531     1531              
  Lines      121832   121808      -24     
  Branches    14701    14699       -2     
==========================================
- Hits        89190    89169      -21     
- Misses      31625    31645      +20     
+ Partials     1017      994      -23     
Flag Coverage Δ
admin-tests 54.41% <ø> (+0.02%) ⬆️
e2e-tests 73.20% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@EvanHahn EvanHahn requested a review from 9larsons April 2, 2026 17:06
EvanHahn added a commit that referenced this pull request Apr 2, 2026
ref 353552f
ref #27084

We recently added some tests which generate new snapshots, but forgot to
commit them. This fixes that.

#27084 prevents this from happening again.

All I did was run `cd ghost/core && yarn test:e2e`. I don't really know
what this snapshot is about.
Base automatically changed from yarn-test-e2e-adding-snapshots to main April 2, 2026 17:08
ref #27083
ref 353552f

Here's the prompt I gave to Claude Sonnet 4.6:

> When I ran `cd ghost/core && yarn test:e2e` on a fresh clone, new
> snapshots were generated, and `git status` wasn't clean. That's not
> what I want!
>
> Update the GitHub CI scripts so that, if `git diff`/`git status` isn't
> empty after running tests, CI fails.
@EvanHahn EvanHahn force-pushed the git-status-must-be-clean-after-running-tests branch from 2c65ff4 to a323377 Compare April 2, 2026 17:09
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

579-586: LGTM!

Consistent with the other test jobs.

Consider extracting this check into a small composite action (e.g., .github/actions/check-git-clean) if you anticipate adding it to more test jobs in the future. For now, the duplication is minimal and acceptable.

♻️ Optional: Composite action for reuse

Create .github/actions/check-git-clean/action.yml:

name: Check for unexpected file changes
description: Fails if git working tree is dirty after tests

runs:
  using: composite
  steps:
    - shell: bash
      run: |
        if [ -n "$(git status --porcelain)" ]; then
          echo "Tests generated unexpected file changes. Commit them before merging:"
          git status
          git diff
          exit 1
        fi

Then replace each inline step with:

- name: Check for unexpected file changes
  uses: ./.github/actions/check-git-clean
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 579 - 586, The inline shell step named
"Check for unexpected file changes" is duplicated and should be extracted into a
reusable composite action; create a composite action (e.g., named
check-git-clean) that runs the existing bash snippet to fail when git status is
dirty, then replace each inline step with a uses entry that invokes that
composite action (keep the displayed name "Check for unexpected file changes"
for clarity).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 579-586: The inline shell step named "Check for unexpected file
changes" is duplicated and should be extracted into a reusable composite action;
create a composite action (e.g., named check-git-clean) that runs the existing
bash snippet to fail when git status is dirty, then replace each inline step
with a uses entry that invokes that composite action (keep the displayed name
"Check for unexpected file changes" for clarity).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 12b8fe89-7136-4ed5-a384-1ed4fb9190bd

📥 Commits

Reviewing files that changed from the base of the PR and between af6f1c9 and a323377.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml

@EvanHahn
Copy link
Copy Markdown
Contributor Author

EvanHahn commented Apr 2, 2026

CI is failing, which will be fixed with TryGhost/Ghost-CLI#2109.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 2, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant