Skip to content

Colocate payment and webhook integration tests - #1899

Merged
stefan-burke merged 2 commits into
mainfrom
split/payment-test-locations
Jul 23, 2026
Merged

Colocate payment and webhook integration tests#1899
stefan-burke merged 2 commits into
mainfrom
split/payment-test-locations

Conversation

@stefan-burke

@stefan-burke stefan-burke commented Jul 23, 2026

Copy link
Copy Markdown
Member

What changed

Moved three payment/webhook test files from test/lib/ to colocate them with the integration suites that already live under test/integration/server/:

  • test/lib/server-payments-success-tokens.test.tstest/integration/server/payments-success-tokens.test.ts
  • test/lib/server-payments/_shared-setup.tstest/integration/server/payments/_shared-setup.ts
  • test/lib/server-webhooks/service-charge-scenario.tstest/integration/server/webhooks/service-charge-scenario.ts

The two helpers are exact 100% renames. The token suite is detected as a 98% rename because its relative helper import (./payment-success-helpers.ts) became an aliased one (#test/lib/payment-success-helpers.ts) so Biome re-sorted the import block — no test logic changed.

The four suites that imported the moved helpers (payments/success, payments/replay, webhooks/modifiers, webhooks/modifier-refunds) had only the import path updated to #test/integration/server/....

Why

These three files were the last payment/webhook holdouts under test/lib/ while every suite that uses them already lives under test/integration/server/. Colocating them keeps related tests in one place and matches the mutation mapper's exempt test/integration/ prefix, so they're no longer flagged as misplaced unit tests.

Verification

  • deno task test:files on all five affected suites: 32 passed
  • deno task test:quality-audit: no findings in any moved or importing suite
  • deno task precommit: passed (typecheck, lint, cpd, tests)
  • Git recognizes both helpers as 100% renames and the token suite as a 98% rename

Summary by CodeRabbit

  • Tests
    • Improved payment integration test setup with shared sold-out listing helpers and consistent helper imports.
    • Added a new reusable integration webhook helper for service-charge scenarios (creates a listing plus a percent-based “Service charge” modifier).
    • Updated existing webhook and payment tests to use the new integration helper locations.
    • Test assertions and logic remain unchanged; only setup/helper wiring was adjusted.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 24c05cbc-1283-401d-97f2-ea2e2cd2a0f1

📥 Commits

Reviewing files that changed from the base of the PR and between 22452eb and 2d30489.

📒 Files selected for processing (2)
  • test/integration/server/payments/_shared-setup.ts
  • test/integration/server/webhooks/service-charge-scenario.ts

📝 Walkthrough

Walkthrough

Payment and webhook integration tests now import helpers from integration-specific paths. The payment setup helper declares its listing return type, and a webhook helper creates a listing with a 10% service-charge modifier.

Changes

Integration test helper relocation

Layer / File(s) Summary
Payment sold-out listing setup
test/integration/server/payments/_shared-setup.ts, test/integration/server/payments/*, test/integration/server/payments-success-tokens.test.ts
Adds an explicit Promise<TestListing> return type to fillSoldOutListing and updates payment test helper imports.
Webhook service-charge scenario
test/integration/server/webhooks/service-charge-scenario.ts, test/integration/server/webhooks/modifier-refunds.test.ts, test/integration/server/webhooks/modifiers.test.ts
Adds createServiceChargeScenario, which creates a listing and 10% charge modifier, and updates webhook test imports.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • chobbledotcom/tickets#1872: Updates integration test imports to use #test/... module aliases and adds enforcement for relative-import patterns.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: moving related payment and webhook tests into the integration test tree.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch split/payment-test-locations
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch split/payment-test-locations

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@test/integration/server/payments/_shared-setup.ts`:
- Line 9: Update the exported fillSoldOutListing helper with an explicit async
return type, using the repository’s named listing type or an equivalent
Promise<Awaited<ReturnType<typeof createTestListing>>> annotation. Keep the
helper’s existing behavior unchanged.

In `@test/integration/server/webhooks/service-charge-scenario.ts`:
- Line 12: Add a named result interface or type for createServiceChargeScenario
and explicitly annotate the exported function with Promise of that type,
preserving the helper’s existing returned shape.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 807363ea-6f2e-4e09-a36f-e2f73c773d2f

📥 Commits

Reviewing files that changed from the base of the PR and between e6d32a7 and 22452eb.

📒 Files selected for processing (7)
  • test/integration/server/payments-success-tokens.test.ts
  • test/integration/server/payments/_shared-setup.ts
  • test/integration/server/payments/replay.test.ts
  • test/integration/server/payments/success.test.ts
  • test/integration/server/webhooks/modifier-refunds.test.ts
  • test/integration/server/webhooks/modifiers.test.ts
  • test/integration/server/webhooks/service-charge-scenario.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 2

🤖 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 `@test/integration/server/payments/_shared-setup.ts`:
- Line 9: Update the exported fillSoldOutListing helper with an explicit async
return type, using the repository’s named listing type or an equivalent
Promise<Awaited<ReturnType<typeof createTestListing>>> annotation. Keep the
helper’s existing behavior unchanged.

In `@test/integration/server/webhooks/service-charge-scenario.ts`:
- Line 12: Add a named result interface or type for createServiceChargeScenario
and explicitly annotate the exported function with Promise of that type,
preserving the helper’s existing returned shape.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 807363ea-6f2e-4e09-a36f-e2f73c773d2f

📥 Commits

Reviewing files that changed from the base of the PR and between e6d32a7 and 22452eb.

📒 Files selected for processing (7)
  • test/integration/server/payments-success-tokens.test.ts
  • test/integration/server/payments/_shared-setup.ts
  • test/integration/server/payments/replay.test.ts
  • test/integration/server/payments/success.test.ts
  • test/integration/server/webhooks/modifier-refunds.test.ts
  • test/integration/server/webhooks/modifiers.test.ts
  • test/integration/server/webhooks/service-charge-scenario.ts
🛑 Comments failed to post (2)
test/integration/server/payments/_shared-setup.ts (1)

9-9: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Declare the exported helper’s return type.

fillSoldOutListing is exported but its async return type is inferred. Add an explicit named listing type (or equivalent Promise<Awaited<ReturnType<typeof createTestListing>>>) as required by the repository guidelines.

Proposed fix
+type TestListing = Awaited<ReturnType<typeof createTestListing>>;
+
-export const fillSoldOutListing = async () => {
+export const fillSoldOutListing = async (): Promise<TestListing> => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

type TestListing = Awaited<ReturnType<typeof createTestListing>>;

export const fillSoldOutListing = async (): Promise<TestListing> => {
🤖 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 `@test/integration/server/payments/_shared-setup.ts` at line 9, Update the
exported fillSoldOutListing helper with an explicit async return type, using the
repository’s named listing type or an equivalent
Promise<Awaited<ReturnType<typeof createTestListing>>> annotation. Keep the
helper’s existing behavior unchanged.

Source: Coding guidelines

test/integration/server/webhooks/service-charge-scenario.ts (1)

12-12: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add an explicit return type to the exported helper.

createServiceChargeScenario currently relies on inference, but exported functions must declare an explicit return type. Add a named result interface/type and annotate the function as returning Promise<...>.

🤖 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 `@test/integration/server/webhooks/service-charge-scenario.ts` at line 12, Add
a named result interface or type for createServiceChargeScenario and explicitly
annotate the exported function with Promise of that type, preserving the
helper’s existing returned shape.

Source: Coding guidelines

@stefan-burke

Copy link
Copy Markdown
Member Author

Addressed both CodeRabbit findings (the inline comments failed to post as threads, so replying here).

fillSoldOutListing return type (test/integration/server/payments/_shared-setup.ts): Valid — AGENTS.md requires explicit return types on exported functions, and 100+ test helpers already follow this rule. Added type TestListing = Awaited<ReturnType<typeof createTestListing>> and annotated the function Promise<TestListing>, matching the established pattern in test/lib/server-listing-parents/helpers.ts:13. Behavior unchanged.

createServiceChargeScenario return type (test/integration/server/webhooks/service-charge-scenario.ts): Same rule, same fix. Added a ServiceChargeScenario interface ({ listing: TestListing; modifier: ModifierRow }) and annotated the function Promise<ServiceChargeScenario>.

Both fixes are in commit 2d30489. deno task precommit passes.

@stefan-burke
stefan-burke added this pull request to the merge queue Jul 23, 2026
Merged via the queue into main with commit d7e8830 Jul 23, 2026
3 checks passed
@stefan-burke
stefan-burke deleted the split/payment-test-locations branch July 23, 2026 18:41
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.

1 participant