Skip to content

Improve e2e test performance with polling and parallelization#3415

Merged
steven-tey merged 2 commits intomainfrom
restructure-tests
Feb 4, 2026
Merged

Improve e2e test performance with polling and parallelization#3415
steven-tey merged 2 commits intomainfrom
restructure-tests

Conversation

@devkiran
Copy link
Collaborator

@devkiran devkiran commented Feb 4, 2026

Summary by CodeRabbit

  • Tests
    • Optimized test suite execution with parallel test runs for improved performance
    • Enhanced commission verification with polling mechanism and timeout handling for more reliable transaction tracking

@vercel
Copy link
Contributor

vercel bot commented Feb 4, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
dub Ready Ready Preview Feb 4, 2026 4:58am

Request Review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 4, 2026

📝 Walkthrough

Walkthrough

Test execution patterns shift from sequential to parallel HTTP requests using Promise.all, and commission verification moves from single blocking fetch to a polling mechanism with configurable intervals and timeout thresholds.

Changes

Cohort / File(s) Summary
Concurrent Test Execution
apps/web/tests/tracks/track-sale.test.ts
Test suite converted to concurrent execution with describe.concurrent, wrapping multiple POST requests in Promise.all for parallel execution and consolidated commission verifications into single awaited promises.
Commission Polling Mechanism
apps/web/tests/utils/verify-commission.ts
Replaced single fetch with polling loop (5s interval, 30s timeout), removed initial 3s delay, moved customer ID resolution earlier, and added in-loop verification for invoiceId, customerId, amount, and earnings.

Sequence Diagram

sequenceDiagram
    participant Test as Test Suite
    participant API as API Endpoints
    participant Verify as verifyCommission()
    
    Test->>Test: Issue multiple POST /track/sale<br/>(in parallel via Promise.all)
    
    Note over Test,API: Old behavior (sequential)
    Test->>API: POST /track/sale (wait)
    API-->>Test: Response
    Test->>API: POST /track/sale (wait)
    API-->>Test: Response
    
    Note over Test,Verify: New behavior (polling)
    Test->>Verify: verifyCommission(invoiceId)
    loop Every 5s until 30s timeout
        Verify->>API: GET /customers
        API-->>Verify: Customer list
        Verify->>API: GET /commissions
        API-->>Verify: Commission data
        alt Status 200 & exactly one commission
            Verify->>Verify: Verify invoiceId,<br/>customerId,<br/>amount, earnings
            Verify-->>Test: ✓ Success
        else Not ready yet
            Note over Verify: Wait 5s, retry
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #3020: Modifies the same verify-commission.ts test utility with potential interaction points for commission verification logic.
  • PR #2645: Updates the same test area (track-sale.test.ts) and commission verification helper with related reward modifier changes.

Suggested reviewers

  • steven-tey

Poem

🐰 Hop hop, tests run fast and free,
Polling waits with melody,
Concurrent dreams, no more delays,
Commissions verified in all the ways!

🚥 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 title accurately captures the two main improvements: replacing sequential test execution with concurrent execution and adding polling logic to verify commissions, directly reflecting the primary changes in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch restructure-tests

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/tests/tracks/track-sale.test.ts (1)

42-78: ⚠️ Potential issue | 🟠 Major

Concurrent suite makes the idempotency test unreliable.

describe.concurrent runs tests in parallel using Promise.all. Both tests currently use the same shared sale.invoiceId, so when they execute concurrently, the second POST request for "already processed invoiceId" is not guaranteed to hit an already-processed state—both requests may hit the server simultaneously. This makes the idempotency test ineffective.

Fix this by making the idempotency test self-contained: have a single test post the same invoiceId twice sequentially within that test, then verify the responses match.

✅ Suggested fix (self-contained idempotency test)
-  test("track a sale with an invoiceId that is already processed (should return the same response as before) ", async () => {
-    const response = await http.post<TrackSaleResponse>({
-      path: "/track/sale",
-      body: {
-        ...sale,
-        customerExternalId: E2E_CUSTOMER_EXTERNAL_ID,
-        invoiceId: sale.invoiceId,
-      },
-    });
-
-    // should return the same response since it's idempotent
-    expectValidSaleResponse(response, sale);
-  });
+  test("track a sale with an invoiceId that is already processed (should return the same response as before) ", async () => {
+    const invoiceId = `INV_${randomId()}`;
+    const salePayload = {
+      ...sale,
+      customerExternalId: E2E_CUSTOMER_EXTERNAL_ID,
+      invoiceId,
+    };
+
+    const first = await http.post<TrackSaleResponse>({
+      path: "/track/sale",
+      body: salePayload,
+    });
+    const second = await http.post<TrackSaleResponse>({
+      path: "/track/sale",
+      body: salePayload,
+    });
+
+    expectValidSaleResponse(first, salePayload);
+    expectValidSaleResponse(second, salePayload);
+  });
🤖 Fix all issues with AI agents
In `@apps/web/tests/tracks/track-sale.test.ts`:
- Around line 115-127: The test currently uses optional chaining plus non-null
assertion (response1.data.sale?.amount! / response2.data.sale?.amount!) which
violates noNonNullAssertedOptionalChain; instead guard that sale exists first
(e.g., assert/throw/expect that response1.data.sale and response2.data.sale are
non-null) and then access .amount directly when calling verifyCommission; update
the calls to verifyCommission (referencing verifyCommission, response1,
response2, expectedAmount) to remove the ?. and ! by first checking
responseX.data.sale and then using responseX.data.sale.amount.

@steven-tey steven-tey merged commit 693bfd3 into main Feb 4, 2026
10 of 11 checks passed
@steven-tey steven-tey deleted the restructure-tests branch February 4, 2026 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants