ci: run the whole suite under the race detector, 450s → 123s by gzliudan · Pull Request #2912 · XinFinOrg/XDPoSChain

gzliudan · 2026-10-27T17:06:30Z

Proposed changes

CI ran every test twice: the non-race tests job and the race detector covered the same 130 packages, so the non-race half asserted nothing the detector does not. The detector becomes the only job that tests: the whole suite under -race, split into 13 lanes that run in parallel, with a bound on every job and a FIFO queue per ref so a push is never dropped. The slowest test job goes from 450s to 123s.

Summary

 tests
-  9 slices by package-name range (A-B ... T-Z), full suite, -coverage
-  the same 130 packages the detector also runs
 static
   gofmt, tidy, generate and the shard checks, once per run
 race
   the whole suite under the detector, now the only test job
-  5 lanes, -short: bulk 1/4..4/4 by position, then rpc
+  13 lanes, full: bulk 4, engine_v2 5, core 2, cmd/XDC 1, rpc
   bulk split by import path, heavy packages by test name
+  runs on dev-upgrade and tags even when no Go changed
+  runs of one ref queue behind each other; none is cancelled
 deploy
+  tag_build and devnet_build_push now wait for the detector

Bulk slices are a function of the import path (cksum(path) % 4 in build/shard.sh), so a new package moves one package instead of every boundary after it. engine_v2_tests (110 tests) and core (459) are sliced by test name, enumerated per lane with go test -list '^(Test|Example|Fuzz)', so a new test, example or fuzz target lands in exactly one lane with no table to maintain; a fuzz target runs its seed corpus in normal mode. cmd/XDC (44 tests) and rpc are single lanes; rpc cannot share one because its 32 MiB websocket test trips the codec's 30s pong timeout under -p 4.

static reads the bulk lane count back from the workflow, checks the partition (build/shard.sh --check), and verifies every heavy package declares lanes 1..n and that nothing outside the heavy list does (--check-groups).

Watchdogs: each lane passes -timeout 10m (4m for rpc) under a 20m (9m) step bound, so a hung test panics and prints its goroutine dump instead of being killed with the runner; static, lint, race-scope and the Docker and kubectl steps each carry a job-level bound.

Scope: race-scope skips the detector only for a Markdown-only diff that touches nothing under testdata/; a failed fetch, an empty diff or a new branch runs it; renames are compared with --no-renames, so a moved source path still counts. dev-upgrade and tags always run it, because the deploy jobs wait for it.

Queue: concurrency: {group: ci-CI-<ref>, queue: max} keeps one run per ref and queues the rest FIFO, up to 100 deep. A newer push never cancels an older run, and every pull request has its own ref, so different pull requests never wait for each other.

build/ci.go test forwards -run and -timeout, which the name slices need; the default stays 45m for a local full pass.

Evidence

Before: run 41998780636, dev-upgrade at 93899befe6: 20 jobs, slowest race (bulk 2/4) 450s, the nine tests slices 127-296s, then the deploy chain 09:05:03 → 09:07:28.
After: run 42028424944 on 8d0a349d0e: 21 jobs, slowest test lane race (cmd XDC) 123s (core 2/2 106s, bulk lanes 36-56s, rpc 66s), whole run 158s with lint 156s and static 85s alongside.
Coverage accounting: make check-shards on this tree → 26 + 33 + 34 + 33 packages, 4 shards cover 126 packages, ok: 3 group packages, every lane 1..n present. The 13 lanes run all 130 packages that have test files; core/vm/privacy stays out because its tests carry //go:build privacytests and no lane has ever run them (go test -tags=privacytests ./core/vm/privacy by hand).
core enumerates 459 cases, not 456: the inventory now includes ExampleGenerateChain and the two chainconfig fuzz targets, whose seed corpora ran under the old tests job and would otherwise have stopped running.
Local on this head: make race-test-full-group GROUP_PKG=./core GROUP_I=1 GROUP_N=2 27s; go run build/ci.go test forwarding; make generate, make tidy, make lint (0 issues), go vet ./..., make quick-test, make test all rc=0.
Markdown-only skip: race-scope reads the whole changed list before deciding, so a diff past the 64 KiB pipe buffer cannot flip the decision to "skip". The earlier grep -q form did exactly that (measured: 2.19 MB of changed paths → run=false).
build/shard.sh --check fails on a package listed twice; --check-groups fails when a heavy package loses its lanes or a package outside RACE_HEAVY claims one.

Merge Danger

Door: two-way. CI configuration only; reverting restores the previous topology, with no state to migrate and no binary to roll back.

Blast Radius: CI only, but five things to know before merging.

