Conversation
Excludes identity update (disable key), register name, document update, document delete
📝 WalkthroughWalkthroughAdds end-to-end tutorial testing: a GitHub Actions workflow, new npm test scripts, subprocess-based test runner and assertion utilities, plus read-only and read-write test suites that execute tutorial scripts and validate outputs. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as Ubuntu VM (job)
participant Node as Node/npm
participant Script as Tutorial script
participant Asserts as Test assertions
GH->>Runner: trigger job (push/PR/dispatch)
Runner->>Runner: checkout code\nsetup Node.js\nnpm ci
Runner->>Node: npm run test:read-*/test:all
Node->>Script: spawn node <tutorial script>
Script-->>Node: stdout/stderr, exit
Node->>Asserts: pass output to assertions
Asserts-->>Node: success/failure result
Node-->>Runner: test job exit status
Runner-->>GH: job result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test-tutorials.yml:
- Around line 40-44: The "Run read-only tutorial tests" job is exporting the
secret PLATFORM_MNEMONIC into the step environment even for pull_request runs;
remove PLATFORM_MNEMONIC from the env block (or conditionally inject it only for
trusted runs) so that the step that runs npm run test:read-only does not receive
the funded mnemonic; alternatively, move any tests that require
PLATFORM_MNEMONIC behind a trusted/manual workflow or a separate job that only
runs on push/dispatch so the secret is never exposed to untrusted PRs.
In `@package.json`:
- Around line 9-12: Add an engines constraint to package.json so contributors
run a supported Node version for the test CLI flags: update package.json to
include "engines": { "node": ">=20.11.0" } (this ensures the test scripts
"test", "test:read-only", "test:read-write", and "test:all" that use
--test-timeout work correctly).
In `@test/read-write.test.mjs`:
- Around line 20-28: Add assertions that the spawned subprocess exited
successfully: after the existing assert.match on result.stdout,
assert.strictEqual(result.exitCode, 0, `Expected zero exit code.\nSTDOUT:
${result.stdout}\nSTDERR: ${result.stderr}`) and
assert.strictEqual(result.killed, false, `Process was killed
unexpectedly.\nSTDOUT: ${result.stdout}\nSTDERR: ${result.stderr}`) so the test
requires both the "Identity registered!" output and a successful process
termination; keep the existing stdout/stderr diagnostic strings and the current
workaround that reads result.stdout/result.stderr.
- Around line 107-109: The test currently silently continues when
extractId(result.stdout) returns falsy, leaving state.dataContractId unset;
update the block around extractId(result.stdout) so that if no id is returned
you immediately fail the test (e.g., throw an Error or use an assertion) with a
clear message including the stdout, otherwise set state.dataContractId to the
id; refer to the extractId function result and state.dataContractId assignment
to implement the early-fail behavior so downstream tests don’t silently skip.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee8bac4f-baa9-49c4-805a-727861c13b15
📒 Files selected for processing (6)
.github/workflows/test-tutorials.ymlpackage.jsontest/assertions.mjstest/read-only.test.mjstest/read-write.test.mjstest/run-tutorial.mjs
Fail early when extractId returns null so downstream tests don't silently skip. Also support the $id key format used by document output. Update readme
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/read-write.test.mjs (1)
20-28:⚠️ Potential issue | 🟠 MajorAssert process success here as well.
This still goes green on a non-zero exit or timeout as long as stdout contains
Identity registered!. Keep the special-case output check if needed, but also requireresult.killed === falseandresult.exitCode === 0so the subprocess actually succeeded.Suggested tightening
assert.match( result.stdout, /Identity registered!/, `Expected identity registration output.\nSTDOUT: ${result.stdout}\nSTDERR: ${result.stderr}`, ); - console.log(result.stdout) + assert.equal( + result.killed, + false, + `identity-register timed out.\nSTDOUT: ${result.stdout}\nSTDERR: ${result.stderr}`, + ); + assert.equal( + result.exitCode, + 0, + `identity-register exited with code ${result.exitCode}.\nSTDOUT: ${result.stdout}\nSTDERR: ${result.stderr}`, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/read-write.test.mjs` around lines 20 - 28, The test currently only asserts stdout contains "Identity registered!" via the assert.match call; tighten it by also asserting the child process actually succeeded: add checks that result.killed === false and result.exitCode === 0 immediately after the assert.match (or before) so the test fails on non-zero exit or timeout; reference the existing result object fields result.killed and result.exitCode and the assert.match usage to locate where to insert the two additional assertions.
🧹 Nitpick comments (1)
README.md (1)
57-65: Documentnpm run test:read-onlyexplicitly too.
npm testis enough for the default path, but the PR also adds a namedtest:read-onlyscript. Listing that alias here would keep the README aligned with the workflow wording and make the available entry points clearer for contributors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 57 - 65, Update the README test section to explicitly mention the named script alias "npm run test:read-only" alongside "npm test" so the documentation matches the new package.json scripts; reference both "npm test" (default/read-only) and "npm run test:read-only" (explicit read-only alias), and keep the descriptions for "npm run test:read-write" and "npm run test:all" unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/read-write.test.mjs`:
- Around line 20-28: The test currently only asserts stdout contains "Identity
registered!" via the assert.match call; tighten it by also asserting the child
process actually succeeded: add checks that result.killed === false and
result.exitCode === 0 immediately after the assert.match (or before) so the test
fails on non-zero exit or timeout; reference the existing result object fields
result.killed and result.exitCode and the assert.match usage to locate where to
insert the two additional assertions.
---
Nitpick comments:
In `@README.md`:
- Around line 57-65: Update the README test section to explicitly mention the
named script alias "npm run test:read-only" alongside "npm test" so the
documentation matches the new package.json scripts; reference both "npm test"
(default/read-only) and "npm run test:read-only" (explicit read-only alias), and
keep the descriptions for "npm run test:read-write" and "npm run test:all"
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf0594df-be38-410b-8590-d690ab198098
📒 Files selected for processing (3)
README.mdtest/assertions.mjstest/read-write.test.mjs
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
1 participant
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.
Summary
node:test— zero new dependenciesDetails
Test architecture
Each tutorial is spawned as a child process with
child_process.execFile. The harness captures stdout/stderr, checks exit codes, and validates output against expected patterns. Environment variables are injected to pass state between dependent tutorials (e.g., contract ID from registration flows into document submission).Files added
test/run-tutorial.mjs— subprocess runnertest/assertions.mjs— shared validation helpers (assertTutorialSuccess,extractId)test/read-only.test.mjs— 9 read-only tutorial teststest/read-write.test.mjs— 12 sequential write tutorial tests with state passing.github/workflows/test-tutorials.yml— CI with choice dropdown (read-only / read-write / all)Scripts
npm test/npm run test:read-only— read-only testsnpm run test:read-write— write pipeline (sequential, consumes testnet credits)npm run test:all— both suitesNot yet testable (hardcoded values, no env var override)
document-update.mjs,document-delete.mjs—DOCUMENT_IDhardcodedname-register.mjs—NAME_LABELhardcodedidentity-update-disable-key.mjs—KEY_IDhardcodedTest plan
npm run test:read-onlypasses (9/9)npm run test:read-writepasses with funded testnet walletSummary by CodeRabbit