Repository navigation
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Follow-up to #2897: give
tools/bazel-retry.sha behavioral test harness sothis copy can serve as the canonical one, with the retry/no-retry contract
pinned by tests rather than by reading the regex. (A second copy of this
wrapper lives in an internal repo; unifying the two is tracked internally,
and pinning this copy's behavior gives that comparison a fixed point.)
Also drops a misleading
execin the wrapper: inside the pipeline it onlyever replaced the subshell, so behavior is unchanged, but it reads like a bug
to anyone auditing the retry flow.
How was this verified?
python3 tools/test-bazel-retry.py(stdlib only, no bazel needed). It runsthe real script with a fake
bazelshimmed onto PATH that plays a scriptedper-attempt plan and logs every invocation, so each test asserts both the
exit code and exactly how many attempts the wrapper made. Covered: first-try
success; transient HTTP 503 retry; the #2897 remote-endpoint patterns
(capabilities-query connection-refused, bare
UNAVAILABLE:); a compile errorexiting 1 after exactly one attempt (the invariant that keeps red builds from
costing 3x CI); budget exhaustion after three transient failures; and argv
pass-through with embedded spaces. The attempt-count assertions fail against
a wrapper that retries unconditionally, and the suite takes ~30s because the
real 5s/10s backoff sleeps run.
Not covered here: reconciling the internal copy of the script and its test
harness. This PR just pins this copy's behavior so that comparison has a
fixed point.
Risk
Low. The wrapper change is a no-op
execremoval on a CI-only script; thetest file is new and runs nowhere automatically yet (happy to wire it into a
workflow here or leave that for the follow-up, maintainer's call).
AI assistance
Claude (Fable 5) drafted the harness and the
execremoval from my direction;I reviewed every line before opening this.