Every tests (...) check name is gone, and the race lanes changed (race (bulk 1/4..4/4), race (cmd XDC), plus the new race-scope and static). Scripts that grep the old names need updating.
This pull request reports mergeStateStatus=BLOCKED with every check green and three approvals. Repository-level required checks are empty, the one repository ruleset has no rules, CODEOWNERS does not cover .github/, Makefile or build/, and the branch is up to date. Likely an organisation-level ruleset the public API cannot show; confirm with an administrator before merging.
The detector is the only job that runs tests. It is skipped only for a Markdown-only diff, and not when any changed path is under testdata/; on dev-upgrade and tags it always runs because the deploy jobs wait for it.
Runs of one ref wait in a FIFO queue (queue: max, up to 100 deep): a run starts only after the previous run of the same ref has finished, and none is cancelled or replaced. A pull request has its own ref, so different pull requests never queue behind each other; a full queue cancels the run that arrives 101st.
Caches are keyed by commit and job index: about 156 MB per lane of the 10 GB budget, with LRU eviction behind it.

Types of changes

What type of changes does your code introduce to XDC network?
Put an ✅ in the boxes that apply

Impacted Components

Which parts of the codebase does this PR touch?
Put an ✅ in the boxes that apply

Checklist

Put an ✅ in the boxes once you have confirmed below actions (or provide reasons on not doing so) that

This PR has sufficient test coverage (unit/integration test) OR I have provided reason in the PR description for not having test coverage
Tested on a private network from the genesis block and monitored the chain operating correctly for multiple epochs. - not applicable, CI-only change.
Provide an end-to-end test plan in the PR description on how to manually test it on the devnet/testnet. - not applicable, CI-only change; the plan is the run links under Evidence.
Tested the backwards compatibility. - not applicable, no runtime behaviour changes.
Tested with XDC nodes running this version co-exist with those running the previous version. - not applicable, no node code changes.
Relevant documentation has been updated as part of this PR
N/A

coderabbitai · 2026-10-27T17:06:37Z

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: bcba143e-ba3d-48d5-8674-bcd135945255

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

The case walked 11,225,088 epochs of big.Float arithmetic to reach the point where the geometric schedule pays nothing: 24.7 s on an idle machine and about 13 min under the race detector, past the 10m timeout the race lanes pass - the engine_v2 2/3 lane died on it (XinFinOrg#2912). The series converges to the halving supply for any parameter pair - the reward/supply ratio only sets how fast - so the sweep now runs on a scaled-down supply whose whole curve is 3,450 epochs, and asserts the same two properties: the per-epoch reward dies out and what is issued stays below the supply. Mainnet's numbers are checked at the two epochs the walk pinned down (11,225,087 pays 1 wei, 11,225,088 pays nothing), which costs two calls. The -short gate goes away with the cost: the case runs in 10 ms.

Copilot

🟡 Changes recommended

Unresolved CI defects can skip the only test job for code-changing PRs, omit existing example and fuzz seed checks, and cancel pending push CI/deployment runs.

3 open issues

What changed in this PR

This PR consolidates CI testing into a full race-detector suite and rebalances its workload across 13 lanes. It changes CI tooling and documentation without changing node behavior.

Changes:

Replaces duplicate test jobs with full race testing, split by package or test name.
Adds shard validation, configurable test filters and timeouts, and a separate static-check job.
Updates test skip rules, run cancellation, caches, and deployment dependencies.

File	Description
`Makefile`	Adds full-suite shard targets and partition checks.
`build/shard.sh`	Assigns packages deterministically and validates shard coverage.
`build/ci.go`	Supports test-name filters and configurable timeouts.
`AGENTS.md`	Documents full race testing and matching local commands.
`.github/workflows/ci.yml`	Reorganizes test lanes, static checks, timeouts, and deployment gates.

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

Rename detection can incorrectly skip the sole CI test job after source or fixture moves, and the Makefile filter silently omits valid entry points named exactly Example, Fuzz, or Test.

2 open issues

3 resolved since last review

_{Review effort: Balanced}

… job The non-race `tests` job and the detector ran the same tests: of the 130 packages that have test files, both covered all 130. `core/vm/privacy` had no lane in either - its tests are behind -tags=privacytests - and it is unused and scheduled for deletion (XinFinOrg#2895), so the test job asserted nothing the detector does not. The detector becomes the test job: the whole suite, under -race, in thirteen lanes. The bulk of the tree is split by import path (cksum) into four lanes, so a new package lands in one lane instead of shifting every boundary after it, and `make check-shards` asserts that the lanes partition the package list. The assignment and both checks live in `go run build/ci.go shard`, so the split needs no shell script. The static job runs the checks, and reads the lane count back from the workflow: a lane dropped without the total moving fails there instead of silently leaving its packages untested. engine_v2_tests (624s) runs as five lanes sliced by test name and core (308s) as two, both enumerated from the package itself so no table needs maintaining. cmd/XDC is one more: at 79s it is a single package, and no split of the tree can move it. rpc stands alone because its 32 MiB websocket test trips the codec's 30s pong timeout under -p 4. core/vm/privacy keeps no lane at all. gofmt, `go mod tidy -diff` and `go generate` move to a `static` job that runs once per run instead of once per shard, with a commit-keyed go-build cache: a key holding only go.sum froze an entry at the commit that first wrote it, and every lane then paid 50-80s to recompile. The deploy jobs followed `tests`; they now wait for the detector, which runs on dev-upgrade and tags even when the diff holds no Go code - otherwise a documentation merge would skip the detector and, through `needs`, the deployment as well. build/ci.go test forwards -run, which the name slices need.

Copilot

🔵 Needs a closer look

Removing and renaming CI checks amid unexplained merge blocking requires an administrator to confirm compatibility with organization-level required-check rules.

No open issues 🎉

2 resolved since last review

_{Review effort: Balanced}

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.

gzliudan force-pushed the ci-speed-tests branch 2 times, most recently from 55288d8 to e77e2fd Compare October 27, 2026 17:11

gzliudan requested review from AnilChinchawale, anunay-xin, benjamin202410, liam-lai and wanwiset25 and removed request for benjamin202410 October 27, 2026 17:13

gzliudan force-pushed the ci-speed-tests branch 2 times, most recently from 8f1fd22 to ead0920 Compare October 28, 2026 08:21

gzliudan changed the title ~~ci: balance the test shards and stop paying for what nothing reads~~ ci: balance the test shards, run the race job in full, stop paying for what nothing reads Oct 29, 2026

gzliudan mentioned this pull request Oct 29, 2026

ci: split engine_v2_tests out of the c[n-o] test job #2902

Merged

1 of 2 tasks

gzliudan force-pushed the ci-speed-tests branch from c69350f to b9a4343 Compare October 29, 2026 16:39

wanwiset25 approved these changes Oct 29, 2026

View reviewed changes

gzliudan force-pushed the ci-speed-tests branch 3 times, most recently from ad20412 to 68e4fc7 Compare October 30, 2026 02:10

gzliudan changed the title ~~ci: balance the test shards, run the race job in full, stop paying for what nothing reads~~ ci: rebalance the test shards, run the race job in full, and stop repeating work per shard Oct 30, 2026

gzliudan changed the title ~~ci: rebalance the test shards, run the race job in full, and stop repeating work per shard~~ ci: rebalance the test shards and run the race job in full Oct 30, 2026

gzliudan mentioned this pull request Oct 30, 2026

test(consensus): cut the reward-halving vanishing case to milliseconds #2949

Merged

4 tasks done

gzliudan force-pushed the ci-speed-tests branch from e2709a8 to a5367eb Compare October 30, 2026 07:38

anunay-xin approved these changes Oct 30, 2026

View reviewed changes

gzliudan force-pushed the ci-speed-tests branch 2 times, most recently from 0faaea0 to 784c4f4 Compare October 30, 2026 10:06

gzliudan force-pushed the ci-speed-tests branch 6 times, most recently from a71816c to e538686 Compare November 5, 2026 11:32

gzliudan requested a balanced review from Copilot November 5, 2026 11:48

Copilot started reviewing on behalf of gzliudan November 5, 2026 11:49

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

Comment thread .github/workflows/ci.yml Resolved

Comment thread .github/workflows/ci.yml Resolved

Comment thread Makefile Resolved

gzliudan force-pushed the ci-speed-tests branch 2 times, most recently from 6e2341e to 8d0a349 Compare November 5, 2026 12:35

gzliudan requested a balanced review from Copilot November 5, 2026 13:15

Copilot started reviewing on behalf of gzliudan November 5, 2026 13:16

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

Comment thread .github/workflows/ci.yml Resolved

Comment thread Makefile Resolved

gzliudan changed the title ~~ci: rebalance the test shards and run the race job in full~~ ci: run the whole suite under the race detector, 450s → 123s Nov 5, 2026

gzliudan added 2 commits November 5, 2026 22:34

ci: bound every job and queue runs

8195949

gzliudan force-pushed the ci-speed-tests branch from 8d0a349 to 8195949 Compare November 5, 2026 14:36

gzliudan requested a balanced review from Copilot November 5, 2026 14:40

Copilot started reviewing on behalf of gzliudan November 5, 2026 14:41

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

gzliudan merged commit 285a3f1 into XinFinOrg:dev-upgrade Nov 5, 2026
26 checks passed

gzliudan deleted the ci-speed-tests branch November 5, 2026 15:07

gzliudan mentioned this pull request Nov 5, 2026

ci: run tests only on pull requests and gate devnet deploys on PR checks #3029

Open

gzliudan mentioned this pull request Nov 6, 2026

test(common/countdown): cut the countdown test time from 38s to 1.4s #3002

Merged

4 of 9 tasks

Navigation Menu

Repository navigation

ci: run the whole suite under the race detector, 450s → 123s - #2912

ci: run the whole suite under the race detector, 450s → 123s#2912
gzliudan merged 2 commits into
XinFinOrg:dev-upgradefrom
gzliudan:ci-speed-tests

gzliudan commented Oct 27, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 27, 2026 •

edited

Loading

Review skipped

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

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

Uh oh!

Proposed changes

Summary

Evidence

Merge Danger

Types of changes

Impacted Components

Checklist

Uh oh!

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

Uh oh!

Review skipped

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gzliudan commented Oct 27, 2026 •

edited

Loading

coderabbitai Bot commented Oct 27, 2026 •

edited

Loading