test: cut the -short carve-out at 4s by gzliudan · Pull Request #2947 · XinFinOrg/XDPoSChain

gzliudan · 2026-10-29T10:31:11Z

Summary

short-mode carve-out, by package
  p2p/discover                 10 tests, ~4s each (timeout waits)
  rpc                           3 tests, 4-10s (teardown, unsubscribe)
  accounts/abi/abigen           1 test,  12s (bindings)
  ethclient/simulated           1 test,  6s
  metrics                       1 test,  5.3s
  core/txpool/legacypool        1 test,  5.3s
  eth                           1 test,  4s (tx announcement)

go test ./... -count=1 -json -p 12 reports 7066 test entries, 76 at 4s or more. 22 of those had no short-mode carve-out; this PR adds it.

consensus/tests/engine_v2_tests goes the other way. 52 of its 109 tests carry the gate, and 42 of those cost under 4s - the cheapest, TestTimeoutPoolKeyGoodHygiene, costs 0.14s. Those 42 lose the gate; the 10 at 4.14s or more keep it (4.14, 4.33, 4.35, 4.36, 4.48, 5.15, 5.15, 8.70, 9.14, 23.82s). One test that dev-upgrade leaves ungated is over the line here - TestProcessQcShallSetForensicsCommittedQc, 5.14s - so it gains the gate.

That gate is temporary by design. The 5.14s is a literal five-second sleep, not work, and #2984 replaces the sleep with a poll that returns in 0.18s - at which point the gate comes off with it.

The figures are the median per-test elapsed of three full runs of the package with -parallel 1. #2944 has landed, and it both shortened the V2 periods and let the suite run its tests in parallel, so a default run charges each test the contention of its neighbours rather than its own cost.

One helper instead of three. skipLongInShortMode existed in three packages: a byte-identical short_mode_test.go in p2p/discover, a definition inside common/countdown's test file, and consensus/tests/engine_v2_tests/helper.go. Go cannot import a symbol from another package's _test.go, so every package that wanted the gate copied the five lines, and the copies could drift. It now lives in internal/testshort, and every call site calls it: testshort.Skip(t) for tests and testshort.Skip(b) for the six benchmark guards in core/bench_test.go - which is why it takes testing.TB rather than *testing.T. Same skip string, same t.Helper(), no behaviour change.

That package is the one place where this PR makes -short slower: 12.9s -> 21.7s, 98 of 109 tests instead of 57. Under -race -short it is 22.8s -> 48.2s. Both are what running 42 tests of 0.14-2.90s costs, and both are cheaper than the same trade was before #2944 landed, when 19 un-gated tests cost 44.1s and 116.5s.

Four candidates were dropped after measuring them alone - their batch figures are contention, not cost:

test	in the `-p 12` batch	alone
`core.TestChainIndexerSingle`	4.79s	1.47s
`core.TestPrunedImportSide`	4.65s	3.65s
`core/bloombits.TestSchedulerSingleClientSingleFetcher`	4.59s	3.84s
`trie.TestIncompleteSyncHash`	4.02s	3.74s

Apart from the engine_v2 trim above, no gated test turned out to be under 4s, so nothing else lost its carve-out.

Evidence

go test -short -count=1, before (dev-upgrade) and after:

package	before	after
`p2p/discover`	28.80s	5.58s
`rpc`	24.97s	7.92s
`consensus/tests/engine_v2_tests`	12.94s (57 tests run)	21.7s (98 run)
`eth`	15.66s	11.65s
`core/txpool/legacypool`	6.25s	1.13s
`ethclient/simulated`	6.12s	0.07s
`accounts/abi/abigen`	5.75s	0.45s
`metrics`	5.51s	0.17s

consensus/tests/engine_v2_tests is the only package that grows. It is also the only one whose growth buys coverage rather than cost: 98 of 109 tests run under -short instead of 57, and the full run is unchanged at 68s on this tree. The -race -short figure of 48.2s is the price of running those 42 under the detector.

go vet clean on every package touched; the full (non-short) suite is untouched by these edits - testshort.Skip is a no-op outside -short, and the full engine_v2 run stays at 219.1s on dev-upgrade.

Merge Danger

Door: two-way. Skipped tests still run in the normal suite; reverting restores the -short behaviour.

Blast Radius: CI coverage. The race job runs -race -short, so 19 tests stop running under the race detector - the 18 above, including rpc.TestUnsubscribeTimeout and trie-adjacent sync paths that are race-flavoured by nature, plus TestProcessQcShallSetForensicsCommittedQc. That last one is #2984's to fix: it turns the 5s sleep into a poll and removes the gate, so the detector gets it back. If any of the others should stay in the race job, the alternative is to gate them on a different flag rather than on -short. Note that -short is the make quick-test carve-out: #2912 moves the CI race job to the full suite, after which this PR only affects quick-test.

