test: add setupDashClient unit tests and mnemonic validation by thephez · Pull Request #60 · dashpay/platform-tutorials

thephez · 2026-03-14T16:42:56Z

Summary

Add comprehensive unit tests for setupDashClient covering client creation, key derivation, DIP-13 path logic, identity handling, and error paths (~920 lines, 138 assertions)
Validate mnemonic in setupDashClient before key derivation, throwing a clear error for invalid BIP39 mnemonics
Add test:setup script for running setupDashClient tests via Mocha
Add Chai and Mocha dev dependencies; pin existing devDependencies to exact versions

Summary by CodeRabbit

Bug Fixes
- Improved mnemonic validation with clearer error messages.
New Features
- Added a test setup script and updated test runner invocation.
- Enabled dotenv-based config loading for tests.
Tests
- Added extensive integration tests for key management, identity handling, client setup, deterministic key/address derivation, and error paths.
Chores
- Updated dependencies and devDependencies, including new testing libraries.

Replace hardcoded DIP-13 path construction in deriveKeysFromMnemonic with the exported dip13KeyPath() helper, ensuring a single source of truth for path derivation. Add test cases for missing setupDashClient() permutations including undefined manager guards, requireIdentity flag, and identityIndex forwarding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Catch invalid BIP39 mnemonics early with a clear error message instead of letting them propagate to the WASM SDK as opaque WasmSdkError objects. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Verify key-role correctness (keyId per getSigner method), signer depth (keyCount for getMaster/getFullSigner), createClient success branches (testnet/mainnet isConnected), findNextIndex loop logic with stubbed occupied indices, createForNewIdentity nonzero auto-index, and AddressKeyManager getInfo/getInfoAt with stubbed SDK. Fix stale comment and remove weak clientConfig direct-usage tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Don't run setup in test:all

coderabbitai · 2026-03-14T16:43:15Z

📝 Walkthrough

Walkthrough

Add runtime mnemonic validation to SDK setup, introduce dotenv and new test scripts/dependencies, and add extensive integration tests plus conditional mnemonic-gated read-only tests.

Changes

Cohort / File(s)	Summary
Package + Test scripts `package.json`	Added `test:setup` script; updated `test:all` string (trailing comma); moved `dotenv` to `dependencies` (v17.3.1); removed `dotenv` from devDeps; added `chai` 6.2.2 and `mocha` 11.7.5; `eslint` pinned at 8.45.0.
SDK mnemonic validation `setupDashClient.mjs`	Added runtime mnemonic validation: calls `wallet.validateMnemonic(mnemonic)` and throws a clear error when an invalid mnemonic is provided prior to client/key manager initialization.
Integration tests `test/setupDashClient.test.mjs`	Added ~919-line integration test suite exercising IdentityKeyManager, AddressKeyManager, createClient, setupDashClient; includes helpers to derive keys, mock occupied indices, and many cases for deterministic keys, signer/address behavior, identity indexing, and error paths.
Read-only tests (dotenv + gating) `test/read-only.test.mjs`	Imported and invoked `dotenv.config()`; added `requiresMnemonic: true` to a tutorial entry; introduced `hasMnemonic = !!process.env.PLATFORM_MNEMONIC` and conditional test execution (it vs it.skip) when a test requires a mnemonic.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through keys and tests today,

Mnemonics checked to keep mistakes at bay,
New scripts bloom, and mocks parade,
Keys align, decisions made,
Cheers — the client starts to play. 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately summarizes the main changes: adding setupDashClient unit tests and implementing mnemonic validation in setupDashClient, which are the core objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📝 Coding Plan

Generate coding plan for human review comments

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

test/setupDashClient.test.mjs (1)
744-772: Consider removing or converting the skipped test to a documentation comment.

This skipped test documents a potential future improvement (clearer errors when keyManager is undefined) but doesn't actively test anything. If the behavior isn't implemented, consider either:

Removing the test and tracking the improvement as a GitHub issue

Converting it to a brief code comment explaining the limitation

