-
Notifications
You must be signed in to change notification settings - Fork 1
test: add test for contract create with history #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Change form field property naming in API definitions to use 'default' instead of 'defaultValue' for consistency with standard conventions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Reveals a bug in platform
WalkthroughUpdated API schema field naming from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
public/api-definitions.json (1)
1666-1671: Same field name change as the previous checkbox.This change is consistent with the previous checkbox update.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/api-definitions.json(1 hunks)tests/e2e/transitions/state-transitions.spec.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/e2e/transitions/state-transitions.spec.js (1)
tests/e2e/queries/query-execution.spec.js (4)
result(39-39)evoSdkPage(325-325)parameterInjector(326-326)contractData(172-172)
⏰ 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). (1)
- GitHub Check: Run SDK Site UI Tests
🔇 Additional comments (1)
public/api-definitions.json (1)
1659-1664: No breaking change—frontend code already uses thedefaultfield.The verification shows that
public/app.jsline 1248 explicitly reads fromfieldSchema.default, notdefaultValue. The field name change aligns with the frontend's actual consumption pattern and does not introduce any breaking changes.Likely an incorrect or invalid review comment.
| test('should create data contract with history enabled', async () => { | ||
| // Execute the data contract create transition with keepsHistory: true | ||
| const result = await executeStateTransitionWithCustomParams( | ||
| evoSdkPage, | ||
| parameterInjector, | ||
| 'dataContract', | ||
| 'dataContractCreate', | ||
| 'testnet', | ||
| { keepsHistory: true } // Override to enable history | ||
| ); | ||
|
|
||
| // Validate basic result structure | ||
| validateBasicStateTransitionResult(result); | ||
|
|
||
| // Validate data contract creation specific result | ||
| validateDataContractResult(result.result, false); | ||
|
|
||
| // Parse result and verify keepsHistory is set to true | ||
| const contractData = JSON.parse(result.result); | ||
| expect(contractData).toBeDefined(); | ||
| expect(contractData.contractId).toBeDefined(); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add assertion to verify that keepsHistory is actually set in the created contract.
The test comment on Line 521 states "Parse result and verify keepsHistory is set to true", but the actual implementation only checks that contractId is defined. This means the test doesn't verify that the keepsHistory: true parameter was actually applied to the created contract.
Apply this diff to add the missing verification:
// Parse result and verify keepsHistory is set to true
const contractData = JSON.parse(result.result);
expect(contractData).toBeDefined();
expect(contractData.contractId).toBeDefined();
+ expect(contractData.keepsHistory).toBe(true);🤖 Prompt for AI Agents
In tests/e2e/transitions/state-transitions.spec.js around lines 504 to 525, the
test claims to verify keepsHistory is set to true but only asserts contractId is
defined; update the test to parse the created contract object and add an
assertion that contractData.keepsHistory (or
contractData.definition?.keepsHistory depending on shape) strictly equals true
so the keepsHistory override is actually validated.
Related to dashpay/platform#2842
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests