Skip to content

Commit f60c91e

Browse files
ilblackdragonclaudeCopilot
authored
ci: enforce regression tests for fix commits (#517)
* ci: enforce regression tests for fix commits Add a commit-msg hook and CI workflow that require test changes alongside bug fix commits, ensuring every fix includes a regression test that would have caught the bug. - scripts/commit-msg-regression.sh: local git hook (blocks fix commits without test changes; exempts static/docs-only; bypass via [skip-regression-check] marker) - .github/workflows/regression-test-check.yml: CI mirror on PRs (checks title + commit messages; skip via label) - scripts/dev-setup.sh: install hook in step 6 - .github/scripts/create-labels.sh: add skip-regression-check label - CLAUDE.md: document regression test policy Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback on regression test enforcement - Use here-strings instead of echo|grep to avoid misinterpreting special characters in variables - Use git diff -W (whole-function context) to detect edits inside existing test functions, not just new #[test] attributes - Honor [skip-regression-check] in commit messages in CI (not just the PR label) - Use git rev-parse --git-path hooks for worktree-safe hook install [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update .github/workflows/regression-test-check.yml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent a22d44f commit f60c91e

5 files changed

Lines changed: 211 additions & 5 deletions

File tree

.github/scripts/create-labels.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ create "scope: ci" "546E7A" "CI/CD workflows"
6262
create "scope: docs" "78909C" "Documentation"
6363
create "scope: dependencies" "90A4AE" "Dependency updates"
6464

65+
echo "==> Creating workflow labels..."
66+
create "skip-regression-check" "9E9E9E" "Acknowledged: fix without regression test"
67+
6568
echo "==> Creating contributor labels..."
6669
create "contributor: new" "FFF9C4" "First-time contributor"
6770
create "contributor: regular" "FFE082" "2-5 merged PRs"
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
name: Regression Test Check
2+
3+
on:
4+
pull_request:
5+
6+
jobs:
7+
regression-test:
8+
name: Regression test enforcement
9+
runs-on: ubuntu-latest
10+
steps:
11+
- name: Checkout repository
12+
uses: actions/checkout@v4
13+
with:
14+
fetch-depth: 0
15+
16+
- name: Check for regression tests
17+
env:
18+
PR_TITLE: ${{ github.event.pull_request.title }}
19+
PR_LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }}
20+
run: |
21+
set -euo pipefail
22+
23+
BASE_REF="origin/${{ github.event.pull_request.base.ref }}"
24+
25+
# --- 1. Is this a fix PR? Check title first, then commit messages ---
26+
IS_FIX=false
27+
28+
if grep -qiE '^(fix(\(.*\))?|hotfix|bugfix):' <<< "$PR_TITLE"; then
29+
IS_FIX=true
30+
fi
31+
32+
if [ "$IS_FIX" = false ]; then
33+
COMMITS=$(git log --format='%s' "${BASE_REF}..HEAD")
34+
if grep -qiE '^(fix(\(.*\))?|hotfix|bugfix):' <<< "$COMMITS"; then
35+
IS_FIX=true
36+
fi
37+
fi
38+
39+
if [ "$IS_FIX" = false ]; then
40+
echo "Not a fix PR — skipping regression test check."
41+
exit 0
42+
fi
43+
44+
echo "Fix PR detected."
45+
46+
# --- 2. Skip label or commit message marker ---
47+
if grep -qF ',skip-regression-check,' <<< ",$PR_LABELS,"; then
48+
echo "skip-regression-check label present — skipping."
49+
exit 0
50+
fi
51+
52+
COMMIT_BODIES=$(git log --format='%B' "${BASE_REF}..HEAD")
53+
if grep -qF '[skip-regression-check]' <<< "$COMMIT_BODIES"; then
54+
echo "[skip-regression-check] found in commit message — skipping."
55+
exit 0
56+
fi
57+
58+
# --- 3. Exempt static-only / docs-only changes ---
59+
CHANGED_FILES=$(git diff --name-only "${BASE_REF}...HEAD")
60+
61+
if [ -z "$CHANGED_FILES" ]; then
62+
echo "No changed files — skipping."
63+
exit 0
64+
fi
65+
66+
ALL_EXEMPT=true
67+
while IFS= read -r file; do
68+
case "$file" in
69+
src/channels/web/static/*) ;;
70+
*.md) ;;
71+
*) ALL_EXEMPT=false; break ;;
72+
esac
73+
done <<< "$CHANGED_FILES"
74+
75+
if [ "$ALL_EXEMPT" = true ]; then
76+
echo "All changes are static assets or docs — skipping."
77+
exit 0
78+
fi
79+
80+
# --- 4. Look for test changes ---
81+
82+
# Fast path: new test attributes or test modules in added lines.
83+
if git diff "${BASE_REF}...HEAD" -U0 -- '*.rs' | grep -qE '^\+.*(#\[test\]|#\[tokio::test\]|#\[cfg\(test\)\]|mod tests)'; then
84+
echo "Test changes found in .rs files."
85+
exit 0
86+
fi
87+
88+
# Whole-function context: detect edits inside existing test functions.
89+
if git diff "${BASE_REF}...HEAD" -W -- '*.rs' | awk '
90+
/^@@/ { if (has_test && has_add) { found=1; exit } has_test=0; has_add=0 }
91+
/^ .*#\[test\]/ || /^ .*#\[tokio::test\]/ || /^ .*#\[cfg\(test\)\]/ || /^ .*mod tests/ { has_test=1 }
92+
/^\+.*#\[test\]/ || /^\+.*#\[tokio::test\]/ || /^\+.*#\[cfg\(test\)\]/ || /^\+.*mod tests/ { has_test=1 }
93+
/^\+[^+]/ { has_add=1 }
94+
END { if (has_test && has_add) found=1; exit !found }
95+
'; then
96+
echo "Test changes found in existing test functions."
97+
exit 0
98+
fi
99+
100+
if grep -qE '^tests/' <<< "$CHANGED_FILES"; then
101+
echo "Test file changes found under tests/."
102+
exit 0
103+
fi
104+
105+
# --- 5. No tests found ---
106+
echo "::warning::This PR looks like a bug fix but contains no test changes. Every fix should include a regression test. Add a #[test] or #[tokio::test], or apply the 'skip-regression-check' label if not feasible."
107+
exit 1

CLAUDE.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,13 +321,16 @@ cargo check --all-features # all features
321321
```
322322
Dead code behind the wrong `#[cfg]` gate will only show up when building with a single feature.
323323

324+
**Regression test with every fix:** Every bug fix must include a test that would have caught the bug. Add a `#[test]` or `#[tokio::test]` that reproduces the original failure. Exempt: changes limited to `src/channels/web/static/` or `.md` files. Use `[skip-regression-check]` in commit message or PR label if genuinely not feasible. The `commit-msg` hook and CI workflow enforce this automatically.
325+
324326
**Zero clippy warnings policy:** Fix ALL clippy warnings before committing, including pre-existing ones in files you didn't change. Never leave warnings behind — treat `cargo clippy` output as a zero-tolerance gate.
325327

326328
**Mechanical verification before committing:** Run these checks on changed files before committing:
327329
- `cargo clippy --all --benches --tests --examples --all-features` -- zero warnings
328330
- `grep -rnE '\.unwrap\(|\.expect\(' <files>` -- no panics in production
329331
- `grep -rn 'super::' <files>` -- use `crate::` imports
330332
- If you fixed a pattern bug, `grep` for other instances of that pattern across `src/`
333+
- Fix commits must include regression tests (enforced by `commit-msg` hook; bypass with `[skip-regression-check]`)
331334

332335
## Configuration
333336

scripts/commit-msg-regression.sh

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
#!/usr/bin/env bash
2+
# commit-msg hook: require regression tests for fix commits.
3+
#
4+
# Installed by scripts/dev-setup.sh as .git/hooks/commit-msg.
5+
# Bypass with [skip-regression-check] in the commit message.
6+
7+
set -euo pipefail
8+
9+
MSG_FILE="$1"
10+
FIRST_LINE=$(head -1 "$MSG_FILE")
11+
12+
# --- 1. Is this a fix commit? ---
13+
if ! grep -qiE '^(fix(\(.*\))?|hotfix|bugfix):' <<< "$FIRST_LINE"; then
14+
exit 0
15+
fi
16+
17+
# --- 2. Skip marker ---
18+
if grep -qF '[skip-regression-check]' "$MSG_FILE"; then
19+
exit 0
20+
fi
21+
22+
# --- 3. Exempt static-only / docs-only changes ---
23+
# Get staged files (commit-msg runs after staging is finalized).
24+
STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACMR)
25+
26+
if [ -z "$STAGED_FILES" ]; then
27+
exit 0
28+
fi
29+
30+
ALL_EXEMPT=true
31+
while IFS= read -r file; do
32+
case "$file" in
33+
src/channels/web/static/*) ;;
34+
*.md) ;;
35+
*) ALL_EXEMPT=false; break ;;
36+
esac
37+
done <<< "$STAGED_FILES"
38+
39+
if [ "$ALL_EXEMPT" = true ]; then
40+
exit 0
41+
fi
42+
43+
# --- 4. Look for test changes in staged .rs files ---
44+
45+
# Fast path: new test attributes or test modules in added lines.
46+
if git diff --cached -U0 -- '*.rs' | grep -qE '^\+.*(#\[test\]|#\[tokio::test\]|#\[cfg\(test\)\]|mod tests)'; then
47+
exit 0
48+
fi
49+
50+
# Whole-function context: detect edits inside existing test functions.
51+
# -W shows the full enclosing function, so #[test] appears in context
52+
# lines when changes are inside a test function.
53+
if git diff --cached -W -- '*.rs' | awk '
54+
/^@@/ { if (has_test && has_add) { found=1; exit } has_test=0; has_add=0 }
55+
/^ .*#\[test\]/ || /^ .*#\[tokio::test\]/ || /^ .*#\[cfg\(test\)\]/ || /^ .*mod tests/ { has_test=1 }
56+
/^\+.*#\[test\]/ || /^\+.*#\[tokio::test\]/ || /^\+.*#\[cfg\(test\)\]/ || /^\+.*mod tests/ { has_test=1 }
57+
/^\+[^+]/ { has_add=1 }
58+
END { if (has_test && has_add) found=1; exit !found }
59+
'; then
60+
exit 0
61+
fi
62+
63+
# Also check for new/modified files under tests/
64+
if grep -qE '^tests/' <<< "$STAGED_FILES"; then
65+
exit 0
66+
fi
67+
68+
# --- 5. No test found — block the commit ---
69+
echo ""
70+
echo "╔══════════════════════════════════════════════════════════════╗"
71+
echo "║ REGRESSION TEST REQUIRED ║"
72+
echo "║ ║"
73+
echo "║ This commit looks like a bug fix but has no test changes. ║"
74+
echo "║ Every fix should include a test that reproduces the bug. ║"
75+
echo "║ ║"
76+
echo "║ Options: ║"
77+
echo "║ • Add a #[test] or #[tokio::test] that catches the bug ║"
78+
echo "║ • Add [skip-regression-check] to your commit message ║"
79+
echo "╚══════════════════════════════════════════════════════════════╝"
80+
echo ""
81+
exit 1

scripts/dev-setup.sh

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,40 @@ if ! command -v rustup &>/dev/null; then
2424
echo "ERROR: rustup not found. Install from https://rustup.rs"
2525
exit 1
2626
fi
27-
echo "[1/5] rustup found: $(rustup --version 2>/dev/null | head -1)"
27+
echo "[1/6] rustup found: $(rustup --version 2>/dev/null | head -1)"
2828

2929
# 2. Add WASM target (required by build.rs for channel compilation)
30-
echo "[2/5] Adding wasm32-wasip2 target..."
30+
echo "[2/6] Adding wasm32-wasip2 target..."
3131
rustup target add wasm32-wasip2
3232

3333
# 3. Install wasm-tools (required by build.rs for WASM component model)
34-
echo "[3/5] Installing wasm-tools..."
34+
echo "[3/6] Installing wasm-tools..."
3535
if command -v wasm-tools &>/dev/null; then
3636
echo " wasm-tools already installed: $(wasm-tools --version)"
3737
else
3838
cargo install wasm-tools --locked
3939
fi
4040

4141
# 4. Verify the project compiles
42-
echo "[4/5] Running cargo check..."
42+
echo "[4/6] Running cargo check..."
4343
cargo check
4444

4545
# 5. Run tests using libsql temp DB (no Docker/external DB needed)
46-
echo "[5/5] Running tests (no external DB required)..."
46+
echo "[5/6] Running tests (no external DB required)..."
4747
cargo test
4848

49+
# 6. Install git hooks
50+
echo "[6/6] Installing git hooks..."
51+
HOOKS_DIR=$(git rev-parse --git-path hooks 2>/dev/null) || true
52+
if [ -n "$HOOKS_DIR" ]; then
53+
mkdir -p "$HOOKS_DIR"
54+
SCRIPT_ABS="$(cd "$(dirname "$0")" && pwd)/commit-msg-regression.sh"
55+
ln -sf "$SCRIPT_ABS" "$HOOKS_DIR/commit-msg"
56+
echo " commit-msg hook installed (regression test enforcement)"
57+
else
58+
echo " Skipped: not a git repository"
59+
fi
60+
4961
echo ""
5062
echo "=== Setup complete ==="
5163
echo ""

0 commit comments

Comments
 (0)