test: split integration suite by resource and tighten SDK/API contract#166
Open
Gabrielpanga wants to merge 1 commit into
Open
test: split integration suite by resource and tighten SDK/API contract#166Gabrielpanga wants to merge 1 commit into
Gabrielpanga wants to merge 1 commit into
Conversation
…tract Restructures tests/integration.test.ts (one 338-line file) into per-resource specs under tests/integration/, modeled on plaid/plaid-node. Each spec that needs server state owns the full lifecycle of its sandbox item: beforeAll creates and waits for UPDATED, afterAll deletes defensively (non-throwing). Plaid skips cleanup; Pluggy items are heavier so we keep it. Helpers (tests/integration/helpers.ts): - createClient / createSandboxItem / deleteItemSafely - retry() for sandbox eventual consistency (mirrors Plaid's getTransactionsWithRetries pattern) - RUN_ID + TEST_TAG so resources created in a single run can be spotted later (clientUserId on connect tokens, webhook URLs, etc.) - captureRejection() because BaseApi rejects with the parsed error body, not a thrown Error with .statusCode — we let each test inspect the body rather than fake a uniform shape Coverage additions vs the monolith: - items.test.ts (fetchItem shape, dates) - bills.test.ts (fetchCreditCardBills / fetchCreditCardBill) - errors.test.ts (4xx for fetchItem / fetchAccount / fetchTransaction / fetchConnector with non-existent ids) - accounts.test.ts now exercises fetchAccountStatements - transactions.test.ts now exercises updateTransactionCategory - connectors.test.ts now exercises validateParameters Coverage retained from the monolith: connectors list/get, accounts, transactions (cursor + by-id + dedup on fetchAllTransactions), investments (+ fetchAllInvestmentTransactions dedup), identity (by item + by id), consents, loans, categories, connect token, webhooks CRUD. Out of scope (next PR): PluggyPaymentsClient (recipients, requests, intents, smart accounts, automatic PIX, scheduled, smart transfers). package.json: - "test" ignores tests/integration (directory) instead of the deleted single file. - "test:integration" matches tests/integration explicitly. Local verification: - pnpm build clean - pnpm test still 11/11 - pnpm test:integration without creds: 13 suites skipped, 31 tests skipped (auto-skip via describeIntegration). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
c990d6a to
efbdc32
Compare
cernadasjuan
approved these changes
May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Restructures the integration test suite from one 338-line
tests/integration.test.tsmonolith into per-resource specs undertests/integration/, modeled onplaid/plaid-node. Per-resource isolation: each spec that needs server state owns the full lifecycle of its own sandbox item (beforeAllcreates and waits forUPDATED,afterAlldeletes defensively).Research note: I originally framed this as a "Stripe pattern" rewrite, but Stripe's open-source SDK doesn't actually run integration tests against the real API — they use
stripe-mock(OpenAPI-driven HTTP fake) and spy-based unit tests. Plaid was the right reference for what was actually being asked for.What's new
Helpers —
tests/integration/helpers.tscreateClient/createSandboxItem/deleteItemSafely.retry()for sandbox eventual consistency (mirrors Plaid'sgetTransactionsWithRetries).RUN_ID+TEST_TAG— every test run is tagged withGITHUB_RUN_ID(CI) orlocal-<timestamp>. Resources that accept free-form identifiers (connect token'sclientUserId, webhook URLs) get tagged so orphans from failed runs are identifiable.captureRejection()—BaseApirejects with the parsed error response body (not a thrownErrorwith.statusCode), so error tests capture the body and let each test inspect it.describeIntegration— auto-skip viadescribe.skipwhenPLUGGY_CLIENT_ID/PLUGGY_CLIENT_SECRETare absent.Coverage additions vs the monolith
items.test.tsfetchItemshape + date instancesbills.test.tsfetchCreditCardBills/fetchCreditCardBill(skips silently when sandbox has noCREDITaccount)errors.test.tsfetchItem,fetchAccount,fetchTransaction,fetchConnectoraccounts.test.tsfetchAccountStatementstransactions.test.tsupdateTransactionCategoryconnectors.test.tsvalidateParametersCoverage retained from the monolith
Connectors list/get, accounts, transactions (cursor + by-id + dedup), investments +
fetchAllInvestmentTransactionsdedup, identity (by item + by id), consents, loans, categories, connect token, webhooks CRUD.Why "no cleanup" (Plaid's choice) wasn't copied
Plaid's sandbox items are cheap; the suite assumes the platform garbage-collects them. Pluggy items are heavier — leaks pile up against the test tenant's quota. So every spec that creates an item registers
afterAllwithdeleteItemSafely(try/catch + stderr on failure so leaks are visible but never mask test failures).Why per-suite items (not shared via
globalSetup)You picked this option explicitly. Trade-off: ~30+ minutes of wall time per run (each suite waits a full sandbox sync) vs. true isolation between suites. Per-suite isolation means one broken test can't poison the next.
Out of scope (next PR)
PluggyPaymentsClient— recipients, requests, intents, payment customers, institutions, bulk, smart accounts, automatic PIX, scheduled, smart transfers. These need their own credentials path and live in a separate domain, so they belong in a focused follow-up.package.jsonscript tweakstest: ignorestests/integration(directory) instead of the deleted single file.test:integration: matchestests/integrationexplicitly.Test plan
pnpm build— clean.pnpm test— 11/11 passing (unit suite unchanged).pnpm test:integrationwithout credentials — 13 suites, 31 tests, all auto-skipped.SDK Integration Testsworkflow run against sandbox is green after merge.🤖 Generated with Claude Code