Skip to content

test: wait for PR head sha after force-push in roundtrip harness#457

Merged
tomasz-tomczyk merged 1 commit intomainfrom
fix-456-anchor-flake
May 4, 2026
Merged

test: wait for PR head sha after force-push in roundtrip harness#457
tomasz-tomczyk merged 1 commit intomainfrom
fix-456-anchor-flake

Conversation

@tomasz-tomczyk
Copy link
Copy Markdown
Owner

Summary

  • After the test's git push --force rewinds the PR head SHA, GitHub takes a moment to recompute the PR diff. If the next crit push fires too soon, the new comment's commit_id no longer matches the PR's recomputed diff and GitHub rejects with HTTP 422.
  • Adds a waitForPRHeadSHA helper to roundtripEnv that polls GET /repos/{slug}/pulls/{N} until head.sha matches the local HEAD, with a 15s deadline (no raw time.Sleep).
  • Calls it from TestRoundtrip_AnchorLineDeleted_Outdated after the force-push, before the next crit pull / crit comment / crit push.

Closes #456.

Review

  • Code review: passed (test-only change, behind e2e_github build tag)
  • Parity audit: N/A (no frontend / no production code touched)

Test plan

  • gofmt clean, go vet clean, golangci-lint clean (default + e2e_github tag)
  • go test ./...: PASS
  • Targeted: ./scripts/e2e-roundtrip.sh -run TestRoundtrip_AnchorLineDeleted_Outdated -v — 7 consecutive PASS
  • Full suite: make e2e-roundtrip — 2 consecutive PASS

🤖 Generated with Claude Code

Fixes #456: TestRoundtrip_AnchorLineDeleted_Outdated could flake with HTTP
422 from gh api when run in the full suite. After the test's
git push --force rewinds the PR head, GitHub takes a moment to recompute
the PR diff. If the next crit push fires too soon, its commit_id no longer
matches the current PR head and GitHub rejects the new comment.

Add a waitForPRHeadSHA helper to roundtripEnv that polls
GET /repos/{slug}/pulls/{N} until head.sha matches the local HEAD, with a
15s deadline. Call it after the force-push in the
AnchorLineDeleted_Outdated scenario before issuing further crit commands.

Verified with 5x targeted runs and 2x full-suite runs (all green).
@tomasz-tomczyk tomasz-tomczyk merged commit 64a2f57 into main May 4, 2026
6 checks passed
@tomasz-tomczyk tomasz-tomczyk deleted the fix-456-anchor-flake branch May 4, 2026 20:56
@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.23%. Comparing base (0f9f7c1) to head (60ef7fb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #457      +/-   ##
==========================================
+ Coverage   68.18%   68.23%   +0.05%     
==========================================
  Files          35       35              
  Lines        9936     9936              
==========================================
+ Hits         6775     6780       +5     
+ Misses       2642     2639       -3     
+ Partials      519      517       -2     
Flag Coverage Δ
e2e 33.68% <ø> (ø)
unit 65.52% <ø> (+0.05%) ⬆️

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.

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.

TestRoundtrip_AnchorLineDeleted_Outdated flakes when run in full suite

1 participant