OUT-3987: L0.3 — scenario factories + DB seeders#121
Open
SandipBajracharya wants to merge 9 commits into
Open
Conversation
Typed factory/seeder library backing the L0.3 scenario fixtures: native sequence counters, params-based variants, and a build()/create() split that maps onto pure factories vs DB seeders. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pure build() factories emitting typed remote-system fixtures for the test harness, no DB access: - sequence.ts: one shared monotonic counter (nextSeq/seqUuid) + resetFactories() so a logical entity carries one deterministic id across Dropbox/Copilot/DB. - dropbox.ts: dropboxEntryFactory (+ folder/deleted .params() variants) → DropboxFileListFolderSingleEntry. - copilot.ts: copilotFileFactory (+ folder/pending/downloadable/renamed variants) and copilotListPage() → CopilotFileRetrieve / CopilotFileList. Typed against the production Zod/TS schemas so fixtures fail to compile on drift. vitest.config.ts include widened to run test/**/*.test.ts in the fast unit project (integration files stay excluded). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Async create() seeders writing real rows through the db singleton against the Testcontainers Postgres harness (integration-only): - dropboxConnectionSeeder, channelSeeder, fileSyncSeeder — each returns the persisted row (InferSelectModel), typed from the Drizzle insert schemas. - Auto-parent wiring: seeding a child with no parent id creates the chain (file -> channel -> connection) with a consistent portalId. channelSeeder reuses an existing connection when only portalId is given, and throws clearly on an unresolvable/ nonexistent parent. - Trait helpers (Partial<FileSeed> spread into create()): pendingCreate/ pendingDelete (respecting the pending_action_target_consistency CHECK), tombstone, synced (Assembly-side), and fromDropboxEntry to keep a seeded row consistent with its Dropbox fixture. - resetFactories() wired into the integration beforeEach alongside the existing TRUNCATE so the shared sequence is deterministic per test. Covered by seeders.integration.test.ts, incl. a round-trip through the real MapFilesService.getDbxMappedFileFromPath read path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds test factories and database seeders for sync scenarios. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "refactor(OUT-3987): centralize channel a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
When both portalId and channelSyncId were supplied, the seeder inserted them without verifying the channel belongs to that portal — the FK only proves the channel exists. That let a test seed a file under portal-A pointing at a portal-B channel: an impossible tenant/channel pairing that production reads and the (portalId, channelSyncId, ...) unique indexes could never produce. Now the channel-provided path always derives portalId from the channel row (its portalId is the ownership source of truth) and, when the caller also passes portalId, throws on a mismatch instead of seeding the drift. Adds a test covering the rejection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The portalId-only path fell back to acc-${n} when the existing connection had
a null accountId (the legitimate pre-OAuth state). That synthetic account
matches neither the connection nor the real Dropbox account OAuth later
records, so webhook/update paths filtered by dbxAccountId would miss the
seeded channel.
Now the channel always adopts the connection's own accountId; if the existing
connection has none yet, it throws (set the connection's accountId or pass
dbxAccountId explicitly) instead of forging a mismatched link. Also drops the
dead acc-${n} fallback on the freshly-created-connection branch (its accountId
is always defaulted). Adds a test for the rejection.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fromDropboxEntry mapped a .tag==='deleted' entry to ObjectType.FILE with a live path and null content hash, seeding a live DB row for an already-deleted remote item. Deleted entries are handled by the deletion/delta flow, so this could hide deletion or resync bugs in a scenario. Now it throws on a deleted entry, pointing the author at tombstone() or the deletion flow to model a gone item. Adds a test for the rejection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
@greptileai rereview |
When both portalId and dbxAccountId were supplied, the channel was inserted
without checking the portal's existing connection. A test could create a
connection for portal-x (accountId acc-real) then seed
channelSeeder.create({ portalId: 'portal-x', dbxAccountId: 'acc-other' }),
producing a channel detached from the real Dropbox account so account-filtered
webhook/update reads would miss it.
Now, when a connection exists for the portal, the explicit dbxAccountId is
validated against it and a mismatch throws. With no connection for the portal
the channel is still inserted as-is (minting nothing), preserving existing
behavior. Adds a test for the rejection.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
@greptileai review again |
channelSeeder's both-supplied check only caught a non-null account mismatch, so a null-account (pre-OAuth) connection let a caller pin a dbxAccountId that may not match the account OAuth later records. Now an existing connection's account must be set and equal the explicit one, else it throws. Also trims verbose comments in the seeders. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace channelSeeder's four inline reconciliation branches with a single resolveChannelAccount(portalId, dbxAccountId?) helper enforcing one rule: a channel's dbxAccountId must equal its portal connection's account, and that account must exist. Behavior unchanged (all channelSeeder tests pass); the account/ownership edge cases are now proven exhaustive in one place. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
@greptileai review whole changes again |
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
1 participant
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.
What & why
L0.3 of the test-harness pipeline (parent OUT-3984). Adds typed fishery factories for the two remote systems and DB seeders for the three sync tables, so tests can declare "the state of both remote systems + the DB" for a scenario, run the real code, and assert on the resulting rows. Test infrastructure only — no production code changes.
What's included
test/factories/sequence.ts) — one deterministic monotonic counter (nextSeq/seqUuid) +resetFactories(), so a logical entity carries one id across its Dropbox / Copilot / DB representations.build(), no DB) —dropboxEntryFactory(file/folder/deleted) andcopilotFileFactory(folder/pending/downloadable/renamed) +copilotListPage(). Typed against the production Zod/TS schemas so they fail to compile on drift.create(), integration-only) —dropboxConnectionSeeder,channelSeeder,fileSyncSeederagainst the Testcontainers Postgres harness. Auto-parent wiring builds thefile → channel → connectionchain with a consistentportalId;channelSeederreuses an existing connection when onlyportalIdis given and throws clearly on an unresolvable parent.pendingCreate/pendingDelete(respect thepending_action_target_consistencyCHECK),tombstone,synced(Assembly-side, composes withfromDropboxEntry), andfromDropboxEntryto keep a seeded row consistent with its Dropbox fixture.resetFactories()in the integrationbeforeEachalongside the existing TRUNCATE.Testing
pnpm test) — factories run container-free.pnpm test:integration) — seeders against real Postgres, incl. a round-trip through the realMapFilesService.getDbxMappedFileFromPathread path.pnpm typecheck+pnpm lintclean.🤖 Generated with Claude Code