Add integration tests for pledge reconcile flow - #456
Conversation
|
@meshackyaro is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@meshackyaro Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdates backend integration tests: standardized wallet fixtures and tx-hash helper, switched campaign/pledge payload fields to the new schema, changed history assertions to use ChangesPledge Reconciliation Integration Testing
Sequence DiagramsequenceDiagram
participant Test as Integration Test
participant API as Campaign API
participant ReconcileSvc as Reconcile Service
participant Store as Campaign Store
Test->>API: POST /campaigns (acceptedTokens)
API->>Store: createCampaign(payload)
Store-->>API: campaignCreated(id)
API-->>Test: campaign ID
Test->>API: POST /campaigns/:id/pledges (assetCode)
API->>Store: addPledge(payload)
Store-->>API: pledgeAdded
API-->>Test: pledge confirmed
Test->>API: POST /campaigns/:id/pledges/reconcile (transactionHash)
API->>ReconcileSvc: reconcilePledge(request)
ReconcileSvc->>Store: verifyAndUpdatePledge(txHash)
Store-->>ReconcileSvc: updatedPledgeState
ReconcileSvc-->>API: reconcileResult
API-->>Test: reconciliation response
Test->>API: GET /campaigns/:id/history
API->>Store: getCampaignHistory(id)
Store-->>API: history (includes reconcile event)
API-->>Test: history data
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🧹 Nitpick comments (1)
backend/tests/integration.test.ts (1)
918-923: ⚡ Quick winReal 2.5s sleep risks exceeding the default test timeout.
This test sleeps 2500ms in addition to multiple sequential HTTP round-trips (pledge, claim, two detail fetches, reconcile). Against Vitest's default 5s per-test timeout, this is tight and can flake on slower CI. Consider passing an explicit timeout to
it(...)(and ideally shrinking the wait).♻️ Set an explicit timeout
- it("should prevent reconciling pledge to closed campaign", async () => { + it("should prevent reconciling pledge to closed campaign", async () => { + // ... + }, 15000); // explicit timeout to absorb the 2.5s deadline wait + HTTP latency🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/integration.test.ts` around lines 918 - 923, The real 2.5s sleep plus several HTTP calls can flake the test; update the test that uses the 2500ms setTimeout (the Promise wait before calling claimTestCampaign with CREATOR_1) to both shrink the hard sleep (e.g., reduce to a much smaller value or replace with a short polling loop that checks the campaign deadline) and give the test an explicit larger timeout by passing a timeout to the it(...) call for this test; locate the test containing the await new Promise((resolve) => setTimeout(resolve, 2500)) and the subsequent claimTestCampaign(campaignId, CREATOR_1) and modify the test declaration to set an explicit timeout while reducing/removing the long sleep.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/tests/integration.test.ts`:
- Around line 781-784: Tests in integration.test.ts inconsistently access the
campaign history payload (some use history.data while other places correctly use
history.data.data per the getCampaignHistory AxiosResponse typing); find all
assertions and usages of getCampaignHistory / variable history and change them
to read the events array as history.data.data (e.g., in assertions, length
checks, and indexing) so every test consistently consumes the { data: any[] }
shape returned by GET /api/campaigns/:id/history.
---
Nitpick comments:
In `@backend/tests/integration.test.ts`:
- Around line 918-923: The real 2.5s sleep plus several HTTP calls can flake the
test; update the test that uses the 2500ms setTimeout (the Promise wait before
calling claimTestCampaign with CREATOR_1) to both shrink the hard sleep (e.g.,
reduce to a much smaller value or replace with a short polling loop that checks
the campaign deadline) and give the test an explicit larger timeout by passing a
timeout to the it(...) call for this test; locate the test containing the await
new Promise((resolve) => setTimeout(resolve, 2500)) and the subsequent
claimTestCampaign(campaignId, CREATOR_1) and modify the test declaration to set
an explicit timeout while reducing/removing the long sleep.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad16a369-d2a0-4cad-8b48-f21c9fc8d64a
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
backend/tests/integration.test.ts
|
@meshackyaro please resolve conflicts |
- Implement comprehensive test suite covering full reconcile workflow - Test: Create campaign → Pledge → Reconcile → Verify history → Idempotency - Test: Prevent reconciling pledges that exceed campaign target - Test: Prevent reconciling to closed/claimed campaigns - Test: Reject duplicate transaction hashes across campaigns - Test: Handle multiple reconciled pledges from different contributors - Fix mock Stellar addresses to be exactly 56 characters - All 5 reconcile flow tests passing Closes ritik4ever#316
- Change all history.data accesses to history.data.data
- Ensures consistent consumption of { data: any[] } shape from GET /api/campaigns/:id/history
- Fixes inconsistency where some tests used history.data while others correctly used history.data.data
- All reconcile flow tests still passing
bc17451 to
2ed94ef
Compare
|
✅ Conflicts Resolved I've successfully fetched from upstream, performed a safe rebase, and resolved the conflicts in:
The branch is now cleanly rebased on |
|
Conflicts resolved |
2 participants
Summary
This PR adds comprehensive integration tests for the complete pledge reconcile flow, covering the full sequence from campaign creation to confirmed pledge in event history.
Changes
Test Coverage
Acceptance Criteria Met
Test Results
All 5 tests in the "Pledge Reconcile Flow - Integration" suite are passing.
Closes #316
Summary by CodeRabbit