engine_v2 timing: after #2944 the package's -short cost grows by 8.8s and its -race -short cost by 25.4s, and 42 tests regain -short coverage in return.

coderabbitai · 2026-10-29T10:31:20Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 0a5cc43f-571a-4a1a-a180-25f32a2a1591

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

Trigger review

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

skipLongInShortMode was copied into three packages: a byte-identical short_mode_test.go in p2p/discover, a definition inside common/countdown's test file, and consensus/tests/engine_v2_tests/helper.go. Go cannot import a symbol from another package's _test.go, which is why the copies grew - and the gates this series adds would each have brought another one. Move the five-line helper to internal/testshort and call testshort.Skip at the 82 call sites: 76 tests through testshort.Skip(t) and the six benchmark guards in core/bench_test.go through testshort.Skip(b), which is why Skip takes testing.TB rather than *testing.T. The skip string and the t.Helper() marking are unchanged, so a skipped test still reports the line that asked to be skipped; the benchmarks give up their own "Skipping in short-mode" wording for the shared one. No behaviour change: -short skips exactly the same tests and benchmarks, and the full runs are untouched.

…behind -short Across the tree `go test ./... -count=1 -json -p 12` reports 7066 test entries, 76 of them at 4s or more. 22 had no short-mode carve-out; the rest already used the shared helper or testing.Short(). Add the carve-out where it was missing, through testshort.Skip: - p2p/discover: 10 tests, each waiting ~4s for a timeout - rpc: 3 teardown/unsubscribe tests (4-10s) - accounts/abi/abigen, ethclient/simulated, metrics, core/txpool/legacypool, eth: one each (5-12s) Four candidates were dropped after measuring them alone: their batch figures (4.6-4.8s) are contention, not cost (TestChainIndexerSingle: 4.79s in the batch, 1.47s alone), so they keep running in -short. No gated test turned out to be under 4s, so nothing lost its carve-out. Note the CI race job runs -race -short: these 18 tests no longer run there.

The shared gate covers 52 of the package's 109 tests, but the gate exists for cost, and most of those tests are cheap now that the fixture rework (XinFinOrg#2944) shortens the V2 periods and lets the suite run its tests in parallel: 42 of the 52 cost under 4s, the cheapest 0.14s. Cut the carve-out at 4s, using the median per-test elapsed of three full runs of the package with -parallel 1. That is what a test costs on its own; the default parallel run charges it the contention of its neighbours, which is not what the gate is for. The rework is also why the figures move: this branch was cut when the package ran one test at a time, and XinFinOrg#2944 landed in between. 11 tests keep the gate, 4.14s to 23.82s 42 tests lose it, 0.14s to 2.90s One test that dev-upgrade leaves ungated is over the line on this tree - TestProcessQcShallSetForensicsCommittedQc, 5.14s - so it gains the gate. -short runs 98 of the 109 tests and grows from 12.9s to 21.7s; the full run is unchanged. The 8.8s buys -short and -race -short coverage of 42 tests, and it is cheap because their fixtures overlap: on the pre-XinFinOrg#2944 tree the same 42 tests would have cost about 32s one at a time.

…ng it TestProcessQcShallSetForensicsCommittedQc slept five seconds and only then asserted that three committed QCs had been recorded. XinFinOrg#2947 gates it at 4s for that cost, which is fair: the sleep is five seconds of nothing. The sleep was there because the forensics record had no lock. Polling it read the field while the committer goroutine wrote it, so the test would have traded a hidden race for a reported one rather than removing either - that is why XinFinOrg#2944 kept the sleep when it cut the package's other fixed waits. XinFinOrg#2946 then put the read and the write under the same mutex, and GetCommittedQCs returns a copy, so the poll is safe now: TestProcessQcShallSetForensicsCommittedQc 5.17s -> 0.176s The five seconds survive as a deadline, so the test keeps its teeth: dropping the vote that triggers the commit makes it fail after the same five seconds with "expected: 3, actual: 0". At 0.176s it is no longer a 4s test, so XinFinOrg#2947's gate comes off with it. -short runs it again, and so does -race -short.

Five timer tests here waited four to five seconds per assertion, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the only requirement is that a mid-flight Reset lands before the timeout, or the select takes the callback and the test reports a reset that never happened. At 5000ms/4000ms that margin was one second. At 500ms/200ms it is 300ms, which still clears scheduler jitter under `-race` with a single CPU and every core busy, and every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short and in the `-race -short` CI job, covering the resetc/quitc Reset path there for the first time. The package costs 38.0s full and 1.0s -short before, 1.2s both ways after.

testSyncTransaction gave the fetcher one second for the whole round trip: all 1024 transactions had to be announced or broadcast, fetched and pooled before `time.After(time.Second)` fired. Upstream ethereum#20234 built the timer inside the select, so it re-armed whenever a batch arrived; hoisting it out during the eth/65 fetch port (XinFinOrg#2490) turned a liveness check into a latency assertion. On a loaded race shard that is a coin flip: `race (bulk 4/4)` failed with "Failed to retrieve all transaction" while the neighbouring commits passed the same job, and the package ran 90s there against 24s a run earlier. Since XinFinOrg#2947 the announcement variant is gated behind -short, so the race job no longer runs it; the non-short tests job still does. Arm a single idle timer on every batch instead, and report how many transactions did arrive so the next failure is diagnosable. Comparing against len(alltxs) replaces the `got == 1024` literal, and a count above the batch size now fails immediately with the duplicate diagnosis. A second timer caps the whole wait at two minutes, so a fetcher that keeps trickling just under the idle limit cannot outlive the package timeout. Ref: ethereum#20234

) testSyncTransaction gave the fetcher one second for the whole round trip: all 1024 transactions had to be announced or broadcast, fetched and pooled before `time.After(time.Second)` fired. Upstream ethereum#20234 built the timer inside the select, so it re-armed whenever a batch arrived; hoisting it out during the eth/65 fetch port (#2490) turned a liveness check into a latency assertion. On a loaded race shard that is a coin flip: `race (bulk 4/4)` failed with "Failed to retrieve all transaction" while the neighbouring commits passed the same job, and the package ran 90s there against 24s a run earlier. Since #2947 the announcement variant is gated behind -short, so the race job no longer runs it; the non-short tests job still does. Arm a single idle timer on every batch instead, and report how many transactions did arrive so the next failure is diagnosable. Comparing against len(alltxs) replaces the `got == 1024` literal, and a count above the batch size now fails immediately with the duplicate diagnosis. A second timer caps the whole wait at two minutes, so a fetcher that keeps trickling just under the idle limit cannot outlive the package timeout. Ref: ethereum#20234

Five timer tests here waited four to five seconds per assertion, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the only requirement is that a mid-flight Reset lands before the timeout, or the select takes the callback and the test reports a reset that never happened. At 5000ms/4000ms that margin was one second. At 500ms/200ms it is 300ms, which still clears scheduler jitter under `-race` with a single CPU and every core busy, and every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short and in the `-race -short` CI job, covering the resetc/quitc Reset path there for the first time. The package costs 38.0s full and 1.0s -short before, 1.2s both ways after.

…ng it TestProcessQcShallSetForensicsCommittedQc slept five seconds and only then asserted that three committed QCs had been recorded. XinFinOrg#2947 gates it at 4s for that cost, which is fair: the sleep is five seconds of nothing. The sleep was there because the forensics record had no lock. Polling it read the field while the committer goroutine wrote it, so the test would have traded a hidden race for a reported one rather than removing either - that is why XinFinOrg#2944 kept the sleep when it cut the package's other fixed waits. XinFinOrg#2946 then put the read and the write under the same mutex, and GetCommittedQCs returns a copy, so the poll is safe now: TestProcessQcShallSetForensicsCommittedQc 5.17s -> 0.176s The five seconds survive as a deadline, so the test keeps its teeth: dropping the vote that triggers the commit makes it fail after the same five seconds with "expected: 3, actual: 0". At 0.176s it is no longer a 4s test, so XinFinOrg#2947's gate comes off with it. -short runs it again, and so does -race -short.

Five timer tests here waited four to five seconds per assertion, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the only requirement is that a mid-flight Reset lands before the timeout, or the select takes the callback and the test reports a reset that never happened. At 5000ms/4000ms that margin was one second. At 500ms/200ms it is 300ms, which still clears scheduler jitter under `-race` with a single CPU and every core busy, and every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short and in the `-race -short` CI job, covering the resetc/quitc Reset path there for the first time. The package costs 38.0s full and 1.0s -short before, 1.2s both ways after.

…ng it TestProcessQcShallSetForensicsCommittedQc slept five seconds and only then asserted that three committed QCs had been recorded. XinFinOrg#2947 gates it at 4s for that cost, which is fair: the sleep is five seconds of nothing. The sleep was there because the forensics record had no lock. Polling it read the field while the committer goroutine wrote it, so the test would have traded a hidden race for a reported one rather than removing either - that is why XinFinOrg#2944 kept the sleep when it cut the package's other fixed waits. XinFinOrg#2946 then put the read and the write under the same mutex, and GetCommittedQCs returns a copy, so the poll is safe now: TestProcessQcShallSetForensicsCommittedQc 5.17s -> 0.176s The five seconds survive as a deadline, so the test keeps its teeth: dropping the vote that triggers the commit makes it fail after the same five seconds with "expected: 3, actual: 0". At 0.176s it is no longer a 4s test, so XinFinOrg#2947's gate comes off with it. -short runs it again, and so does -race -short.

