[DO NOT MERGE] Phase A: stest x trond integration test (fork-only)#4
[DO NOT MERGE] Phase A: stest x trond integration test (fork-only)#4warku123 wants to merge 2 commits into
Conversation
…MERGE]
Replace the inline `nohup java -jar build/libs/FullNode.jar --witness
-c <conf> &` block with a declarative trond invocation that
encapsulates: HOCON template embedding, systemd-managed lifecycle,
deterministic teardown, and structured log/journal capture.
Net effect:
- Same single-SR private chain, same testng.conf endpoints
(HTTP 8090 / gRPC 50051 / P2P 18888 / JSON-RPC 8545).
- The 80+ line "Copy config and start FullNode" step shrinks to
one bash invocation: `bash system-test/scripts/stest-up.sh`.
- On failure, the systemd journal for the unit is dumped to
logs/journal.log instead of relying on ad-hoc fullnode.log.
- Teardown step (`if: always()`) runs `trond network destroy`
so the next CI run starts clean.
This commit is Phase A: the system-test checkout points at
warku123/system-test:feat/trond-bridge and stest-up.sh defaults
to warku123/tron-deployment v0.1.0 release. A follow-up Phase B
commit on this branch will flip both references to upstream
(tronprotocol/system-test:release_workflow + the upstream trond
release URL) once tron-deployment has merged the
system-test-integration change and cut v0.1.0 on upstream.
Until then this PR must not be merged into upstream.
Linked: openspec change `system-test-integration` in
tronprotocol/tron-deployment (Phase 4).
📝 WalkthroughWalkthroughThe system-test workflow now checks out warku123/system-test@release_workflow, prefers running system-test/scripts/stest-up.sh with STEST_JAR to bring up the stest single-SR (falls back to java -jar start), always captures node logs into logs/ and uploads them as stest-node-logs, and always runs stest-down.sh for teardown when present. ChangesSystem Test Workflow Restructuring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/system-test.yml:
- Around line 42-43: The workflow currently checks out a mutable branch via
"ref: feat/trond-bridge", which is a TOCTOU risk; change the checkout step to
pin the fork to a specific commit SHA (replace "ref: feat/trond-bridge" with the
full 40‑char commit SHA) and update the Phase A comment above the job to record
that exact SHA so reviewers can verify the exact revision being tested; also
ensure the Phase A comment references the scripts stest-up.sh and stest-down.sh
to indicate why the SHA is pinned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a9208ad9-d383-4ea7-8a16-12f717ae0bc2
📒 Files selected for processing (1)
.github/workflows/system-test.yml
| repository: warku123/system-test | ||
| ref: feat/trond-bridge |
There was a problem hiding this comment.
Pin the fork checkout to a full commit SHA to prevent silent TOCTOU code-execution risk.
ref: feat/trond-bridge is a mutable branch. Any push to warku123/system-test after this PR is opened silently replaces stest-up.sh (line 71) and stest-down.sh (line 102) — both of which run as unrestricted bash on the CI runner — without triggering any review gate. Because stest-up.sh also downloads the trond binary (whose SHA256 pin lives inside the script itself), an attacker or accidental push to the fork branch can change both the verification logic and the executed code in a single unreviewed commit.
🔒 Proposed fix — pin to a specific commit SHA
- repository: warku123/system-test
- ref: feat/trond-bridge
+ repository: warku123/system-test
+ ref: <full-40-char-commit-sha> # e.g. the HEAD of feat/trond-bridge at time of reviewRecord the pinned SHA in the Phase A comment above so reviewers can verify what exact revision is being tested.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/system-test.yml around lines 42 - 43, The workflow
currently checks out a mutable branch via "ref: feat/trond-bridge", which is a
TOCTOU risk; change the checkout step to pin the fork to a specific commit SHA
(replace "ref: feat/trond-bridge" with the full 40‑char commit SHA) and update
the Phase A comment above the job to record that exact SHA so reviewers can
verify the exact revision being tested; also ensure the Phase A comment
references the scripts stest-up.sh and stest-down.sh to indicate why the SHA is
pinned.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/system-test.yml">
<violation number="1" location=".github/workflows/system-test.yml:43">
P2: Pin this checkout to a full commit SHA instead of a mutable branch name. Using `ref: feat/trond-bridge` means any subsequent push to that branch silently replaces the scripts (`stest-up.sh`, `stest-down.sh`) that execute as unrestricted bash on the runner — including the SHA256 verification logic for the `trond` binary download. This is a supply-chain risk even for a temporary fork-only workflow.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| repository: tronprotocol/system-test | ||
| ref: release_workflow | ||
| repository: warku123/system-test | ||
| ref: feat/trond-bridge |
There was a problem hiding this comment.
P2: Pin this checkout to a full commit SHA instead of a mutable branch name. Using ref: feat/trond-bridge means any subsequent push to that branch silently replaces the scripts (stest-up.sh, stest-down.sh) that execute as unrestricted bash on the runner — including the SHA256 verification logic for the trond binary download. This is a supply-chain risk even for a temporary fork-only workflow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/system-test.yml, line 43:
<comment>Pin this checkout to a full commit SHA instead of a mutable branch name. Using `ref: feat/trond-bridge` means any subsequent push to that branch silently replaces the scripts (`stest-up.sh`, `stest-down.sh`) that execute as unrestricted bash on the runner — including the SHA256 verification logic for the `trond` binary download. This is a supply-chain risk even for a temporary fork-only workflow.</comment>
<file context>
@@ -27,11 +27,20 @@ jobs:
- repository: tronprotocol/system-test
- ref: release_workflow
+ repository: warku123/system-test
+ ref: feat/trond-bridge
path: system-test
</file context>
Wrap the stest-up.sh invocation in an existence check, and fall back to the legacy inline `cp config + nohup java -jar + poll 8090` path if scripts/stest-up.sh is absent in the checked-out system-test branch. Same for the stest-down.sh teardown step. This handles the scenario where merge order across the three involved repos (tron-deployment / system-test / java-tron) skews — e.g., the java-tron workflow change lands upstream before the system-test scripts/ commit does. Without this fallback, every java-tron PR's CI would go red until system-test catches up. With it, CI gracefully degrades to the pre-trond bring-up path. Combined with the script-layer fallback inside stest-up.sh (which handles trond release URL failures), there are now two independent safety nets: Layer 1 (this step): stest-up.sh missing → legacy nohup Layer 2 (stest-up.sh): trond release unreachable → nohup Both layers ultimately land at the same end-state: java-tron started on /usr/bin/java JDK 8, HTTP 8090 reachable, ready for stest TestNG. Log capture also covers both modes: journalctl + node.log (trond), fullnode.log (legacy). Plan: keep the fallback through Phase B and the upstream PR cycle; remove it in a follow-up PR once the trond-mode path is consistently green across all three repos.
warku123
force-pushed
the
feat/system-test-trond
branch
from
796a7de to
f93be63
Compare
1 participant
[fork-only test PR — DO NOT MERGE]
Validates the system-test integration end-to-end on this fork before opening upstream PRs. Will be closed (not merged) once CI is green.
Cross-repo dependencies (all on warku123 forks)
warku123/tron-deploymentv0.1.0 release — trond binary with embeddedsystem-testHOCON templatewarku123/system-test:feat/trond-bridge—scripts/stest-up.shandscripts/stest-down.shwarku123/java-tron:feat/system-test-trond(this branch) —.github/workflows/system-test.ymlchangeWhat this PR validates
The new node bring-up flow:
warku123/system-test:feat/trond-bridge(Phase A temporary; Phase B will repoint totronprotocol/system-test:release_workflow)bash system-test/scripts/stest-up.sh:warku123/tron-deploymentrelease (sha256-pinned)/opt/tron/stest-singlenode/trond recipe run system-test→ systemd-managed node, ports 8090/50051/8545/18888testcase/stest-down.shalways runs to clean up systemd unit + install pathPhase B (after this is green)
actions/checkout'srepositoryback totronprotocol/system-test,refback torelease_workflowTROND_RELEASE_URLdefault instest-up.shtotronprotocol/tron-deploymentupstream releaseLinked
openspec change
system-test-integrationin tronprotocol/tron-deployment (Phase 5 — fork-side end-to-end validation)Summary by cubic
Fork-only CI validation to run system tests using
trondfor node bring-up with deterministic teardown and structured logs. Useswarku123/system-test@feat/trond-bridge; ports and test endpoints remain unchanged.warku123/system-test@feat/trond-bridgeviaactions/checkout.bash system-test/scripts/stest-up.shusingSTEST_JAR; if the script is missing, fall back to legacynohup java -jar(script-layer fallback insidestest-up.shalso remains).system-test/scripts/stest-down.shwhen present to ensure clean runs.journalctlfortron-stest-singlenode, copynode.log(if present) and legacyfullnode.log, then upload asstest-node-logs.trondv0.1.0 fromwarku123/tron-deployment(sha256-pinned); will flip to upstream in Phase B.Written for commit f93be63. Summary will update on new commits.
Summary by CodeRabbit