ci(rsk-test): cover consumer package tests - #50
Open
fedejinich wants to merge 4 commits into
Open
Conversation
|
PR Triager kept |
github-actions
Bot
added
triaged
fedejinich
force-pushed
the
rsk/ci/consumer-package-tests
branch
from
September 3, 2026 12:41
6a19f63 to
f348e54
Compare
github-actions
Bot
added
triaged
Replace the six-line justification with two lines: the test needs gitignored cannon artifacts and its package imports no RSK-edited code, so skipping it loses no signal. Drop the claim that PAYROLLUP-132 forbids build/vet/lint work; the ticket only excludes them as gates (PAYROLLUP-206).
github-actions
Bot
added
triaged
fedejinich
marked this pull request as ready for review
September 4, 2026 00:14
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
github-actions
Bot
added
triaged
There was a problem hiding this comment.
🟢 Approval recommended
The workflow change is focused, validated, and has no unresolved issues.
Pull request overview
Expands RSK CI coverage for shipped services consuming modified op-service code.
Changes:
- Adds full
op-batcher,op-proposer, andop-challengertest trees. - Skips only the Cannon test requiring generated artifacts.
- Documents the targeted exclusion.
File summaries
| File | Description |
|---|---|
.github/workflows/rsk-test.yml |
Expands scoped Go tests and documents the targeted exclusion. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…yer packages oprsk-node ships all of op-node and oprsk-deployer ships op-deployer, and both import RSK-patched packages, but go-tests only exercised fragments of those trees: two op-node subtrees and a single op-deployer package. A regression anywhere else in what we actually ship went untested. Replace ./op-node/rollup/derive/... and ./op-node/node/... with ./op- node/..., and add the eight op-deployer packages that run without Foundry: pkg/deployer itself (that package only, not its subtree), artifacts, clean, standard, state, upgrade/embedded, upgrade/v2_0_0 and validate. bootstrap, manage, opcm, pipeline, verify and integration_test still need forge-artifacts or the synced superchain registry, so they are left for the Foundry tier (rsk-contracts-test.yml) to pick up later; a comment above the job records that split. The go-checks header no longer names a count of scoped trees. Local run of the exact job command with -count=1: all packages PASS, no failures. op-node 29/29 ok, the nine op-deployer packages ok, 88 packages ok overall in about 50 s. Refs: PAYROLLUP-132
Author
ci(rsk-test): cover the rest of op-node and the Foundry-free op-deployer packages
github-actions
Bot
added
triaged
There was a problem hiding this comment.
🟡 Changes recommended
The description, test evidence, and Foundry-tier wording must accurately reflect the final workflow coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Balanced
Comment on lines
+125
to
+127
| # op-deployer bootstrap, manage, opcm, pipeline, verify and integration_test | ||
| # need forge-artifacts or the synced superchain registry, so they stay with | ||
| # the Foundry tier (PAYROLLUP-132). |
| ./op-node/rollup/derive/... \ | ||
| ./op-node/node/... \ | ||
| go test -timeout 15m -skip '^TestLastStepCacheAccuracy$' \ | ||
| ./op-node/... \ |
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.
Why
The fork ships
op-batcher,op-proposer, andop-challenger, butrsk-test.ymldid not run their full test trees. These packages consume RSK-edited code inop-service/txmgrandop-service/sources, so a change there could break a shipped binary without failing the scoped test job.This closes that gap without touching the RSK-specific test suites or adding
op-e2e.What changes
./op-proposer/...replaces the narrower./op-proposer/contracts/...entry, so the workflow does not run overlapping package roots.TestLastStepCacheAccuracyis skipped for two reasons, both recorded in the workflow header:op-challenger/game/fault/trace/cannonimports neitherop-service/txmgrnorop-service/sources, so it is not a consumer of RSK-edited code and skipping it loses no signal (git grepover the package confirms it).cannon/bin/cannonandcannon/testdata/go-1-24/bin/hello.64.elf, produced bycd cannon && make cannon elf. Building them in the runner would add a build step for a test with no RSK signal.The exact
-skip '^TestLastStepCacheAccuracy$'filter keeps every other challenger test in the gate.The header now says only that. Trimming the rest of the header is tracked separately in PAYROLLUP-206.
Test evidence
The requested unfiltered run produced these results:
op-batcherop-proposerop-challengerTestLastStepCacheAccuracycould not find the generated ELFgo test -timeout 15m ./op-batcher/... ./op-proposer/... ./op-challenger/...The command used by the updated workflow passes:
Also checked:
git diff --checkrsk/**rsk/developScope
No Cannon build setup,
op-e2e, CircleCI port, static-analysis change, or rootstack submodule bump is included.Jira: PAYROLLUP-132 · header trim: PAYROLLUP-206