…ling TestProcessQcShallSetForensicsCommittedQc slept five seconds and only then asserted that three committed QCs had been recorded. XinFinOrg#2947 gates it at 4s for that cost, which is fair: the sleep is five seconds of nothing. The sleep was there because the forensics record had no lock. Polling it read the field while the committer goroutine wrote it, so the test would have traded a hidden race for a reported one rather than removing either - that is why XinFinOrg#2944 kept the sleep when it cut the package's other fixed waits. XinFinOrg#2946 then put the read and the write under the same mutex, and GetCommittedQCs returns a copy, so the poll is safe now: TestProcessQcShallSetForensicsCommittedQc 5.17s -> 0.176s The five seconds survive as a deadline, so the test keeps its teeth: dropping the vote that triggers the commit makes it fail after the same five seconds with "expected: 3, actual: 0". At 0.176s it is no longer a 4s test, so XinFinOrg#2947's gate comes off with it. -short runs it again, and so does -race -short.

…ling TestProcessQcShallSetForensicsCommittedQc slept five seconds and only then asserted that three committed QCs had been recorded. XinFinOrg#2947 gates it at 4s for that cost, which is fair: the sleep is five seconds of nothing. The sleep was there because the forensics record had no lock. Polling it read the field while the committer goroutine wrote it, so the test would have traded a hidden race for a reported one rather than removing either - that is why XinFinOrg#2944 kept the sleep when it cut the package's other fixed waits. XinFinOrg#2946 then put the read and the write under the same mutex, and GetCommittedQCs returns a copy, so the poll is safe now: TestProcessQcShallSetForensicsCommittedQc 5.17s -> 0.176s The five seconds survive as a deadline, so the test keeps its teeth: dropping the vote that triggers the commit makes it fail after the same five seconds, reporting "[]" should have 3 item(s), but has 0. At 0.176s it is no longer a 4s test, so XinFinOrg#2947's gate comes off with it. -short runs it again, and so does -race -short.

