[DX-4035] go core deployments tests sharding#22349
Merged
Conversation
Contributor
|
✅ No conflicts with other open PRs targeting |
Tofel
force-pushed
the
dx-4035-go-core-deployments-tests-sharding
branch
2 times, most recently
from
7daebae to
4c55e00
Compare
Tofel
force-pushed
the
dx-4035-go-core-deployments-tests-sharding
branch
4 times, most recently
from
dd48687 to
ad2923a
Compare
Tofel
force-pushed
the
dx-4035-go-core-deployments-tests-sharding
branch
from
cdda728 to
98ba6ce
Compare
product-security-plaid-production
Bot
requested review from
kalverra,
pavel-raykov and
skudasov
Contributor
There was a problem hiding this comment.
Pull request overview
Risk Rating: HIGH — This introduces a new CI workflow and reshapes how deployment tests are executed (sharded) and reported for branch protection. Small workflow-condition mistakes can cause false failures (blocking merges) or false passes (missing test failures).
Changes:
- Added a new Go tool (
tools/ci-testshard) to deterministically shard a newline-delimited list of Go test packages (plus unit tests). - Updated
tools/bin/go_deployment_teststo discover test packages and run only the packages assigned to a shard. - Split deployment tests out of
ci-core.ymlinto a new workflowci-deployments.ymlthat runs shards in parallel and aggregates results.
Scrupulous human review recommended:
.github/workflows/ci-deployments.yml: gating/needs/iflogic aroundfilter.should-run, shard matrix generation, and the “result” job behavior (branch protection check correctness).tools/bin/go_deployment_tests: correctness of package discovery (go listtemplate), sharded package invocation, and interaction with thedeployment/module +replacedirective.- Artifact paths (coverage/junit/output) relative to
cd deployment.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tools/ci-testshard/main.go |
Implements list/verify subcommands to shard package lists deterministically. |
tools/ci-testshard/main_test.go |
Adds unit tests for package parsing and sharding behavior. |
tools/bin/go_deployment_tests |
Discovers deployment test packages and runs only the shard’s subset. |
.github/workflows/ci-deployments.yml |
New sharded Deployment Tests workflow + result aggregation job for required checks. |
.github/workflows/ci-core.yml |
Removes deployment test wiring from core workflow; formatting/parameterization updates. |
|
jmank88
approved these changes
May 8, 2026
mchain0
approved these changes
May 8, 2026
erikburt
reviewed
May 8, 2026
Comment on lines
+134
to
+146
| - name: Changed modules | ||
| id: changed-modules | ||
| uses: smartcontractkit/.github/actions/changed-modules-go@changed-modules-go/v1 | ||
| with: | ||
| # when scheduled/workflow_dispatch, run against all modules | ||
| no-change-behaviour: all | ||
| file-patterns: | | ||
| **/*.go | ||
| **/go.mod | ||
| **/go.sum | ||
| module-patterns: | | ||
| ** | ||
| !core/scripts/cre/environment/examples/workflows/** |
Collaborator
There was a problem hiding this comment.
This was only used for figuring out which modules to lint.
Comment on lines
+47
to
+132
| - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 | ||
| id: match-some | ||
| with: | ||
| # "if any changed file matches one or more of the conditions" (https://github.com/dorny/paths-filter/issues/225) | ||
| predicate-quantifier: some | ||
| # deployment - any changes in the deployment module | ||
| # workflow - any changes that could affect this workflow definition | ||
| # - Assume any repository action changes affect this workflow | ||
| filters: | | ||
| deployment: | ||
| - 'deployment/**' | ||
| workflow: | ||
| - '.github/workflows/ci-deployments.yml' | ||
| - '.github/actions/**' | ||
| - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 | ||
| id: match-every | ||
| with: | ||
| # "if any changed file match all of the conditions" (https://github.com/dorny/paths-filter/issues/225) | ||
| # - Enables listing of files matching each filter. | ||
| # - Paths to files will be available in `${FILTER_NAME}_files` output variable. | ||
| # - Paths will be formatted as JSON array | ||
| predicate-quantifier: every | ||
| # core-non-ignored - all changes except for paths which do not affect core module | ||
| # - This is opt-in on purpose. To be safe, new files are assumed to have an affect on core unless listed here specifically. | ||
| # - For example: core module does not depend on deployment or integration-tests module. | ||
| # all - changes in any directory | ||
| # - This is used resolve all affected modules based on changed files | ||
| list-files: json | ||
| filters: | | ||
| core-non-ignored: | ||
| - '**' | ||
| - '!deployment/**' | ||
| - '!integration-tests/**' | ||
| - '!tools/secrets/**' | ||
| - '!tools/docker/**' | ||
| - '!tools/benchmark/**' | ||
| - '!**/README.md' | ||
| - '!**/CHANGELOG.md' | ||
| - '!*.nix' | ||
| - '!sonar-project.properties' | ||
| - '!nix.conf' | ||
| - '!nix-darwin-shell-hook.sh' | ||
| - '!LICENSE' | ||
| - '!.github/**' | ||
| - '!core/scripts/cre/environment/examples/workflows/**' | ||
| all: | ||
| - '**' | ||
|
|
||
| - name: Decide which tests to run (rollout-only) | ||
| # To validate that this properly tests, we will run this beside the dorny/paths-filter actions | ||
| # before using its output to actually gate any jobs. | ||
| id: triggers | ||
| uses: smartcontractkit/.github/actions/advanced-triggers@advanced-triggers/v1 | ||
| continue-on-error: true | ||
| with: | ||
| file-sets: | | ||
| go-files: | ||
| - "**/*.go" | ||
| - "**/go.mod" | ||
| - "**/go.sum" | ||
| core-files: | ||
| - "core/**" | ||
| core-test-files: | ||
| - "testdata/**" | ||
| - "core/**/testdata/**" | ||
| - "core/**/*_test.go" | ||
| e2e-tests-files: | ||
| - "system-tests/**" | ||
| - "integration-tests/**" | ||
| workflow-files: | ||
| - ".github/workflows/ci-deployments.yml" | ||
| - ".github/actions/**" | ||
| deployment-files: | ||
| - "deployment/**" | ||
| ignored-files: | ||
| - "core/scripts/cre/environment/examples/workflows/**" | ||
| # Note: | ||
| # - for pull_request, merge_group, and push events, a trigger will resolve to true if any changed files match the path/glob patterns | ||
| # - exclusion-sets/negations are applied first, and therefore filter all changed files before inclusion sets are applied | ||
| # - by default these will resolve to true for schedule, and workflow_dispatch events | ||
| triggers: | | ||
| deployment-tests: | ||
| exclusion-sets: [ e2e-tests-files, core-test-files, ignored-files ] | ||
| inclusion-sets: [ go-files, core-files, deployment-files, workflow-files ] | ||
| paths: | ||
| - "tools/bin/go_deployment_tests" |
Collaborator
There was a problem hiding this comment.
All of this should be simplified for deployment tests specifically.
Comment on lines
+148
to
+184
| shard-matrices: | ||
| name: Build Shard Matrices | ||
| needs: [filter] | ||
| runs-on: ubuntu-latest | ||
| if: ${{ needs.filter.outputs.should-run == 'true' }} | ||
| permissions: {} | ||
| outputs: | ||
| deployment-test-matrix: ${{ steps.shard-matrices.outputs.deployment-test-matrix }} | ||
| steps: | ||
| - name: Build shard matrices | ||
| id: shard-matrices | ||
| shell: bash | ||
| run: | | ||
| python - <<'PY' >> "$GITHUB_OUTPUT" | ||
| import json | ||
| import os | ||
|
|
||
| run_id = os.environ["GITHUB_RUN_ID"] | ||
|
|
||
| rows = [] | ||
|
|
||
| def add_deployment_rows(): | ||
| count = int(os.environ["DEPLOYMENT_TEST_SHARD_COUNT"]) | ||
| for i in range(count): | ||
| rows.append({ | ||
| "job-name": f"go_deployment_tests shard {i}/{count}", | ||
| "os": f"runs-on={run_id}-deployment-{i}/cpu=8/ram=32/family=m6id+m6idn/spot=false/image=ubuntu24-full-x64/extras=s3-cache", | ||
| "shard-count": str(count), | ||
| "shard-index": str(i), | ||
| "run-timeout-minutes": "25", | ||
| "logs-artifact-name": f"go_deployment_tests_logs_shard_{i}", | ||
| }) | ||
|
|
||
| add_deployment_rows() | ||
|
|
||
| print(f"deployment-test-matrix={json.dumps(rows)}") | ||
| PY |
Collaborator
There was a problem hiding this comment.
This job is unnecessary. We can use a simple matrix strategy on the job itself, and reference the index and total through the strategy context.
https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#strategy-context
${{ strategy.job-index }} / ${{ strategy.job-total }}
All other properties in the JSON object are more or less static.
| uses: ./.github/actions/setup-go | ||
| with: | ||
| restore-build-cache-only: false | ||
| build-cache-version: go_deployment_tests |
Collaborator
There was a problem hiding this comment.
This will cause only 1 shard to own the mutex for the build cache, and effectively remove the build/test cache for all other shards.
| include: ${{ fromJson(needs.shard-matrices.outputs.deployment-test-matrix) }} | ||
| name: Deployment Tests (${{ matrix.job-name }}) | ||
| if: ${{ github.actor != 'dependabot[bot]' }} | ||
| needs: [filter, shard-matrices, run-frequency] |
Collaborator
There was a problem hiding this comment.
Doesn't need run-frequency.
Comment on lines
+368
to
+390
| run-frequency: | ||
| name: Run frequency | ||
| outputs: | ||
| one-per-day-frequency: ${{ steps.check-time.outputs.one-per-day-frequency || 'false' }} | ||
| runs-on: ubuntu-latest | ||
| permissions: {} | ||
| steps: | ||
| - name: Check time and set frequencies | ||
| id: check-time | ||
| shell: bash | ||
| run: | | ||
| if [ "$GITHUB_EVENT_NAME" != "schedule" ]; then | ||
| # Not a scheduled event - no frequencies to set. They default to false. | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Scheduled event, check current time for frequencies | ||
| current_hour=$(date +"%H") | ||
|
|
||
| # Check if the current hour is 00 (one per day) | ||
| if [ "$current_hour" -eq "00" ]; then | ||
| echo "one-per-day-frequency=true" | tee -a "$GITHUB_OUTPUT" | ||
| fi |
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.
No description provided.