Skipped tests can accumulate and become stale over time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/setupDashClient.test.mjs` around lines 744 - 772, The skipped test case
using it.skip titled 'should not produce a TypeError when write tutorials use
keyManager without a mnemonic' is documentation-only and should be removed or
converted to a comment; either delete the entire it.skip block (including
references to setupDashClient, clientConfig.mnemonic, keyManager and
addressKeyManager) and open a GitHub issue to track the improvement, or replace
the it.skip block with a short inline comment describing the limitation and the
desired future behavior so the test suite contains no skipped tests.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/setupDashClient.test.mjs`:
- Around line 744-772: The skipped test case using it.skip titled 'should not
produce a TypeError when write tutorials use keyManager without a mnemonic' is
documentation-only and should be removed or converted to a comment; either
delete the entire it.skip block (including references to setupDashClient,
clientConfig.mnemonic, keyManager and addressKeyManager) and open a GitHub issue
to track the improvement, or replace the it.skip block with a short inline
comment describing the limitation and the desired future behavior so the test
suite contains no skipped tests.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 33812d60-d18c-47bf-9824-ddcc6529ad00

📥 Commits

Reviewing files that changed from the base of the PR and between 9115cda and 68cae9f.

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (3)

package.json
setupDashClient.mjs
test/setupDashClient.test.mjs

thephez · 2026-03-14T16:55:03Z

Close in favor of #61 which can run tests successfully (not from fork)

Tutorials depend on dotenv to load .env files, so it should be a regular dependency to ensure it is available after `npm install`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fork PRs don't have access to repository secrets, so identity-retrieve fails in CI. Skip tests marked with requiresMnemonic when the env var is absent. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

coderabbitai

🧹 Nitpick comments (1)

package.json (1)

12-13: Include setup tests in the aggregate test command.

At Line 12, test:all does not run the new test:setup suite from Line 13, so “all” tests can still miss setupDashClient coverage.

💡 Proposed update

-    "test:all": "node --test --test-timeout=300000 --test-concurrency=1 test/read-only.test.mjs test/read-write.test.mjs",
+    "test:all": "node --test --test-timeout=300000 --test-concurrency=1 test/read-only.test.mjs test/read-write.test.mjs && npm run test:setup",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 12 - 13, The package.json script "test:all"
currently omits the setup suite "test:setup"; update the "test:all" definition
so it runs the setup tests too — either append "test/setupDashClient.test.mjs"
to the list of files executed by "test:all" or change the command to invoke the
"test:setup" script (e.g., run "npm run test:setup" before the other tests using
&&); edit the "test:all" entry to reference the "test:setup" script or file so
"test:setup" is executed as part of "test:all".

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@package.json`:
- Around line 12-13: The package.json script "test:all" currently omits the
setup suite "test:setup"; update the "test:all" definition so it runs the setup
tests too — either append "test/setupDashClient.test.mjs" to the list of files
executed by "test:all" or change the command to invoke the "test:setup" script
(e.g., run "npm run test:setup" before the other tests using &&); edit the
"test:all" entry to reference the "test:setup" script or file so "test:setup" is
executed as part of "test:all".

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 81c4201b-c419-4fd2-b457-76c0a58fa1ad

📥 Commits

Reviewing files that changed from the base of the PR and between 68cae9f and 394f94f.

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (2)

package.json
test/read-only.test.mjs

thephez and others added 7 commits March 13, 2026 16:12

test: add setup dash client test cases and related dependencies

ffb778d

fix: validate mnemonic in setupDashClient before key derivation

b6ca0af

Catch invalid BIP39 mnemonics early with a clear error message instead of letting them propagate to the WASM SDK as opaque WasmSdkError objects. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

test: add dip13KeyPath unit tests for path derivation

2943bbd

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

chore: update test scripts

83f42f1

Don't run setup in test:all

chore: update mocha to latest and pin

68cae9f

coderabbitai bot reviewed Mar 14, 2026

View reviewed changes

thephez closed this Mar 14, 2026

thephez and others added 2 commits March 14, 2026 13:09

build: move dotenv from devDependencies to dependencies

8cef1de

Tutorials depend on dotenv to load .env files, so it should be a regular dependency to ensure it is available after `npm install`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

test: skip mnemonic-dependent tests when PLATFORM_MNEMONIC is unset

394f94f

Fork PRs don't have access to repository secrets, so identity-retrieve fails in CI. Skip tests marked with requiresMnemonic when the env var is absent. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

thephez reopened this Mar 14, 2026

coderabbitai bot reviewed Mar 14, 2026

View reviewed changes

thephez closed this Mar 14, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add setupDashClient unit tests and mnemonic validation#60

test: add setupDashClient unit tests and mnemonic validation#60
thephez wants to merge 9 commits intodashpay:mainfrom
thephez:test/setupdashclient-test

thephez commented Mar 14, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 14, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

thephez commented Mar 14, 2026

Uh oh!

coderabbitai bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

thephez commented Mar 14, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

thephez commented Mar 14, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

thephez commented Mar 14, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 14, 2026 •

edited

Loading