P2 02 matrix sharding by state group - #482
Open
javicoin wants to merge 11 commits into
Open
Conversation
Fan out the full suite into 4 state-group shards (from the P2-01 state map) via a dynamic matrix: prepare (build image + compute matrix) -> per-shard test -> aggregate gate. Each shard boots its own node env and runs the 00_sync bootstrap plus its group through the INCLUDE_CASES seam. Add an include-cases action input threaded to INCLUDE_CASES in the entrypoint; empty by default so the single build+test path is unchanged for the rskj/powpeg-node callers that consume this action via @main. The aggregate job gates on build + all shard results and renders one consolidated timing summary across shards.
Add a MODE switch to the container entrypoint: - all (default): clone+build rskj/powpeg AND run tests in one container (unchanged, so the rskj/powpeg-node @main callers are unaffected) - build: only build the fat jar, stage it to the workspace, emit its version - test: skip the build, place a prebuilt jar, run the tests CI now builds the jar once in the prepare job, uploads it as an artifact, and each shard downloads it and runs in test mode. This removes the per-shard rebuild (~5 min x N) and the N-fold exposure to transient Gradle distribution download failures that broke shards in the first sharded run. Expose the mode + powpeg-version inputs on the action for external callers.
Add scripts/ci-merge-junit.js to merge the per-shard mocha-junit-reporter XML files into a single <testsuites> document. The aggregate job now merges the downloaded shard reports, uploads the result as the rit-junit-report artifact, and renders one cumulative timing summary over it. Each shard reruns the 00_sync bootstrap, so that suite appears once per shard in the merged report (counts reflect what actually ran).
federation -> federation-change, bridge-queries -> bridge-methods, param-mutators -> bridge-state-changes; 2wp kept. Names only (matrix.name); shard membership unchanged.
Retry the Gradle build with backoff: the wrapper's distribution download has returned transient 504s that sink an otherwise healthy build. Under build-once a single build feeds every shard, so one flake failed the whole run; this also protects the single-container path used by rskj/powpeg-node. Set overwrite on the artifact uploads so 're-run failed jobs' replaces the previous attempt's artifacts instead of failing on a name clash. Add scripts/ci-check-shard-coverage.js, run in prepare, which fails the build when a runnable test file is not assigned to any shard (or when a shard pattern matches nothing), so new tests cannot be silently dropped. Pass the rskj/powpeg branch names to the shard containers so their logs show which branches the prebuilt jar came from.
The coverage check ran against the active matrix, so a short-suite PR (the default for PRs) hit the empty-cases no-op and skipped the check entirely. A new test file outside tests/**/extra/ does not auto-escalate the PR to the full suite, so it would pass PR CI and only fail later on main. Always emit the full shard definitions and validate those, independent of which suite the run executes.
Explain that CI fans the full suite out into state-group shards, what each shard contains and why, and how to pick the right one when adding a test. Notes that the coverage check fails the build when a file is unassigned, and that shard patterns are prefix matches.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the CI pipeline to run Rootstock Integration Tests in parallel shards while building the environment once, then aggregates shard results into a single gate/check and notification path to reduce redundant work and improve run reliability.
Changes:
- Added a
preparejob that builds the container image, computes a shard matrix, builds the powpeg fat jar once, and uploads it as an artifact for shard reuse. - Updated the test job to run as a matrix of shards (with per-shard JUnit upload, timing summary, and PR annotations).
- Added an
aggregatejob to download all shard reports, merge them into a cumulative JUnit report, and gate overall status + Slack notifications.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/ci-merge-junit.js |
New helper to merge per-shard JUnit XML reports into a cumulative report. |
scripts/ci-check-shard-coverage.js |
New guard to ensure shard patterns cover all runnable test files and detect stale patterns. |
README.md |
Documents shard intent and the process for adding new tests to shards. |
container-action/entrypoint.sh |
Adds sharding support via INCLUDE_CASES and introduces build/test split modes with Gradle retry. |
action.yml |
Adds new action inputs (include-cases, mode, powpeg-version) to support sharding and build/test split. |
.github/workflows/ci.yml |
Major CI restructure: prepare → shard matrix tests → aggregate gate + notifications; publish depends on aggregate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Validate argv-derived paths in ci-merge-junit.js before touching the file system, using the same inline base-dir check already used in ci-timing-summary.js, and report a refused path as a clean error instead of an uncaught stack (3 path-injection findings). Reduce the cognitive complexity of the shard-coverage check by extracting matrix parsing and the expected-to-run predicate out of main(). Use [[ ]] instead of [ ] for the conditionals added by the sharding work, matching the rest of the entrypoint.
The short suite has nothing to fan out (extra/ is skipped, 3 files run), so splitting it into build-then-test only added a second runner, an artifact round-trip and an aggregate hop: measured 12.8 min against a ~11 min single-job baseline. Carry a mode per matrix entry instead. The short suite runs one 'all'-mode container (the original clone+build+test path), and the jar build is now its own job that only runs for the full suite. This also keeps the 'all' path — the one rskj/powpeg-node use via @main — exercised on every PR, which the sharded flow no longer did. The aggregate gate treats a skipped jar build as valid for short runs.
Fail closed in the shard-coverage check: a matrix whose include is not an array, or a shard without a string cases field, is now an error. Previously a malformed matrix hit the falsy 'runs everything' sentinel and silently disabled the guard. Resolve real paths in ci-merge-junit.js before reading or writing, so a symlinked reports directory cannot escape the workspace, matching the guard already used in ci-timing-summary.js. Correct the header comment, which claimed the script never fails: an unwritable or refused output path does exit non-zero. Reject an unknown INPUT_MODE in the entrypoint instead of silently falling through to the default build-and-test path.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
scripts/ci-merge-junit.js:29
- The comment says this checks the canonical path stays inside the workspace, but
safeReadDir/safeReadFileusepath.resolve(which does not resolve symlinks). Either change the implementation to use realpaths per access, or update the comment to match the actual behavior.
const inputDir = process.argv[2] || 'shard-reports';
const outFile = process.argv[3] || 'reports/junit.xml';
// Paths come from argv, so guard them before any file-system access: resolve against the working
// directory (which is GITHUB_WORKSPACE in CI, where the reports live) and confirm the canonical
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:581
- The publish job’s cache comment is inaccurate: the Docker layers are built/cached by the
build-jarjob for full-suite runs (and by the short-suitetest-container-actionjob), not byprepare(which only computes vars/matrix). This can mislead future maintenance of the cache flow.
# Replays the layers already built (and fully cached with mode=max) by the prepare job.
Passing an empty cache-to to docker/build-push-action on the shard path was an untested expression; the shards have no useful layers to export anyway since build-jar (and the publish job on main) already write the cache with mode=max, and four shards exporting concurrently would just contend. The publish job's comment credited the prepare job for the cached layers; that build moved to build-jar.
|
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.
This pull request significantly refactors the CI workflow in
.github/workflows/ci.ymlto introduce sharded test execution, improve artifact reuse, and streamline reporting and notifications. The main improvements are the addition of a build-and-prepare stage that shards the test suite, runs each shard in parallel, and aggregates results for a single status and notification. This change aims to reduce redundant work and make CI runs faster and more reliable.Key changes include:
Sharded Test Execution and Preparation
preparejob that builds the Docker image and test jar once, computes the test shard matrix, and uploads the jar as an artifact for all shards to reuse. This avoids rebuilding the environment for each shard, significantly speeding up CI runs.preparejob.Test Execution and Reporting
test-container-actionjob now consumes the prebuilt jar and runs only its assigned test cases, uploading individual JUnit reports and timing summaries. Test failures are surfaced as PR annotations, and each shard reports its results independently.aggregatejob that collects all shard reports, merges them into a single cumulative JUnit report, and gates the overall CI status on all shards passing. This job also generates a consolidated timing summary.Notification and Publishing Improvements
These changes make the CI pipeline more modular, efficient, and reliable, especially as the test suite grows.