test(qa): fix topology warm-up flake in the blacklist script tests - #129
Merged
Merged
Conversation
CI run 36250468641 (master b3e4476) failed "topology warm-up then 200" while the same tree passed in PR CI. fetch_topology's deadline is $(date +%s) + RESTART_WAIT_S, in whole seconds, so a wait that starts late in a second has less than RESTART_WAIT_S. With RESTART_WAIT_S=4 and two 503s (3s apart), the retry after the second 503 is lost when the wait starts late enough. The fake target can now run the script on a fake clock: with FAKE_CLOCK_START_MS, `date +%s` and `sleep` in the logexec shim read and advance a millisecond clock instead of the real one, and each fake curl request takes FAKE_CLOCK_CALL_MS. The warm-up case now starts the topology wait at .950s with 50ms per request (a fixture guard checks that), which fails deterministically: the second 503 comes back at .050s past the deadline second. This commit is red on purpose; the next one fixes the test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019TcZHooUiiknVWbECVWzk8
fetch_topology's deadline is whole seconds, so it leaves between RESTART_WAIT_S-1 and RESTART_WAIT_S seconds. Two 503s need the retry decided about 3.1s after the wait starts, which RESTART_WAIT_S=4 misses from a start at .900s on. The warm-up and stuck cases now pass RESTART_WAIT_S=5 and run on the fake clock from two starts, .950s and .000s, with a fixture guard for each: - warm-up (two 503s, then 200): passes, clean, three topology requests; - stuck (503 forever): fails, classified as HTTP 503, retried at least once, and leaves config, databases and files as they were. On the fake clock these cases no longer sleep, so the suite is about 12s faster. blacklist-test.sh is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019TcZHooUiiknVWbECVWzk8
dborup
marked this pull request as ready for review
September 29, 2026 07:15
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.
Summary
Fixes a timing flake in the QA blacklist script's unit tests,
qa/scripts/test-blacklist-sql.sh. The change is test-only:qa/scripts/blacklist-test.shand everything undercmd/,public/and the workflows are unchanged.Reference: master CI run 36250468641 on
b3e44761failed the step "QA blacklist script unit tests" with 895 passed and 2 failed:The same tree passed 897/0 in PR CI, and a rerun of the step was green.
Cause
fetch_topologyinblacklist-test.shretries 503s until a deadline:The deadline has whole-second resolution, so the wait lasts between
RESTART_WAIT_S - 1andRESTART_WAIT_Sseconds. It is shortest when the wait starts late in a second.The warm-up case runs with
RESTART_WAIT_S=4and two 503s. The retry after the second 503 is decided about 3 s plus two requests after the wait starts, so a wait that starts late enough in a second gives up with the 503.Reproduction (commit 1
02a25f87, red on purpose)The fake target can now run the script on a fake clock. Real time plays no part.
FAKE_CLOCK_START_MS, thelogexecshim answersdate +%sfrom a millisecond clock file, andsleep Nadvances that clock instead of waiting.FAKE_CLOCK_CALL_MSand records its start time inclock.log.fetch_topologyreturns 503. The run reports❌ hide-failed: /api/analytics/topology HTTP 503, which is the same pair of failures as in CI, every time.With 50 ms requests,
RESTART_WAIT_S=4loses the retry for any wait that starts at .900 s or later.Fix (commit 2
2b37fa0e)The warm-up and stuck cases pass
RESTART_WAIT_S=5, which leaves at least 4 s for a retry needed at about 3.1 s. Both cases run on the fake clock from two starts, .950 s and .000 s, and each has a fixture guard. They still prove both behaviours:✅ topology clean, and exactly three topology requests./api/analytics/topology HTTP 503, retried at least once, and the usualcommon_afterchecks: config restored, databases and files unchanged, nothing sensitive in argv.These cases no longer sleep, so the suite also got faster.
Mutation checks
Each variant was run through the fixed suite; the variant files were not committed.
RESTART_WAIT_S=4blacklist-test.shwithout the 503 retryblacklist-test.shwithout the deadlineResults
bash qa/scripts/test-blacklist-sql.sh97cebd98shellcheck -x -P SCRIPTDIR qa/scripts/blacklist-test.sh qa/scripts/test-blacklist-sql.sh(ShellCheck 0.9.0) produces no findings.Note on
blacklist-test.shThe whole-second deadline also shortens the real tool's wait by up to one second. With the default
RESTART_WAIT_S=120and a 3 s poll interval, that does not matter. The tool is left unchanged; any improvement to it would be a separate change.🤖 Generated with Claude Code
https://claude.ai/code/session_019TcZHooUiiknVWbECVWzk8
Generated by Claude Code