mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
533640662d
* perf(tests): move services a-m onto the canonical shared mocks 70 of 129 files in the services `__tests__` a-m slice: 62 converted by scripts/test-perf/codemod-shared-mocks.mjs, 8 finished by hand where it refused one specifier and took another. The hand cases were a hand-rolled auto-vivifying Proxy standing in for dbRead/dbWrite, and a `safeError: (e) => e` identity stub the canonical registration replaces with the real implementation. Neither was asserted on; both were scaffolding to keep the import graph off real infra, which is what the canonical mocks do centrally. Eleven of the converted files carried a hand-written REDIS_KEYS subset that disagreed with the real table and now get the real constant. Every dropped literal appeared exactly once, inside its own factory: no test asserted on one and none used one to select a fixture branch, so the swap is invisible in both directions rather than merely silent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tests): repair two files the shared-mock conversion broke `collection-collaborator.service.test.ts` collected ZERO tests, losing all 51. The codemod lifted a default out of a `vi.hoisted` block body into module scope, but the value it referenced — `OWNER_ID` — was a local of that block, so the file threw at import. It reported as one failed suite with no failing tests, which is the silent class the collected-count diff exists to catch. The file already sets the same default in `beforeEach`, so the surviving copy sits with the other module-scope constants. `creator-program.service.test.ts` asserted on `cp:*` cache keys it had invented in its own factory; the real ones are `packed:caches:creator- program:*`. Three tests went red on the swap. They now name `REDIS_KEYS.CREATOR_PROGRAM.*` so the test cannot re-invent a key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(redis): pin the wire values the services a-m tests re-invented Nine files in that slice carried a hand-written REDIS_KEYS subset whose values disagreed with the real table — `packed:caches:announcements` for a real `packed:caches:announcement`, `kill` for `system:blocks:emergency-kill-list`, `cp:*` for `packed:caches:creator-program:*`. Every one was used only inside the factory that defined it, so no assertion could see the divergence and none of them could go red. A key's wire value addresses live entries written by deployed code, so a rename orphans whatever sits under the old name. Pinning them here makes that a deliberate decision rather than something a test silently follows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(tests): regenerate the direct-mock allowlist after services a-m 345 -> 275 canonical files; the guard passes in both directions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(tests): split the single-client db aliases in services a-m Eleven files mocked `~/server/db/client` with one local object serving both `dbRead` and `dbWrite`, so a write could satisfy a read assertion. Each is bound to the one client the module under test actually spells for every call it makes, checked against the service source rather than guessed from the local's name — `mockDbRead` in two of the block-registry files turned out to be right for the wrong reason, and `mockDb` in `account-deletion-images` was serving writes. Two things the split surfaced, both kept rather than papered over: `get-engaged-models-by-ids` carried a real in-memory fixture on `resourceReview.findMany`, not a restated default, so binding the client alone dropped four tests' data and they went red. The fixture is now an explicit `mockImplementation` on the canonical node, and the four tests are the evidence it is load-bearing. `account-deletion-images` asserted on `'pending-restores'`, a key it had invented; the real one is `system:pending-image-restores`. It now names `REDIS_SYS_KEYS.SYSTEM.PENDING_IMAGE_RESTORES`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(tests): regenerate the allowlist after the alias split 275 -> 264 canonical files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(tests): migrate 18 more services a-m tests onto the canonical mocks Seven bound automatically where a balanced behaviour check cleared them, nine converted by hand where it refused, and two whose single aliased local was split by routing each call site to the client the service actually spells for it. Three cases the run settled rather than the diff: `minor-hash` wired two separately-declared spies in as client leaves, so deleting the client literal left both alive, armed by `beforeEach`, and connected to nothing — the primary-vs-replica re-read then saw the canonical `null` default. Behaviour-free is not the same as safe to delete; both names now point at the canonical nodes. `article-` and `bounty-locked-properties` spread their transaction object into `dbWrite`, so the canonical `$transaction` default preserves the identity they relied on. `model-appeal` did NOT — its transaction client was a separate object, and inheriting the default would let an in-transaction write satisfy an assertion that means "written outside the transaction". It keeps its own `tx`. `model-version.{donation-goals-cache,idempotent}` each carried a second direct mock of `~/server/redis/client` alongside the db one; both are taken, so neither file is left half-converted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(tests): regenerate the allowlist after the second services a-m batch Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test-perf: add the client-local binder, with the refusal a run taught it Binds a test's own client locals to the canonical mock nodes for the shape the codemod refuses as "hoisted entry is not a bare vi.fn()". It refuses two things rather than guessing. A leaf carrying behaviour a canonical default does not cover, extracted with balanced parens — a lazy `vi.fn\([^;]*?\)` stops inside an arrow's own parameter list and hands the check a truncated call that reads as behaviour-free, which is how five files passed a check written to stop them. And a leaf that is a bare identifier declared elsewhere. The client literal is the only thing wiring such a spy to the module, so deleting it leaves the spy alive, still armed by `beforeEach`, connected to nothing, while the code under test reads the canonical default instead. That cost two tests in minor-hash.service.test.ts and no static check caught it — behaviour-free is not the same as safe to delete. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(test-perf): per-case analysis of the parameterised-client alias split Six files in the services a-m slice mock one local as both db clients against services that choose their client at runtime, so the usual "grep the source for dbRead.model.method" routing finds nothing. Corrects my earlier report: these are NOT permanent hand-work. No test in the six passes a `db` option, so every case falls to its entry point's default, and those are fixed and listed. Four call sites default to dbWrite and one inverts to dbRead, which is the trap. Records the 14 negative assertions individually, because routing one to the client the code never touches makes it pass trivially — the failure a run cannot show — and names the one path I could not resolve rather than guessing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(tests): migrate two more services a-m tests, and harden the binder `model-version-count` named its spy with ES6 shorthand (`{ groupBy }`), which the binder read as an empty object and would have orphaned; bound by hand. `cover-image.service.logging` declares its `logToAxiom` rejection on the canonical node — the rejection IS the fixture there, since the tests exist to prove the `.catch()` on the best-effort log calls, so it is moved rather than dropped. Three refusals added to the binder, each after catching it wrong on a real file: - one local bound to both clients is an alias, and binding it here would silently pick whichever came last; - ES6 shorthand entries, per the orphaning above; - a factory with a BLOCK body, which opened on the block's brace, read an empty object, found no exports to object to, and deleted the whole factory with every local it named. It cleared two files that way and nothing caught it but reading the diff. It now auto-converts none of the 23 remaining files in this slice, which is the honest number: what is left is hand work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(tests): regenerate the allowlist Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(tests): migrate six more services a-m tests onto the canonical mocks All six by hand; the binder refuses every remaining file in this slice. `commentsv2-owner` wired ONE `findUnique` spy into twelve tables, so a case naming `entityType: 'challenge'` could be satisfied by any of them. Each entity type now arms its own table, and the two "no lookup happened" assertions check all twelve rather than the single shared spy — stronger than what they replaced. Four of the twelve have a case; the other eight were in the fixture because the service can read them, not because anything checks that it does. `leaderboard-rank-showcase` records every statement it issues into an array that every assertion reads. The canonical `$executeRaw` returns 0 and records nothing, so the recorder is declared rather than inherited — dropping it would empty the array and throw, which is the loud case. `challenge-results-notification` and the two `contest-entry-*` gates were aliases hiding nothing: every path they assert on is spelled `dbRead` in the service. `bust-caches-for-posts-empty` replaces a permissive Proxy whose `apply` trap answered every un-stubbed call with `[]`; the two assertions in it are on `$queryRaw`, whose canonical default is also `[]`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(tests): regenerate the allowlist after batch 4 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(test-perf): add lag-selected clients as a second unresolvable mechanism `getDbWithoutLag` picks dbRead or dbWrite from runtime replication state, so anything reached through it has no fixed client in the source. Five more files in the slice route reads that way. Under test it does not even behave as production does: REPLICATION_LAG_DELAY is a zod `.default(0)` key absent from TEST_ENV_DEFAULTS, so the canonical env reads it undefined, and `undefined <= 0` is false where `0 <= 0` is true — the staleness branch production never takes runs in every test that reaches it without stubbing db-lag-helpers. 73 defaulted keys, 59 absent from the table, 40 of those numeric or boolean. Also records why the already-converted files were safe: they happen to mock db-lag-helpers, which pins the client. The safe and unsafe conversions are separated by that coincidence, so the population at risk cannot be read off which files converted cleanly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(test-perf): the lag-selected bucket is two files, not five Corrects the section I added an hour ago. Three of the five resolve by the entry point the test imports: deleteVersionById uses dbWrite only, and neither model-file service mentions getDbWithoutLag at all. Only earlyAccessPurchase (via getVersionById) and publishModelVersionById genuinely defer the choice to runtime. The mistake is the one the section is about — reading BOTH off a whole-module scan and treating it as a property of the file. A module containing both spellings says nothing; the entry point the test calls is what decides. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(test-perf): handover for the services a-m mock migration The state a successor cannot recover from the diff: the four remaining buckets with the discriminator for each rather than just their names, the binder's five refusals with the case that taught each one, and what to check on the seam-blocked files after that change lands — they will look convertible the moment it does. Also splits the post-tooling-scare audit into three categories rather than two: claims re-verified explicitly, claims structurally immune because they came from node rather than a shell pipe (an accident of format, not a method), and claims not re-derived at all, named individually. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(tests): migrate four more services a-m tests onto the canonical mocks `engagement-toggle.idempotent` was an alias hiding a real read/write split: `toggleUserBountyEngagement` reads bountyEngagement on dbRead and writes it on dbWrite, through one spy, so a case asserting the read could have been satisfied by a write. Routed per call site from the two entry points the file imports — `toggleModelEngagement` does every modelEngagement operation on dbWrite, and the dbRead spelling elsewhere in user.service belongs to a function this test never calls. `block-registry.slot-reservation` keeps its distinct read/write locals, so the parameterised-client question does not arise. Its `redis.scanIterator` is an empty async generator consumed with `for await`; the canonical node would vivify it as a spy returning undefined, which throws rather than iterating, so the generator is declared explicitly. `file-download-lookup` carried a hand-written `safeError` under a comment claiming the global setup does not provide one. It does — the canonical registration spreads the real module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(tests): regenerate the allowlist after batch 5 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(test-perf): correct the handover after batch 5, and add two mock-system constraints 111 of 129, not the 112 the placeholder claimed. The ordinary bucket grew from 3 to 7 because three files moved back out of "unresolvable" when I re-checked them; the seam-blocked three are now unblocked and are the easiest remaining work, with the check that is not "do they pass". Adds two constraints that belong to the mock system rather than to this slice: a canonical mock cannot statically import anything reading mocked env at module scope, because setup.ts loads it earlier than every hoisted factory it registers — and the failure is zero tests collected reported as one failed suite. And ~25 typecheck errors on this base are phantom, with the tell being that they sit in files you did not touch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(tests): migrate the three withSysReadDeadline seam tests onto the canonical mocks These three were blocked until `withSysReadDeadline` became a seam node on the canonical redis mock (17f994221e). Before that, setup.ts spread the real implementation into the factory, so converting them would have deleted the only lever they have: each one injects a sysRedis read timeout by replacing that export, and the deadline is a wall-clock race a mocked client can never lose. The check afterwards is not that they pass. Each file already asserts the seam was called once per case, and after conversion that spy is the node setup.ts hands the service, so "the injected implementation is reached" is asserted four times per file rather than inferred. What that does not establish is that the timeout leg can fail at all, because each SLOW case is fail-on-revert by hang - and a hang is the one failure a runner cannot report. Probed by resolving the seam instead of rejecting it, which fails on a value in milliseconds: content-markdown returns instead of throwing, and collection returns the cached seed rather than the locally-computed hourly one. daily-challenge is not probed; its fail-open collapses a resolved non-JSON string to null too, so the probe could not distinguish the branches there. content-markdown's REDIS_SYS_KEYS copy had drifted - 'content:region-warning' against the real 'system:content:region-warning'. Its only use is as hGet's first argument, and nothing in the file asserts or branches on it, so the swap changes no behaviour. That is the finding rather than a footnote: the file was never asserting the key. daily-challenge's CUSTOM_CHALLENGE literal looked like the same drift and is byte-identical, so it is recorded as checked and clean. collection's fixture answered every REDIS_KEYS / REDIS_SYS_KEYS / REDIS_SUB_KEYS path through a proxy returning 'k', for the whole of collection.service's import graph rather than just the module under test - keys that could not disagree with themselves. It now gets the real tables. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpwac9RLBE6uxU11W1BAGA * docs(test-perf): record batch 6, and four instrument findings behind it Bucket 3 is done and the burn-down is 114 of 129. The denominator is re-derived off the allowlist at the branch base rather than inherited, which turns up the number the scoreboard is missing: 41 of the 170 files in this range never carried a canonical mock at all, so measuring progress by absence reads 155/170 and 91% against a true 114/129. Third instance of that shape and all three inflate the same way. The seam files needed a probe establishing their timeout leg can go red at all, and the obvious probe is wrong: a pass-through leaves the never-settling read awaited directly, so the test rides to the 60s timeout rather than failing. Resolving the seam instead fails on a value in 7ms and 9ms, and surgically - one test per file. daily-challenge is deliberately not probed because its fail-open collapses a resolved non-JSON string to null too. A vi.mock of db-lag-helpers is not the pin it looks like: purge-by-hash and deregister both carry one and neither is pinned, because both override only preventModelVersionLag through importOriginal. The discriminator is whether the mock replaces getDbWithoutLag. And two instruments that mislead. residual-mocks.mjs only matches `~/` spellings, so the relative-path hole closed for the codemod and guard at92f1728652is still open in the script the recipe tells everyone to run. vitest's numTotalTestSuites counts file-level AND describe-level suites, so reconciling a per-file count against it fires on a healthy run - which is how a correct instrument gets discarded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpwac9RLBE6uxU11W1BAGA * perf(tests): migrate six services a-m tests onto the canonical mocks Each was resolved by the entry point the test imports rather than by scanning the module, and three of the six turned on a distinction that is invisible in the diff. commentsv2.appListing splits per CASE, not per path: thread.findUnique is driven by four entry points that disagree, dbRead for getCommentCount (:433) and getCommentsInfinite (:741), dbWrite for upsertComment (:267) and toggleLockCommentsThread (:471); togglePinComment reads commentV2 on dbRead (:505) and writes it on dbWrite (:508) inside one case. model-file.service's local was named mockDbRead and served writes. count (:599), findFirst (:617) and findMany (:36) are dbRead; findUnique and update belong to markFileReplaced and restoreReplacedFile and are dbWrite. Its comment claiming these helpers only touch dbRead was true of the two functions it was written for and false of the file, so it is gone rather than preserved. $transaction was decided per file, not by pattern. coinbase and deregister inherit the canonical default because their tx and write client were already one object - coinbase's tx member IS mockDbWrite.redeemableCode, and the service touches only tx.redeemableCode.create (:248). Both commentsv2 files and model-early-access-refund keep a local tx, because their assertions are on tx spies and mean "written inside the transaction"; inheriting would collapse that into the direct calls and the assertions would pass for the wrong reason. Removing model-early-access-refund's wholesale logging mock left mockLogToAxiom declared, armed and wired to nothing - a spy no assertion referenced, in a file that would have stayed green. Deleted rather than re-pointed. One assertion is left as it is and flagged rather than fixed: entityAccess.delete carries not.toHaveBeenCalled() and appears on neither client in either entry point, so it cannot fail whichever way it is routed. Making it meaningful is a test change, not a migration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpwac9RLBE6uxU11W1BAGA * perf(tests): migrate model-file-scan onto the canonical mocks One local served both clients across 1,439 lines, and a single line decides it: everything the file drives is dbWrite - modelFile.findUnique/update (model-file-scan.service:132, :161, :203), modelFileHash.* (:171, :213, :214), modelVersion.findUnique (:339, :686), modelFile.updateMany (:670) - except rescanModel's modelFile.findMany at :616, which is dbRead. Nine call sites, scoped to that describe block by line range rather than replaced globally. Reverting that one decision - the naive all-dbWrite conversion - reds five tests in the rescanModel block while collected stays 66, so the routing is load-bearing and a routing failure is distinguishable from a collection failure in the output. It is visible only because every assertion downstream of findMany is positive; the identical mistake under a negative assertion is silent. $transaction here is the ARRAY branch (:212), which the canonical default handles separately from the callback branch the recipe documents. Every test that reaches it sets mockResolvedValue([]) explicitly, so the default is overridden and the array's elements are still evaluated as it is built. logToAxiom and sysRedis.setNxKeepTtlWithEx are re-pointed at the canonical nodes rather than deleted: both are genuinely asserted, at three sites each. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpwac9RLBE6uxU11W1BAGA * perf(tests): migrate the six parameterised-client tests onto the canonical mocks This is the bucket where the service takes its client as a parameter, so which client a case exercises is a fact about the test's call rather than a spelling in the source. None of the six passes a `db` option, so each falls to its entry point's default. The routing table in services-a-m-parameterised-client-analysis.md is wrong about one line, and the paired run caught it. Inside resolveBlockInstance the parameterised local at block-registry.service:1362 serves EVERYTHING on that path - blockUserSubscription.findUnique (:1409), model.findUnique (:1451) and modelVersion.findFirst (:1461) - so all three are dbWrite. The dbRead spelling the analysis cites for model.findUnique is at :2617, in a function these tests never call. Routing model/modelVersion to dbRead reddened six tests across pinned-version and resolve-instance, every one "expected null not to be null", because the code read the canonical null on a client it never uses and returned early. Fifteen sites corrected. appCollaborator.findMany is dbRead, and it is spelled nowhere in appBlockReview.service: getAppInsiderUserIds (:74-77) reaches it through a deferred `await import()` of app-access.service, where the call is at :1111. That closes the one assertion the analysis left explicitly unresolved. appBlockReview.findUnique is NOT on both clients for these entry points either - the dbRead spelling at :264 belongs to getMyAppBlockReview, which the test does not import. spend-cap-config's beforeEach sits above both describes, and the two read appBlock.findUnique through different clients, so it arms both nodes. Arming one would leave the other describe on the canonical null and take the NOT_FOUND branch throughout - green in one block, wrong in the other. Both resetAll loops are replaced by vi.clearAllMocks(). Object.values() over a canonical node enumerates nothing: it is a Proxy over a vi.fn with no ownKeys trap, so the vivified children are never own properties. The loops would have reset nothing, and only a negative assertion downstream would have shown it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpwac9RLBE6uxU11W1BAGA * docs(test-perf): close out the services a-m slice at 127 of 129 Records batches 6-9 with their per-file results, and the two findings that cost the most to learn. The red: batch 9 routed model.findUnique to dbRead on the strength of a line in the parameterised-client analysis, and six tests went red because inside resolveBlockInstance that call goes through the parameterised local at block-registry.service:1362, not through the dbRead spelling at :2617 which belongs to another function. Fourth BOTH-off-a-whole-module-scan instance in this slice and the first to reach a routing table rather than a bucket assignment. A citation was present, it looked checked, and the cited line was not on the path. The probe: a mis-routed negative is silent, demonstrated with the control that makes the demonstration mean something. Mis-routing a dangerous negative passes AND mis-routing a safe one passes identically, so only flipping it to a positive discriminates. And the canonical nodes carry their dotted path as a mockName, so a mis-route now names the client it landed on where the old fixtures read `expected "spy"`. Kept as a log rather than rewritten to current state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpwac9RLBE6uxU11W1BAGA * docs(test-perf): record where the slice landed and what is not established Branch, SHA and PR, plus the three facts a successor cannot re-derive: the stale remote branch diverges and is not a fast-forward, two files appear on both the PR branch and perf/test-mock-system deliberately, and storage-resolver.deregisterByFile stops where it does on purpose. States plainly that nothing here has been adversarially reviewed and should not reach main on the author's own verification. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpwac9RLBE6uxU11W1BAGA * docs(test-perf): flag the stacked-PR retarget so it cannot be forgotten #3973's base is #3959's head, which is the shape CLAUDE.md forbids: a squash-merged parent does not retarget the child, so the child lands on an orphaned branch and its changes go missing. The plan already avoids it by rebasing onto main afterwards - this records that the retarget is the load-bearing step and states the check that catches it having gone wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpwac9RLBE6uxU11W1BAGA * docs(test-perf): narrow the storage-resolver refusal — not lifted, not permanent "Do not finish it" overclaimed. The canonical env proxy DOES honour post-hoc assignment (env.mock.ts:79 writes into overrides, :45 reads overrides first, with defineProperty and deleteProperty traps landing in the same map), which was the one link in the argument the author had not verified. It does not rescue a lift - the proxy honours assignment to `env`, not to a different object that used to be the mock - so the file still must not be lifted. But it does mean the file is convertible by rewrite: move the mutations into setEnv or a direct env.X assignment inside the case that needs them. A refusal to do it the cheap way is not a claim that it cannot be done. Recording it as permanently refused would be a claim nobody has earned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpwac9RLBE6uxU11W1BAGA * docs(test-perf): fold in the reviewer's mechanism, and label my unmeasured claim The shielding hazard cannot re-enter this file at all: logToAxiom is a canonical hybrid node whose identity is cached in a module-level Map for the worker's life (hybrid.ts:30, :48-95), so the binding is correct whether or not the consuming module re-instantiates. That makes converting logging while keeping env the right split rather than a compromise - the canonical mock removes the hazard instead of dodging it. And the reviewer was right to label one of my claims: that a per-file env mock FORCES re-instantiation under isolate:false is reasoned, not measured. Nothing here depends on it. Recorded as reasoned so nobody quotes it as measured. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpwac9RLBE6uxU11W1BAGA * style: Prettier-format the three files this branch adds CI's lint gate checks ADDED files, and all three had been failing it for as long as they existed - the same thing that blocked #3959 for its whole life with six of them. Caught by running the gate rather than by waiting for CI. No content change: formatting only, on files this branch introduces, so the diff against main is the whole file either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpwac9RLBE6uxU11W1BAGA --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
381 lines
14 KiB
JavaScript
381 lines
14 KiB
JavaScript
import fs from 'node:fs';
|
|
|
|
/**
|
|
* Bind a test's own client locals to the canonical mock nodes.
|
|
*
|
|
* Handles the shape the codemod refuses as "hoisted entry is not a bare vi.fn()": a factory
|
|
* whose exports are client-shaped objects of typed spies. Each spy becomes the canonical node
|
|
* at the same path, so `mockDbRead.image.findFirst` keeps working as a name.
|
|
*
|
|
* REFUSES rather than guesses when a leaf carries behaviour that is not a canonical default —
|
|
* deleting one of those is silent, and it cost four tests in get-engaged-models-by-ids.
|
|
*
|
|
* node .test-perf/bind-locals.mjs <list-file> [--write]
|
|
*/
|
|
const WRITE = process.argv.includes('--write');
|
|
const files = fs.readFileSync(process.argv[2], 'utf8').trim().split('\n');
|
|
|
|
const ROOTS = {
|
|
'~/server/db/client': { dbRead: 'dbMock.dbRead', dbWrite: 'dbMock.dbWrite' },
|
|
'~/server/redis/client': { redis: 'redisMock.redis', sysRedis: 'redisMock.sysRedis' },
|
|
'~/server/logging/client': { logToAxiom: 'loggingMock.logToAxiom' },
|
|
};
|
|
const IMPORTS = {
|
|
dbMock: "import { dbMock } from '~/__tests__/mocks/db.mock';",
|
|
redisMock: "import { redisMock } from '~/__tests__/mocks/redis.mock';",
|
|
loggingMock: "import { loggingMock } from '~/__tests__/mocks/logging.mock';",
|
|
};
|
|
const CONSTANT_KEYS = /^REDIS_(SYS_|SUB_)?KEYS$/;
|
|
const DEFAULT_INNER = /^(?:null|\[\]|0|undefined|false|Promise\.resolve\((?:null|\[\]|0)\))$/;
|
|
|
|
const close = (src, open) => {
|
|
const pair = { '(': ')', '{': '}', '[': ']' };
|
|
const stack = [];
|
|
for (let i = open; i < src.length; i++) {
|
|
const c = src[i];
|
|
if (pair[c]) stack.push(pair[c]);
|
|
else if (c === stack[stack.length - 1]) {
|
|
stack.pop();
|
|
if (!stack.length) return i;
|
|
}
|
|
}
|
|
return -1;
|
|
};
|
|
|
|
// top-level `key: value` pairs of an object literal starting at `open`
|
|
function entries(src, open) {
|
|
const end = close(src, open);
|
|
const out = [];
|
|
let depth = 0;
|
|
for (let i = open; i < end; i++) {
|
|
const c = src[i];
|
|
if ('({['.includes(c)) depth++;
|
|
else if (')}]'.includes(c)) depth--;
|
|
else if (depth === 1) {
|
|
// ES6 shorthand: `{ groupBy }` is `groupBy: groupBy`, and reading it as "no entries" is
|
|
// how a spy declared elsewhere gets silently orphaned (mark hit the same shape in a
|
|
// classifier). Rewrite it to the long form before the key/value scan.
|
|
const sh = /^([A-Za-z_$][\w$]*)\s*(?=[,}])/.exec(src.slice(i));
|
|
if (
|
|
sh &&
|
|
/[{,\s]/.test(src[i - 1] ?? '') &&
|
|
!/^(async|await|return|new|typeof)$/.test(sh[1])
|
|
) {
|
|
out.push({ key: sh[1], value: sh[1], start: i, end: i + sh[0].length });
|
|
i += sh[0].length - 1;
|
|
continue;
|
|
}
|
|
const m = /^([A-Za-z_$][\w$]*)\s*:\s*/.exec(src.slice(i));
|
|
if (m && /[{,\s]/.test(src[i - 1] ?? '')) {
|
|
const vs = i + m[0].length;
|
|
let ve;
|
|
if ('({['.includes(src[vs])) ve = close(src, vs) + 1;
|
|
else {
|
|
ve = vs;
|
|
while (ve < end && !',\n'.includes(src[ve])) ve++;
|
|
}
|
|
out.push({ key: m[1], value: src.slice(vs, ve).trim(), start: i, end: ve });
|
|
i = ve;
|
|
}
|
|
}
|
|
}
|
|
return out;
|
|
}
|
|
|
|
const isSpy = (v) => /^vi\.fn\(/.test(v);
|
|
function spyIsDefault(v) {
|
|
// `vi.fn().mockResolvedValue(undefined)` is the canonical logToAxiom default spelled long-hand.
|
|
const chained =
|
|
/^vi\.fn\(\)\.mock(?:ResolvedValue|ReturnValue)\((undefined|null|\[\]|0)?\)$/.exec(v.trim());
|
|
if (chained) return true;
|
|
const inner = v
|
|
.replace(/^vi\.fn\(/, '')
|
|
.replace(/\)$/, '')
|
|
.trim();
|
|
if (!inner) return true;
|
|
const body = inner.replace(/^(?:async\s*)?\([^)]*\)\s*(?::[^=]*)?=>\s*/, '').trim();
|
|
return DEFAULT_INNER.test(body);
|
|
}
|
|
|
|
let ok = 0,
|
|
refused = 0;
|
|
for (const file of files) {
|
|
let src = fs.readFileSync(file, 'utf8');
|
|
const bindings = [];
|
|
const needImports = new Set();
|
|
const cuts = [];
|
|
const drift = [];
|
|
const preserve = [];
|
|
const hoistedExports = new Set(
|
|
[...src.matchAll(/const \{([^}]*)\} = vi\.hoisted\(/g)]
|
|
.flatMap((m) => m[1].split(',').map((s) => s.trim()))
|
|
.filter(Boolean)
|
|
);
|
|
let bad = null;
|
|
|
|
for (const [spec, roots] of Object.entries(ROOTS)) {
|
|
const at = src.indexOf(`vi.mock('${spec}'`);
|
|
if (at === -1) continue;
|
|
const callOpen = src.indexOf('(', at);
|
|
const callEnd = close(src, callOpen);
|
|
// 🔴 Only a CONCISE-body factory — `() => ({ … })` — is parsed here. A block body
|
|
// (`() => { … return { … } }`) opens on the block's brace, so the entry scan reads an empty
|
|
// object, finds no exports to check, and the whole factory gets deleted with every local it
|
|
// named left orphaned. That is a wholesale silent delete, so it is a refusal: two files were
|
|
// cleared this way and only caught by reading the diff.
|
|
const arrow = src.indexOf('=>', callOpen);
|
|
const afterArrow = src.slice(arrow + 2, callEnd).replace(/^\s*/, '');
|
|
if (!afterArrow.startsWith('({')) {
|
|
bad = `${spec}: factory has a block body, not \`() => ({ … })\``;
|
|
break;
|
|
}
|
|
const objOpen = src.indexOf('{', arrow + 2);
|
|
if (objOpen === -1 || objOpen > callEnd) {
|
|
bad = `${spec}: factory is not an object literal`;
|
|
break;
|
|
}
|
|
|
|
for (const e of entries(src, objOpen)) {
|
|
if (CONSTANT_KEYS.test(e.key)) {
|
|
drift.push(`${spec}.${e.key}`);
|
|
continue;
|
|
}
|
|
const target = roots[e.key];
|
|
if (!target) {
|
|
bad = `${spec}: factory declares "${e.key}", which the canonical mock does not own`;
|
|
break;
|
|
}
|
|
needImports.add(target.split('.')[0]);
|
|
|
|
if (/^[A-Za-z_$][\w$]*$/.test(e.value)) {
|
|
bindings.push([e.value, target]);
|
|
} else if (e.value.startsWith('{')) {
|
|
// an inline client literal: bind each leaf spy at its path
|
|
const walk = (open, path) => {
|
|
for (const leaf of entries(src, open)) {
|
|
if (isSpy(leaf.value)) {
|
|
if (!spyIsDefault(leaf.value)) {
|
|
bad = `${spec}.${[...path, leaf.key].join('.')} carries behaviour`;
|
|
return;
|
|
}
|
|
} else if (/^[A-Za-z_$][\w$]*$/.test(leaf.value)) {
|
|
bindings.push([leaf.value, `${target}.${[...path, leaf.key].join('.')}`]);
|
|
} else if (leaf.value.startsWith('{')) {
|
|
walk(src.indexOf('{', leaf.start + leaf.key.length), [...path, leaf.key]);
|
|
} else {
|
|
bad = `${spec}.${[...path, leaf.key].join('.')} is not a spy, a local or an object`;
|
|
return;
|
|
}
|
|
}
|
|
};
|
|
walk(src.indexOf('{', e.start + e.key.length), []);
|
|
if (bad) break;
|
|
} else if (isSpy(e.value)) {
|
|
if (!spyIsDefault(e.value)) {
|
|
bad = `${spec}.${e.key} carries behaviour`;
|
|
break;
|
|
}
|
|
} else {
|
|
bad = `${spec}.${e.key} is neither a local nor a literal`;
|
|
break;
|
|
}
|
|
}
|
|
if (bad) break;
|
|
cuts.push([at, callEnd + (src[callEnd + 1] === ';' ? 2 : 1)]);
|
|
}
|
|
|
|
if (bad) {
|
|
console.log('REFUSED', file.replace('src/server/services/__tests__/', '').padEnd(52), bad);
|
|
refused++;
|
|
continue;
|
|
}
|
|
if (!cuts.length) {
|
|
console.log(
|
|
'SKIP ',
|
|
file.replace('src/server/services/__tests__/', ''),
|
|
'no canonical vi.mock'
|
|
);
|
|
continue;
|
|
}
|
|
|
|
// 🔴 One local serving two clients is an ALIAS, and binding it here would silently pick
|
|
// whichever came last — the collision the split exists to surface. Which client the code
|
|
// actually uses is a question for the module under test (or, where the service takes its
|
|
// client as a parameter, for each test); either way it is not this tool's call.
|
|
const byLocal = new Map();
|
|
for (const [l, t] of bindings) {
|
|
if (byLocal.has(l) && byLocal.get(l) !== t) {
|
|
bad = `"${l}" is bound to both ${byLocal.get(l)} and ${t} — an alias, split it by call site`;
|
|
break;
|
|
}
|
|
byLocal.set(l, t);
|
|
}
|
|
if (bad) {
|
|
console.log('REFUSED', file.replace('src/server/services/__tests__/', '').padEnd(52), bad);
|
|
refused++;
|
|
continue;
|
|
}
|
|
|
|
// every local we bind must have a behaviour-free declaration, which we then remove
|
|
const removals = [];
|
|
for (const [local] of bindings) {
|
|
// `mockX: { … }` / `mockX: vi.fn(…)` in a hoisted object, or a module-scope const of either.
|
|
const hoisted = new RegExp(`\\n\\s*${local}:\\s*(?:vi\\.fn)?[{(]`).exec(src);
|
|
const constDecl = new RegExp(
|
|
`\\n(?:const|let)\\s+${local}(?::[^=]+)?\\s*=\\s*(?:vi\\.fn)?[{(]`
|
|
).exec(src);
|
|
const hit = hoisted ?? constDecl;
|
|
if (!hit) {
|
|
bad = `no declaration found for ${local}`;
|
|
break;
|
|
}
|
|
const braceAt = hit.index + hit[0].length - 1;
|
|
let end = close(src, braceAt);
|
|
// a chained `.mockResolvedValue(…)` belongs to the declaration too
|
|
const chain = /^\.mock\w+\(/.exec(src.slice(end + 1));
|
|
if (chain) end = close(src, end + chain[0].length);
|
|
if (end === -1) {
|
|
bad = `unbalanced declaration for ${local}`;
|
|
break;
|
|
}
|
|
// Balanced extraction: a lazy `[^;]*?` stops inside an arrow body's own parens and hands
|
|
// spyIsDefault a truncated call, which then reads as a bare default. That is how
|
|
// `vi.fn(async (cb) => cb(tx))` slipped through as behaviour-free.
|
|
const decl = src.slice(hit.index, end + 1);
|
|
|
|
// 🔴 A leaf that is a bare IDENTIFIER declared elsewhere is a refusal, not a clear. The
|
|
// client literal is the only thing wiring such a spy to the module; delete the literal and
|
|
// the spy stays alive, still armed by `beforeEach`, connected to nothing — and the code
|
|
// under test silently reads the canonical default instead. Behaviour-free is not the same
|
|
// as safe to delete. (Cost 2 tests in minor-hash.service.test.ts, caught by a run and not
|
|
// by this check, which only asked whether the leaves carried behaviour.)
|
|
for (const m of decl.matchAll(/([\w$]+)\s*:\s*([A-Za-z_$][\w$]*)\s*[,}]/g)) {
|
|
const [, key, ident] = m;
|
|
if (/^(vi|undefined|null|true|false)$/.test(ident)) continue;
|
|
if (new RegExp(`(?:const|let)\\s+${ident}\\b`).test(src) || src.includes(`${ident}:`)) {
|
|
bad = `${local}.${key} is the identifier "${ident}", declared elsewhere — bind it to the canonical node by hand`;
|
|
break;
|
|
}
|
|
}
|
|
if (bad) break;
|
|
// Behaviour is PRESERVED, not dropped: each non-default spy becomes an explicit call on the
|
|
// canonical node at the same path. Refuse only when the body names something that will not
|
|
// exist at module scope after the factory is gone.
|
|
const target = bindings.find(([l]) => l === local)[1];
|
|
for (let k = decl.indexOf('vi.fn('); k !== -1; k = decl.indexOf('vi.fn(', k + 1)) {
|
|
const callEnd = close(decl, k + 'vi.fn'.length);
|
|
if (callEnd === -1) continue;
|
|
let full = decl.slice(k, callEnd + 1);
|
|
let stop = callEnd;
|
|
const chained = /^\.mock\w+\(/.exec(decl.slice(callEnd + 1));
|
|
if (chained) {
|
|
stop = close(decl, callEnd + chained[0].length);
|
|
full = decl.slice(k, stop + 1);
|
|
}
|
|
if (spyIsDefault(full)) continue;
|
|
|
|
// the property path this spy sits at, read backwards from the call
|
|
const before = decl.slice(0, k);
|
|
const path = [];
|
|
let tail = before;
|
|
for (;;) {
|
|
const m = /([A-Za-z_$][\w$]*)\s*:\s*(?:\{[\s\S]*)?$/.exec(tail);
|
|
if (!m) break;
|
|
path.unshift(m[1]);
|
|
tail = tail.slice(0, m.index);
|
|
const opens = (tail.match(/\{/g) ?? []).length - (tail.match(/\}/g) ?? []).length;
|
|
if (opens <= 1) break;
|
|
}
|
|
if (!path.length || path[0] === local) path.shift();
|
|
if (!path.length) {
|
|
bad = `${local}: cannot locate the path of ${full.slice(0, 40)}`;
|
|
break;
|
|
}
|
|
|
|
const free = [
|
|
...new Set([...full.matchAll(/\b([A-Za-z_$][\w$]*)\b/g)].map((m) => m[1])),
|
|
].filter(
|
|
(n) =>
|
|
!/^(vi|fn|async|await|return|const|let|new|typeof|unknown|any|void|Promise|null|undefined|true|false|number|string|boolean|mock\w*)$/.test(
|
|
n
|
|
)
|
|
);
|
|
const unresolved = free.filter(
|
|
(n) =>
|
|
!new RegExp(`(?:const|let)\\s+${n}\\b|\\b${n}\\s*[,}]`).test(src.slice(0, hit.index)) &&
|
|
!hoistedExports.has(n)
|
|
);
|
|
if (unresolved.length) {
|
|
bad = `${local}.${path.join('.')} names ${unresolved.join(', ')} which the factory scoped`;
|
|
break;
|
|
}
|
|
|
|
preserve.push(
|
|
`${target}.${path.join('.')}${full
|
|
.slice('vi.fn'.length)
|
|
.replace(/^\(/, '.mockImplementation(')
|
|
.replace(/^\.mockImplementation\(\)/, '')};`
|
|
);
|
|
}
|
|
if (bad) break;
|
|
const tail = src.slice(end + 1).match(/^[,;]?\n/)?.[0] ?? '';
|
|
removals.push([hit.index + 1, end + 1 + tail.length]);
|
|
}
|
|
if (bad) {
|
|
console.log('REFUSED', file.replace('src/server/services/__tests__/', '').padEnd(52), bad);
|
|
refused++;
|
|
continue;
|
|
}
|
|
|
|
for (const [a, b] of [...cuts, ...removals].sort((x, y) => y[0] - x[0]))
|
|
src = src.slice(0, a) + src.slice(b);
|
|
// an emptied destructuring is dropped; a partly-emptied one keeps its remaining names
|
|
for (const [local] of bindings)
|
|
src = src.replace(new RegExp(`(const \\{[^}]*?)\\b${local}\\b,?\\s*`), '$1');
|
|
// An emptied destructuring leaves `const { } = vi.hoisted(…)`, which is valid and useless.
|
|
// Remove the whole call, whatever its body shape.
|
|
for (;;) {
|
|
const m = /const \{\s*,?\s*\} = vi\.hoisted\(/.exec(src);
|
|
if (!m) break;
|
|
const end = close(src, m.index + m[0].length - 1);
|
|
if (end === -1) break;
|
|
const tail = src.slice(end + 1).match(/^;?\n/)?.[0] ?? '';
|
|
src = src.slice(0, m.index) + src.slice(end + 1 + tail.length);
|
|
}
|
|
|
|
const header =
|
|
[...needImports].map((n) => IMPORTS[n]).join('\n') +
|
|
'\n' +
|
|
bindings.map(([l, t]) => `const ${l} = ${t};`).join('\n') +
|
|
'\n' +
|
|
(preserve.length ? preserve.join('\n') + '\n' : '');
|
|
const anchors = [...src.matchAll(/^(?:import [^\n]*;|\} from '[^']*';)\n/gm)];
|
|
const last = anchors.pop();
|
|
if (!last) {
|
|
console.log('REFUSED', file, 'no import anchor');
|
|
refused++;
|
|
continue;
|
|
}
|
|
src = src.slice(0, last.index + last[0].length) + header + src.slice(last.index + last[0].length);
|
|
|
|
if (/vi\.mock\('~\/server\/(db|redis|logging)\/client'/.test(src)) {
|
|
console.log(
|
|
'REFUSED',
|
|
file.replace('src/server/services/__tests__/', ''),
|
|
'a canonical vi.mock survived'
|
|
);
|
|
refused++;
|
|
continue;
|
|
}
|
|
if (WRITE) fs.writeFileSync(file, src);
|
|
ok++;
|
|
console.log(
|
|
'ok ',
|
|
file.replace('src/server/services/__tests__/', '').padEnd(52),
|
|
bindings.length,
|
|
'bindings',
|
|
drift.length ? `drift:${drift.join(',')}` : ''
|
|
);
|
|
}
|
|
console.log(`\n${ok} convertible, ${refused} refused${WRITE ? ' (written)' : ' (dry)'}`);
|