test(wasm-sdk): expand data contract test coverage #2803
Conversation
- Add comprehensive property validation for V0 and V1 contracts - Test document schemas, config, and metadata fields - Add memory management tests with multiple contract instances - Include JSON round-trip preservation tests - Add defensive assertions for V1-specific features - Comment out problematic assertions for WASM layer investigation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…alues - Add comprehensive platform version compatibility testing for V0/V1 contracts - Test V0 contracts across all platform versions (1-10) - Test V1 contracts work on v9+ and fail on v1-8 - Add edge case testing for invalid version numbers - Replace magic values with dynamic version detection - Use LATEST_KNOWN_VERSION for future-proof invalid version testing - Reorganize tests into logical categories with nested describe blocks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Should pass once the 2794 is merged
WalkthroughAdds comprehensive versioned data-contract unit tests covering V0 and V1, platform-version constants and ranges, a compatibility-testing helper, expanded validation/preservation/memory checks, and a new V1 fixture with documents, groups, tokens, and governance rules. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Runner as Test Runner
participant Spec as data-contract.spec.mjs
participant Helper as testContractAcrossVersions
participant Matrix as Version Matrix Resolver
participant SDK as WASM SDK (builder/validator)
participant Assert as Assertions
Runner->>Spec: run test suites
Spec->>Helper: request compatibility verification for contract
Helper->>Matrix: resolve target platform versions (V0..latest, V1 start)
loop per target version
Helper->>SDK: build/validate contract for version N
alt compatible
SDK-->>Helper: success + normalized contract JSON
Helper->>Assert: verify properties, IDs, round-trip equality
else incompatible
SDK-->>Helper: error/exception
Helper->>Assert: expect failure
end
end
Note over Spec,SDK: Additional flows: invalid inputs, memory tests with multiple contracts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 review |
✅ Actions performedReview triggered.
|
![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
🧹 Nitpick comments (1)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs (1)
18-35: Use thecontractNamelabel in the test titles (optional).
contractNameis currently unused, so the generated test names don’t reveal which fixture regressed. Threading the label into theittitles cleans up the unused parameter and makes failures easier to triage.- it(`should work with platform version ${version}`, () => { + it(`(${contractName}) should work with platform version ${version}`, () => { const contract = sdk.DataContract.fromJSON(contractFixture, version); @@ - incompatibleVersions.forEach(version => { - it(`should fail with platform version ${version}`, () => { + incompatibleVersions.forEach(version => { + it(`(${contractName}) should fail with platform version ${version}`, () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs(1 hunks)packages/wasm-sdk/tests/unit/fixtures/data-contract-v1-with-docs-tokens-groups.mjs(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.28.0)
packages/wasm-sdk/tests/unit/fixtures/data-contract-v1-with-docs-tokens-groups.mjs
[high] 175-175: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
Once the SDK is fixed, test can be enabled.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs (4)
16-21: Consider removing or utilizing the unused parameter.The
contractNameparameter is declared but never used in the function body. You could either:
- Remove it if it's not needed
- Use it to enhance test descriptions:
it(`should work with ${contractName} contract on platform version ${version}`, () => {Apply this diff to remove the unused parameter:
const testContractAcrossVersions = ( contractFixture, - contractName, compatibleVersions, incompatibleVersions = [], ) => {Or apply this diff to use it in test descriptions:
compatibleVersions.forEach((version) => { - it(`should work with platform version ${version}`, () => { + it(`should work with ${contractName} contract on platform version ${version}`, () => { const contract = sdk.DataContract.fromJSON(contractFixture, version);
76-104: Skipped test is acknowledged in PR description.The skipped V1 test is appropriately marked with a TODO comment indicating it's waiting on PR #2794. The test structure looks comprehensive and ready to be enabled once the fix is merged.
However, consider tracking this TODO with a link to the related PR:
- // TODO: enable test once an SDK fix to support this is merged + // TODO: enable test once PR #2794 is merged (https://github.com/dashpay/platform/pull/2794) it.skip('should create a V1 contract from JSON and expose all properties including tokens and groups', async () => {
137-141: Consider a more flexible error assertion.The highly specific regex pattern could make the test brittle if the error message format changes. Consider matching just the essential parts:
expect(() => { sdk.DataContract.fromJSON(contractFixtureV1, PLATFORM_VERSION_CONTRACT_V0); - }).to.throw(/dpp unknown version.*known versions.*\[0\].*received.*1/); + }).to.throw(/unknown version/); });
185-198: Consider a more flexible error assertion.The test correctly validates an important business rule (contracts must have at least one document type or token). However, the specific error message pattern could make the test brittle:
expect(() => { sdk.DataContract.fromJSON(contractWithEmptySchemas, PLATFORM_VERSION_CONTRACT_V0); - }).to.throw(/must have at least one document type or token defined/); + }).to.throw(/at least one document type or token/); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/wasm-sdk/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep WASM SDK docs in sync (run generate_docs.py) when updating the WASM SDK
Files:
packages/wasm-sdk/tests/unit/data-contract.spec.mjs
packages/**/tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Place unit and integration tests alongside each package in packages//tests
Files:
packages/wasm-sdk/tests/unit/data-contract.spec.mjs
🧬 Code graph analysis (1)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs (2)
packages/wasm-sdk/tests/unit/fixtures/data-contract-v1-with-docs-tokens-groups.mjs (1)
contract(1-281)packages/wasm-sdk/tests/unit/fixtures/data-contract-v0-crypto-card-game.mjs (1)
contract(1-96)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (9)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs (9)
1-3: LGTM!The imports are well-organized and appropriate for the expanded test coverage.
5-13: LGTM!The platform version constants and compatibility ranges are well-organized and self-documenting. Using arrays for version ranges makes the test logic clear and maintainable.
50-73: LGTM!The V0 contract creation test is comprehensive and thoroughly validates:
- Contract instantiation
- Property exposure (id, ownerId, version, config, documentSchemas)
- JSON round-trip integrity
- Document schema structure including properties, enums, and indices
Excellent coverage of the V0 contract behavior.
106-133: LGTM!The edge case tests for contracts with only documents or only tokens are well-designed and validate important scenarios:
- Line 106-115: Confirms V0 contracts can exist with only document schemas
- Line 117-133: Confirms V1 contracts can exist with only tokens (no documents)
These tests ensure flexibility in contract composition.
145-183: LGTM!The validation tests provide excellent coverage of error conditions:
- Null and empty object inputs
- Invalid Base58 ID format
- Negative version numbers
- Invalid ownerId format
These tests ensure robust error handling for invalid inputs.
201-229: LGTM!The data preservation tests are critical for ensuring contract integrity through serialization/deserialization cycles. The dual round-trip approach (create → serialize → deserialize → serialize → compare) provides strong validation of data preservation for both V0 and V1 contracts.
232-241: LGTM!Memory management testing is crucial for WASM bindings where resources must be manually freed. This test validates that multiple contract instances can coexist and be properly cleaned up, preventing memory leaks.
244-286: LGTM!The Platform Version Compatibility Matrix is exceptionally comprehensive:
- V0 Compatibility: Tests across all versions 1-10
- V1 Compatibility: Tests success on v9-10 and failure on v1-8
- Edge Cases: Validates behavior with zero, negative, and beyond-range versions
- Boundary Testing: Specifically validates the V9 transition point where V1 support was introduced
This test suite provides excellent coverage of the version compatibility requirements mentioned in the PR objectives. The use of the helper function keeps the tests DRY while maintaining clarity.
1-287: Verify documentation requirements.The coding guidelines state: "Keep WASM SDK docs in sync (run generate_docs.py) when updating the WASM SDK." Since this PR adds comprehensive test coverage without modifying SDK functionality, documentation updates are likely not required. However, please confirm whether any documentation should be updated to reflect the expanded test coverage.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs (1)
5-13: Consider calculating LATEST_KNOWN_VERSION from both compatibility ranges.Currently,
LATEST_KNOWN_VERSIONis calculated from onlyV0_COMPATIBLE_VERSIONS. While this works today sinceV1_COMPATIBLE_VERSIONSis a subset, it could become incorrect if future platform versions are added toV1_COMPATIBLE_VERSIONSbut notV0_COMPATIBLE_VERSIONS.Apply this diff to make the calculation more robust:
-const LATEST_KNOWN_VERSION = Math.max(...V0_COMPATIBLE_VERSIONS); +const LATEST_KNOWN_VERSION = Math.max(...V0_COMPATIBLE_VERSIONS, ...V1_COMPATIBLE_VERSIONS);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/wasm-sdk/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep WASM SDK docs in sync (run generate_docs.py) when updating the WASM SDK
Files:
packages/wasm-sdk/tests/unit/data-contract.spec.mjs
packages/**/tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Place unit and integration tests alongside each package in packages//tests
Files:
packages/wasm-sdk/tests/unit/data-contract.spec.mjs
🧬 Code graph analysis (1)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs (2)
packages/wasm-sdk/tests/unit/fixtures/data-contract-v0-crypto-card-game.mjs (1)
contract(1-96)packages/wasm-sdk/tests/unit/fixtures/data-contract-v1-with-docs-tokens-groups.mjs (1)
contract(1-281)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (8)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs (8)
1-3: LGTM!The fixture imports correctly bring in both V0 and V1 test contracts, supporting the expanded test coverage described in the PR objectives.
15-42: Excellent helper function for reducing test duplication.The
testContractAcrossVersionshelper effectively tests contract compatibility across multiple platform versions, properly handles both success and failure cases, and ensures correct memory cleanup with.free()calls.
49-134: Comprehensive contract creation tests with good edge-case coverage.The tests thoroughly validate:
- V0 contract creation with nested property verification
- V1 contract creation (intentionally skipped until PR #2794 merge, as noted in PR objectives)
- Contracts with only documents (no tokens)
- Contracts with only tokens (no documents)
All tests properly manage WASM memory with
.free()calls.
136-142: LGTM!The version compatibility test correctly validates that V1 contracts fail when attempting to use V0 platform versions, with appropriate error message verification.
144-199: Thorough validation test coverage.The validation tests comprehensively cover:
- Invalid JSON inputs (null, empty objects, malformed structures)
- Invalid property values (Base58 IDs, negative versions, invalid ownerIds)
- Business constraints (requiring at least one document type or token)
201-229: Excellent round-trip data preservation tests.The tests ensure complete data fidelity through multiple JSON serialization/deserialization cycles for both V0 and V1 contracts, which is critical for contract integrity. Memory management is correct with proper
.free()calls.
231-242: LGTM!The memory management test correctly verifies that multiple WASM contract instances can coexist without interference and are properly freed, which is essential for real-world usage scenarios.
244-286: Comprehensive platform version compatibility testing.The test suite systematically validates:
- V0 contracts work across all platform versions (1–10)
- V1 contracts work from version 9 onwards
- Invalid version numbers (0, negative, beyond known range) are properly rejected
- V9 boundary behavior (backward compatibility for V0, first supported version for V1)
The use of
LATEST_KNOWN_VERSIONin edge-case testing makes the tests future-proof, as highlighted in the PR objectives.Based on the coding guidelines for
packages/wasm-sdk/**, verify whether documentation needs to be updated. The guideline states to "Keep WASM SDK docs in sync (run generate_docs.py) when updating the WASM SDK." Since this PR only updates tests and not the SDK implementation itself, documentation updates may not be required. However, please confirm whether the expanded test coverage warrants any documentation changes.
|
The one currently failing test case is due to an upstream bug (rs-dapi not returning expected data for GetStatus). |
|
All tests passing now 👍 |
|
|
||
| // Platform version compatibility ranges | ||
| const V0_COMPATIBLE_VERSIONS = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; // V0 works across all versions | ||
| const V1_COMPATIBLE_VERSIONS = [9, 10]; // V1 only works from version 9+ |
There was a problem hiding this comment.
I would do a 9+ instead of a 9, 10, right?
Issue being fixed or feature implemented
Adds tests to cover more scenarios such as one being addressed by #2794 per discussion in #2791 (comment).
Note: one test case is skipped until the SDK is fixed
expected to fail until the versioning fix in 2794 is merged.What was done?
Expanded data contract testing including:
How Has This Been Tested?
Running tests locally
Breaking Changes
N/A
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit