test: platform-test-suite accepts DAPI_ADDRESSES #2798
Conversation
WalkthroughAdds environment-driven DAPI endpoint selection across test client creators: parse DAPI_ADDRESSES and prefer them over seed discovery; otherwise fall back to getDAPISeeds(). README mentions setting DAPI_ADDRESSES to bypass SML discovery. No exported API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Tester
participant Env as Env (DAPI_ADDRESSES)
participant Factory as create*Client(...)\n(test suite)
participant Seeds as getDAPISeeds()
participant Dash as new Dash.Client(opts)
Tester->>Factory: invoke
Factory->>Env: read DAPI_ADDRESSES
alt DAPI_ADDRESSES provided and non-empty
note right of Factory: Build opts with dapiAddresses
else
Factory->>Seeds: fetch seeds
Seeds-->>Factory: seeds[]
note right of Factory: Build opts with seeds
end
Factory->>Dash: instantiate client
Dash-->>Factory: client
Factory-->>Tester: return client
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
♻️ Duplicate comments (2)
packages/platform-test-suite/lib/test/createFaucetClient.js (1)
24-27: Extract duplicated DAPI address parsing logic.This parsing logic is duplicated from
createClientWithoutWallet.jsandcreateClientWithFundedWallet.js. See the refactoring suggestion increateClientWithoutWallet.js(lines 8-11).packages/platform-test-suite/lib/test/createClientWithFundedWallet.js (1)
24-27: Extract duplicated DAPI address parsing logic.This parsing logic is duplicated from
createClientWithoutWallet.jsandcreateFaucetClient.js. See the refactoring suggestion increateClientWithoutWallet.js(lines 8-11).
🧹 Nitpick comments (3)
packages/platform-test-suite/lib/test/createClientWithoutWallet.js (2)
8-11: Extract duplicated DAPI address parsing logic.This parsing logic is duplicated across three files:
createClientWithoutWallet.js,createClientWithFundedWallet.js, andcreateFaucetClient.js. Extract it into a shared helper function to improve maintainability.Consider creating a helper function in a shared location:
// lib/test/parseDapiAddresses.js function parseDapiAddresses() { return (process.env.DAPI_ADDRESSES || '') .split(',') .map((address) => address.trim()) .filter(Boolean); } module.exports = parseDapiAddresses;Then import and use it in all three files:
+const parseDapiAddresses = require('./parseDapiAddresses'); + function createClientWithoutWallet() { - const dapiAddresses = (process.env.DAPI_ADDRESSES || '') - .split(',') - .map((address) => address.trim()) - .filter(Boolean); + const dapiAddresses = parseDapiAddresses();
8-11: Consider validating DAPI address format.The parsing accepts any non-empty string. Invalid formats like "foo:bar" or malformed addresses will pass through and potentially cause runtime errors when the Dash client attempts to connect.
Consider adding basic validation:
const dapiAddresses = (process.env.DAPI_ADDRESSES || '') .split(',') .map((address) => address.trim()) - .filter(Boolean); + .filter((address) => { + if (!address) return false; + // Basic validation: ensure format is host:port or just host + const parts = address.split(':'); + if (parts.length > 2) return false; + if (parts.length === 2 && isNaN(parseInt(parts[1], 10))) return false; + return true; + });packages/platform-test-suite/lib/test/createClientWithFundedWallet.js (1)
39-43: Consider using conditional spread for consistency.This file uses an if-else block to conditionally add properties, while
createClientWithoutWallet.jsandcreateFaucetClient.jsuse a conditional spread operator for the same purpose. Using consistent patterns improves codebase maintainability.Apply this diff for consistency:
- if (dapiAddresses.length > 0) { - clientOpts.dapiAddresses = dapiAddresses; - } else { - clientOpts.seeds = getDAPISeeds(); - } + Object.assign(clientOpts, + dapiAddresses.length > 0 + ? { dapiAddresses } + : { seeds: getDAPISeeds() } + );Or refactor the entire
clientOptsconstruction to match the pattern in the other files:const clientOpts = { + ...(dapiAddresses.length > 0 + ? { dapiAddresses } + : { seeds: getDAPISeeds() }), network: process.env.NETWORK, timeout: 25000, apps: { dpns: { contractId: dpnsContractId, }, }, }; - - if (dapiAddresses.length > 0) { - clientOpts.dapiAddresses = dapiAddresses; - } else { - clientOpts.seeds = getDAPISeeds(); - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/platform-test-suite/README.md(2 hunks)packages/platform-test-suite/lib/test/createClientWithFundedWallet.js(2 hunks)packages/platform-test-suite/lib/test/createClientWithoutWallet.js(1 hunks)packages/platform-test-suite/lib/test/createFaucetClient.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/platform-test-suite/**
📄 CodeRabbit inference engine (AGENTS.md)
packages/platform-test-suite/**: Keep end-to-end tests and helpers in packages/platform-test-suite
Keep all E2E tests exclusively in packages/platform-test-suite
Files:
packages/platform-test-suite/README.mdpackages/platform-test-suite/lib/test/createClientWithoutWallet.jspackages/platform-test-suite/lib/test/createClientWithFundedWallet.jspackages/platform-test-suite/lib/test/createFaucetClient.js
packages/**/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/**/**/*.{js,ts,jsx,tsx}: Adhere to ESLint with Airbnb/TypeScript configs for JS/TS code
Use camelCase for JS/TS variables and functions
Use PascalCase for JS/TS classes
Prefer kebab-case filenames within JS packages
Files:
packages/platform-test-suite/lib/test/createClientWithoutWallet.jspackages/platform-test-suite/lib/test/createClientWithFundedWallet.jspackages/platform-test-suite/lib/test/createFaucetClient.js
🧬 Code graph analysis (3)
packages/platform-test-suite/lib/test/createClientWithoutWallet.js (2)
packages/platform-test-suite/lib/test/createClientWithFundedWallet.js (3)
dapiAddresses(24-27)Dash(1-1)getDAPISeeds(9-9)packages/platform-test-suite/lib/test/createFaucetClient.js (3)
dapiAddresses(24-27)Dash(1-1)getDAPISeeds(19-19)
packages/platform-test-suite/lib/test/createClientWithFundedWallet.js (2)
packages/platform-test-suite/lib/test/createClientWithoutWallet.js (2)
dapiAddresses(8-11)getDAPISeeds(5-5)packages/platform-test-suite/lib/test/createFaucetClient.js (3)
dapiAddresses(24-27)clientOpts(29-39)getDAPISeeds(19-19)
packages/platform-test-suite/lib/test/createFaucetClient.js (2)
packages/platform-test-suite/lib/test/createClientWithFundedWallet.js (3)
dapiAddresses(24-27)clientOpts(29-37)getDAPISeeds(9-9)packages/platform-test-suite/lib/test/createClientWithoutWallet.js (2)
dapiAddresses(8-11)getDAPISeeds(5-5)
🔇 Additional comments (4)
packages/platform-test-suite/README.md (1)
42-42: LGTM!The documentation accurately describes the new DAPI_ADDRESSES environment variable and its purpose to bypass SML discovery.
Also applies to: 82-82
packages/platform-test-suite/lib/test/createClientWithoutWallet.js (1)
14-16: LGTM!The conditional logic correctly prioritizes environment-provided DAPI addresses over seed discovery, with a clean fallback mechanism.
packages/platform-test-suite/lib/test/createFaucetClient.js (1)
30-32: LGTM!The conditional spread operator correctly integrates DAPI addresses when available, maintaining backward compatibility with seed-based discovery.
packages/platform-test-suite/lib/test/createClientWithFundedWallet.js (1)
39-43: LGTM!The logic correctly prioritizes DAPI_ADDRESSES over seed discovery and integrates properly with the existing client configuration.
1 participant
Issue being fixed or feature implemented
When testing DAPI, we sometimes want to direct platform-test-suite tests to use some prefefined DAPI endpoint
What was done?
Added support for DAPI_ADDRESSES env variable.
How Has This Been Tested?
Local devnet with
DAPI_ADDRESSES=127.0.0.1:2443Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Documentation