…ling (#2984) TestProcessQcShallSetForensicsCommittedQc slept five seconds and only then asserted that three committed QCs had been recorded. #2947 gates it at 4s for that cost, which is fair: the sleep is five seconds of nothing. The sleep was there because the forensics record had no lock. Polling it read the field while the committer goroutine wrote it, so the test would have traded a hidden race for a reported one rather than removing either - that is why #2944 kept the sleep when it cut the package's other fixed waits. #2946 then put the read and the write under the same mutex, and GetCommittedQCs returns a copy, so the poll is safe now: TestProcessQcShallSetForensicsCommittedQc 5.17s -> 0.176s The five seconds survive as a deadline, so the test keeps its teeth: dropping the vote that triggers the commit makes it fail after the same five seconds, reporting "[]" should have 3 item(s), but has 0. At 0.176s it is no longer a 4s test, so #2947's gate comes off with it. -short runs it again, and so does -race -short.

Five timer tests here waited four to five seconds per assertion, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the only requirement is that a mid-flight Reset lands before the timeout, or the select takes the callback and the test reports a reset that never happened. At 5000ms/4000ms that margin was one second. At 500ms/200ms it is 300ms, which still clears scheduler jitter under `-race` with a single CPU and every core busy, and every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short and in the `-race -short` CI job, covering the resetc/quitc Reset path there for the first time. The package costs 38.0s full and 1.0s -short before, 1.2s both ways after.

Five timer tests here waited four to five seconds per assertion, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the only requirement is that a mid-flight Reset lands before the timeout, or the callback fires at the original deadline and the test reports a reset that never happened. At 5000ms/4000ms that margin was one second. At 500ms/200ms it is 300ms, which still clears scheduler jitter under `-race` with a single CPU and every core busy, and every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. The assertions no longer compare `time.Now()` against an exact deadline: each callback stamps itself when it runs, and the tests fail only when the callback arrives earlier than a landed reset allows. Scheduler jitter can only delay a callback, so a slow runner cannot turn these into false failures. Every wait is bounded and every callback channel is buffered, so a timer that stops calling back fails the package instead of hanging it, and each test stops its timer on the way out. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short, which is what `make quick-test` uses. The CI race job has run the full suite since XinFinOrg#2912, where the gate was already a no-op. The package costs 38.0s full and 1.0s -short before, 1.2s both ways after.

Five timer tests here waited four to five seconds per assertion, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the only requirement is that a mid-flight Reset lands before the timeout, or the callback fires at the original deadline and the test reports a reset that never happened. The only timing judgment left is on the early side: a callback that saw the mid-flight Reset must arrive after interval plus half of resetAt, which sits 100ms above the 500ms deadline a callback that missed the reset would hit, so scheduler jitter can only push it further away. Every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. The assertions no longer compare `time.Now()` against an exact deadline: each callback stamps itself when it runs, and the tests fail only when the callback arrives earlier than a landed reset allows. Scheduler jitter can only delay a callback, and every wait is bounded at 4x the interval with a buffered callback channel, so a loaded runner stays green while a timer that stops calling back fails inside that bound instead of hanging the package. Each test stops its timer on the way out, unless the test has already failed: then the countdown may be the reason, its startTimer may be wedged, and StopTimer would block instead of letting the failure surface. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short, which is what `make quick-test` uses. The CI race job has run the full suite since XinFinOrg#2912, where the gate was already a no-op. The package costs 38.0s full and 1.0s -short before, 1.2s both ways after.

Five timer tests here waited four to five seconds per assertion, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the only requirement is that a mid-flight Reset lands before the timeout, or the callback fires at the original deadline and the test reports a reset that never happened. The only timing judgment left is on the early side: a callback that saw the mid-flight Reset must arrive after interval plus half of resetAt, which sits 100ms above the 500ms deadline a callback that missed the reset would hit, so scheduler jitter can only push it further away. Every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. The assertions no longer compare `time.Now()` against an exact deadline: each callback stamps itself when it runs, and the tests fail only when the callback arrives earlier than a landed reset allows. Scheduler jitter can only delay a callback, and every wait is bounded at 4x the interval with a buffered callback channel, so a loaded runner stays green while a timer that stops calling back fails inside that bound instead of hanging the package. Each test stops its timer on the way out, unless the test has already failed: then the countdown may be the reason and its startTimer may be wedged. The stop itself is bounded at the same 4x interval, so a wedged timer loop fails the test instead of hanging the package until the timeout. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short, which is what `make quick-test` uses. The CI race job has run the full suite since XinFinOrg#2912, where the gate was already a no-op. The package costs 38.0s full and 1.0s -short before, 1.2s both ways after.

Five timer tests here waited four to five seconds per assertion, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the only requirement is that a mid-flight Reset lands before the timeout; a test goroutine starved past that point skips the stale callback instead of blaming the reset. The only timing judgment left is on the early side: a callback that saw the mid-flight Reset must arrive a full interval after the reset request, which is later than the 500ms deadline a callback that missed the reset would hit, so scheduler jitter can only push it further away. Callbacks stamped before the reset request belong to the previous schedule and are skipped, so a starved test goroutine cannot fail a reset that landed. Every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. The assertions no longer compare `time.Now()` against an exact deadline: each callback stamps itself when it runs, and the tests fail only when the callback arrives earlier than a landed reset allows. Scheduler jitter can only delay a callback, and every wait is bounded at 4x the interval with a buffered callback channel, so a loaded runner stays green while a timer that stops calling back fails inside that bound instead of hanging the package. Each test stops its timer on the way out, unless the test has already failed: then the countdown may be the reason and its startTimer may be wedged. The stop itself is bounded at the same 4x interval, so a wedged timer loop fails the test instead of hanging the package until the timeout. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short, which is what `make quick-test` uses. The CI race job has run the full suite since XinFinOrg#2912, where the gate was already a no-op. The package costs 38.0s full and 1.0s -short before, 1.2s both ways after.

Five timer tests here waited four to five seconds per assertion, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the normal path needs the mid-flight Reset to land before the timeout, and a test goroutine starved past that point skips the stale callback, re-arms from the reset request and does not blame the reset. The only timing judgment left is on the early side: a callback that saw the mid-flight Reset must arrive a full interval after the reset request, which is later than the 500ms deadline a callback that missed the reset would hit, so scheduler jitter can only push it further away. Callbacks stamped before the reset request belong to the previous schedule and are skipped, so a starved test goroutine cannot fail a reset that landed. Every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. The assertions no longer compare `time.Now()` against an exact deadline: each callback stamps itself when it runs, and the tests fail only when the callback arrives earlier than a landed reset allows. Scheduler jitter can only delay a callback, and every wait is bounded at 4x the interval with a buffered callback channel, so a loaded runner stays green while a timer that stops calling back fails inside that bound instead of hanging the package. Each test stops its timer on the way out, unless the test has already failed: then the countdown may be the reason and its startTimer may be wedged. The stop itself is bounded at the same 4x interval, so a wedged timer loop fails the test instead of hanging the package until the timeout. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short, which is what `make quick-test` uses. The CI race job has run the full suite since XinFinOrg#2912, where the gate was already a no-op. The package costs 38.0s full and 1.0s -short before, 1.2s both ways after.

Five timer tests here waited four to five seconds per assertion, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the normal path only needs the mid-flight Reset to land before the timeout and the stop to land before the first callback. A test goroutine starved past either point skips the stale callback in the reset assertions, which then measure from the reset request, and the stop judges callbacks against the instant StopTimer returns, so a pre-stop callback is not read as a post-stop one. The only timing judgment left is on the early side: a callback that saw the mid-flight Reset must arrive a full interval after the reset request, which is later than the 500ms deadline a callback that missed the reset would hit, so scheduler jitter can only push it further away. Callbacks stamped before the reset request belong to the previous schedule and are skipped, so a starved test goroutine cannot fail a reset that landed. Every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. The assertions no longer compare `time.Now()` against an exact deadline: each callback stamps itself when it runs, and the tests fail only when the callback arrives earlier than a landed reset allows. Scheduler jitter can only delay a callback, and every wait is bounded at 4x the interval with a buffered callback channel, so a loaded runner stays green while a timer that stops calling back fails inside that bound instead of hanging the package. Each test stops its timer on the way out, unless the test has already failed: then the countdown may be the reason and its startTimer may be wedged. The stop itself is bounded at the same 4x interval, so a wedged timer loop fails the test instead of hanging the package until the timeout. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short, which is what `make quick-test` uses. The CI race job has run the full suite since XinFinOrg#2912, where the gate was already a no-op. The package costs 38.0s full and 1.0s -short before, 1.2s both ways after.

Five timer tests here waited four to five seconds per assertion, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the normal path only needs the mid-flight Reset to land before the timeout and the stop to land before the first callback. A test goroutine starved past either point is visible in the timestamps: the reset or the stop returns after the deadline the previous schedule was due at, and the affected check is skipped with a log line instead of blaming the timer. On the normal path the reset assertions measure from the reset request, and the stop judges callbacks against the instant StopTimer returns. The only timing judgment left is on the early side: a callback that saw the mid-flight Reset must arrive a full interval after the reset request, which is later than the 500ms deadline a callback that missed the reset would hit, so scheduler jitter can only push it further away. Callbacks stamped before the reset request belong to the previous schedule and are skipped, so a starved test goroutine cannot fail a reset that landed. Every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. The assertions no longer compare `time.Now()` against an exact deadline: each callback stamps itself when it runs, and the tests fail only when the callback arrives earlier than a landed reset allows. Scheduler jitter can only delay a callback, and every wait is bounded at 4x the interval with a buffered callback channel, so a loaded runner stays green while a timer that stops calling back fails inside that bound instead of hanging the package. Each test stops its timer on the way out, unless the test has already failed: then the countdown may be the reason and its startTimer may be wedged. The stop itself is bounded at the same 4x interval, so a wedged timer loop fails the test instead of hanging the package until the timeout. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short, which is what `make quick-test` uses. The CI race job has run the full suite since XinFinOrg#2912, where the gate was already a no-op. The package costs 38.0s full and 1.0s -short before, 1.2s both ways after.

Five timer tests here waited on 5s timers and a 4s stop timer, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the normal path only needs the mid-flight Reset to land before the timeout and the stop to land before the first callback. A test goroutine starved past either point is visible in the timestamps: the reset or the stop lands at or past the lower bound of the previous schedule's deadline, and the affected check is skipped with a log line instead of blaming the timer. On the normal path the reset assertions measure from the reset request, and the stop judges callbacks against the instant StopTimer returns. The two reset floor checks are on the early side: a callback that saw the mid-flight Reset must arrive a full interval after the reset request, which is later than the 500ms deadline a callback that missed the reset would hit, so scheduler jitter can only push it further away. Callbacks stamped before the reset request belong to the previous schedule and are filtered, but a stamp alone cannot tell which schedule produced the callback: a landing past the window's lower bound no longer trips a floor assertion because the gate skips it, not because the filter removed a stale callback. Every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. The assertions no longer compare `time.Now()` against an exact deadline: each callback stamps itself when it runs; the two reset floors fail on an early arrival, the stop test also fails on a post-stop callback, and all waits are bounded. Scheduler jitter can only delay a callback, and the two await sites and the teardown are bounded at 4x the interval with a buffered callback channel, so a loaded runner stays green while a timer that stops calling back fails inside that bound instead of hanging the package. Each test stops its timer on the way out, unless the test has already failed: then the countdown may be the reason and its startTimer may be wedged. The stop itself is bounded at the same 4x interval, so a wedged timer loop fails the test instead of hanging the package until the timeout. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short, which is what `make quick-test` uses. The CI race job has run the full suite since 1.0s -short before, 1.2s both ways after.

Five timer tests here waited on 5s timers and a 4s stop timer, so `make test`, which runs one package at a time, spent 38s on nothing but sleep in this package: the third slowest in the full run. The durations were picked to be human-observable, but nothing in the tests depends on their absolute size: the normal path only needs the mid-flight Reset to land before the timeout and the stop to land before the first callback. A test goroutine starved past either point is visible in the timestamps: the reset or the stop lands at or past the lower bound of the previous schedule's deadline, and the affected check is skipped with a log line instead of blaming the timer. On the normal path the reset assertions measure from the reset request, and the stop judges callbacks against the instant StopTimer returns. The two reset floor checks are on the early side: a callback that saw the mid-flight Reset must arrive a full interval after the reset request, which is later than the 500ms deadline a callback that missed the reset would hit, so scheduler jitter can only push it further away. Callbacks stamped before the reset request belong to the previous schedule and are filtered, but a stamp alone cannot tell which schedule produced the callback: a landing past the window's lower bound no longer trips a floor assertion because the gate skips it, not because the filter removed a stale callback. Every other wait follows from the interval. The timer tests also run in parallel now, so the package costs its longest test rather than their sum, and the two reset tests, which differed only in whether the callback returns an error, became one table test. The assertions no longer compare `time.Now()` against an exact deadline: each callback stamps itself when it runs; the two reset floors fail on an early arrival, the stop test also fails on a post-stop callback, and all waits are bounded. Scheduler jitter can only delay a callback, and the two await sites and the teardown are bounded at 4x the interval with a buffered callback channel, so a loaded runner stays green while a timer that stops calling back fails inside that bound instead of hanging the package. Each test stops its timer on the way out, unless the test has already failed: then the countdown may be the reason and its startTimer may be wedged. The stop itself is bounded at the same 4x interval, so a wedged timer loop fails the test instead of hanging the package until the timeout. All four cost under 4s now, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short, which is what `make quick-test` uses. Since XinFinOrg#2912 the CI race job has run the full suite, so it already covered them; -short for this package went from 1.0s to 1.4s.

Five timer tests here waited on 5s timers and a 4s stop timer, so `make test` spent 38s on nothing but sleep in this package: the third slowest in the full run. Nothing in the tests depends on their absolute size: the normal path only needs the mid-flight Reset to land before the timeout and the stop to land before the first callback. A runner starved past either point is visible in the timestamps, so the affected check is skipped with a log line instead of blaming the timer. The durations drop to 500ms / 200ms / 100ms, the four tests run in parallel, and the two reset tests became one table test. The assertions no longer compare `time.Now()` against an exact deadline: each callback stamps itself when it runs, and every wait is bounded at 4x the interval. The three callback tests stop their timer on the way out through stopCountdown; the stop test stops it mid-test through the same bounded path. The stop test used to call StopTimer directly, so a wedged timer loop held the package until the go test timeout (200.1s measured with -timeout 200s; make test's ci.go default is 45m); stopCountdown now returns the instant StopTimer returned, stamped in its goroutine, and the stop test uses that instant as its baseline. The bound cannot tell a wedged loop from a starved runner, so the failure message names both. The after-stop check runs only while the stop completes before the lower bound of the first deadline, and its window is two and a half intervals: a rearm that fires within two intervals of the stop lands inside it and fails here, while a rearm longer than the window fires after the test has returned and is not observed. A 2x rearm was a coin flip at two intervals (8/20 plain) and invisible under -race (0/10); the extra half interval makes both 10/10. All four cost under 4s, the line XinFinOrg#2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short, which is what `make quick-test` uses. Since XinFinOrg#2912 the CI race job has run the full suite, so it already covered them; -short for this package went from 1.0s to 1.4s. Same mutation after: the stop test fails in 2.1s, the package in 3s. Deleting setInitilised(false) still fails in 0.10s, return -> continue in 0.50s, and moving the callback out of its goroutine in 2.00s.

…3002) Five timer tests here waited on 5s timers and a 4s stop timer, so `make test` spent 38s on nothing but sleep in this package: the third slowest in the full run. Nothing in the tests depends on their absolute size: the normal path only needs the mid-flight Reset to land before the timeout and the stop to land before the first callback. A runner starved past either point is visible in the timestamps, so the affected check is skipped with a log line instead of blaming the timer. The durations drop to 500ms / 200ms / 100ms, the four tests run in parallel, and the two reset tests became one table test. The assertions no longer compare `time.Now()` against an exact deadline: each callback stamps itself when it runs, and every wait is bounded at 4x the interval. The three callback tests stop their timer on the way out through stopCountdown; the stop test stops it mid-test through the same bounded path. The stop test used to call StopTimer directly, so a wedged timer loop held the package until the go test timeout (200.1s measured with -timeout 200s; make test's ci.go default is 45m); stopCountdown now returns the instant StopTimer returned, stamped in its goroutine, and the stop test uses that instant as its baseline. The bound cannot tell a wedged loop from a starved runner, so the failure message names both. The after-stop check runs only while the stop completes before the lower bound of the first deadline, and its window is two and a half intervals: a rearm that fires within two intervals of the stop lands inside it and fails here, while a rearm longer than the window fires after the test has returned and is not observed. A 2x rearm was a coin flip at two intervals (8/20 plain) and invisible under -race (0/10); the extra half interval makes both 10/10. All four cost under 4s, the line #2947 draws for a -short carve-out, so they lose their testshort.Skip calls and start running in -short, which is what `make quick-test` uses. Since #2912 the CI race job has run the full suite, so it already covered them; -short for this package went from 1.0s to 1.4s. Same mutation after: the stop test fails in 2.1s, the package in 3s. Deleting setInitilised(false) still fails in 0.10s, return -> continue in 0.50s, and moving the callback out of its goroutine in 2.00s.

gzliudan mentioned this pull request Oct 29, 2026

test(consensus,core): cut engine v2 test wall time from 219s to 67s #2944

Merged

gzliudan changed the title ~~test(p2p,accounts,rpc,ethclient,metrics,core,eth): gate tests of 4s or more behind -short~~ test(p2p,accounts,rpc,ethclient,metrics,core,eth,consensus): cut the -short carve-out at 4s Oct 29, 2026

gzliudan changed the title ~~test(p2p,accounts,rpc,ethclient,metrics,core,eth,consensus): cut the -short carve-out at 4s~~ test(p2p,accounts,rpc,ethclient,metrics,core,eth,consensus,common,internal): cut the -short carve-out at 4s Oct 29, 2026

gzliudan force-pushed the test-short-mode-4s branch 4 times, most recently from 82f6d13 to b2f384e Compare October 29, 2026 15:58

wanwiset25 approved these changes Oct 29, 2026

View reviewed changes

gzliudan force-pushed the test-short-mode-4s branch from b2f384e to f44c539 Compare October 30, 2026 01:44

gzliudan requested review from AnilChinchawale, anunay-xin, benjamin202410 and liam-lai October 30, 2026 03:30

gzliudan force-pushed the test-short-mode-4s branch 2 times, most recently from dcd2ecf to fda0084 Compare October 30, 2026 10:22

anunay-xin approved these changes Oct 30, 2026

View reviewed changes

gzliudan changed the title ~~test(p2p,accounts,rpc,ethclient,metrics,core,eth,consensus,common,internal): cut the -short carve-out at 4s~~ test: cut the -short carve-out at 4s Nov 1, 2026

gzliudan force-pushed the test-short-mode-4s branch from fda0084 to 6c3be76 Compare November 1, 2026 10:04

benjamin202410 approved these changes Nov 2, 2026

View reviewed changes

gzliudan force-pushed the test-short-mode-4s branch from 6c3be76 to d4ac782 Compare November 2, 2026 21:28

gzliudan added 3 commits November 3, 2026 05:29

gzliudan force-pushed the test-short-mode-4s branch from d4ac782 to 5deb7af Compare November 2, 2026 21:42

gzliudan mentioned this pull request Nov 2, 2026

test(consensus): cut the forensics QC test from 5.17s to 0.18s by polling #2984

Merged

4 of 9 tasks

gzliudan force-pushed the test-short-mode-4s branch 2 times, most recently from ea4cc23 to 5deb7af Compare November 2, 2026 22:16

gzliudan merged commit 1f44551 into XinFinOrg:dev-upgrade Nov 2, 2026
47 checks passed

gzliudan mentioned this pull request Nov 4, 2026

fix(eth): bound the tx-sync wait by idle progress, not a 1s total #20234 #2955

Merged

4 of 25 tasks

Navigation Menu

Repository navigation

test: cut the -short carve-out at 4s - #2947

test: cut the -short carve-out at 4s#2947
gzliudan merged 3 commits into
XinFinOrg:dev-upgradefrom
gzliudan:test-short-mode-4s

gzliudan commented Oct 29, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 29, 2026 •

edited

Loading

Review skipped

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

gzliudan commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Evidence

Merge Danger

Uh oh!

coderabbitai Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gzliudan commented Oct 29, 2026 •

edited

Loading

coderabbitai Bot commented Oct 29, 2026 •

edited

Loading