mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
9b04487b32
* test(preview): pin reaction.toggle to its restored awaited contract `preview / smoke-tests` has been red on main since #4516. The spec asserted `reaction.toggle` resolves to null; it now resolves to "created". The code is right and the assertion was stale. Timeline:f8756fb5ddand earlier .mutation(toggleReactionHandler) -> returns the resultee376ea7d8(2026-03) perf: fire-and-forget -> returns undefined/null1e810875c8(#4516) revert to awaited -> returns the result So #4516 restored the pre-March contract rather than inventing a new one. It is deliberate on every available signal: the PR body says "Add regression test confirming the mutation returns the handler result", the router carries a "Must stay awaited" comment, and reaction.router.awaited.test.ts pins `expect(result).toBe('created')`. Reverting the router would reintroduce the bug #4516 fixed (the write detached from the request lifecycle, so a reaction could be dropped on pod drain and not survive a refresh). Nothing in production consumes the return value, so this is contained to test code. Both client call sites -- ReactionButton.tsx and Questions/FavoriteBadge.tsx -- destructure only `mutate`/`isPending` with no onSuccess/onSettled/onError and no mutateAsync; UI state is optimistic via zustand/useState. The procedure is not exposed through any REST bridge, the moderator OpenAPI catalog, or the tier-1 public-route catalog, so no published contract names its shape. Pins the exact discriminator rather than loosening to truthiness: the handler can also resolve "removed" or "noop", and both are non-null while meaning the reaction was NOT created -- which is the regression this spec exists to catch. The post is self-seeded in the same test and tester has never reacted to it, so the create branch is deterministic. Also rewrites the header block, which documented the fire-and-forget shape as current. * test(redis): take the queues integration mock off hand-typed key constants `Unit tests (1)` and `(2)` have been red on main since54466eb231, which added src/server/redis/__tests__/queues.integration.test.ts hand-typing REDIS_SYS_KEYS and REDIS_SUB_KEYS inside its `~/server/redis/client` mock: REDIS_SYS_KEYS: { QUEUES: { BUCKETS: `${NS}:buckets` } }, REDIS_SUB_KEYS: { QUEUES: { MERGING: 'merging' } }, The rule is right and the file was wrong. no-hand-typed-redis-key-constants exists to guard exactly this, and it applies to test files by construction -- its glob is `**/*.test.{ts,tsx}` and nothing else, so "it should not apply to __tests__/" would empty the rule rather than narrow it. Its rationale documents 15 constants across 6 files that had already drifted from production unnoticed (#4400), including 'session:user-tokens' for 'session:user-tokens2'. MERGING was a live instance of that shape: hand-typed identical to production today, silently free to drift tomorrow. So no exemption, no suppression comment, and no baseline entry -- the list may only shrink and it does not move here. The one real constraint is that BUCKETS could not simply be the production value: every key the suite writes derives from it, and the file promises to touch nothing real on whatever Redis it is pointed at. Resolved by moving the isolation off the CONSTANT and onto the transport. The mock now spreads @civitai/redis/client for every key, and the sysRedis stub prefixes arg 0 of each command with the run-unique namespace. That mirrors what the Postgres half of this suite already does -- a scratch schema on the session search_path, with the SQL left unqualified -- and it means the suite now exercises the production key names instead of a copy of them. Verified against a real Postgres + Redis (the suite skips without both URLs, so CI never ran it): before 7 passed -- guard red: +queues.integration.test.ts after 7 passed -- guard green: 4 passed, positive control included Confirmed by observation rather than by the suite going green: redis MONITOR during a run shows HSET/SADD on "queues-it:<pid>:queues:buckets" and "queues-it:<pid>:queues:buckets:images_v6:Delete:<ts>", with the bucket name stored as an unprefixed production-shaped value, and cleanup's KEYS "queues-it:<pid>*" reaping both. Mutation-tested so the rewrite is not passing vacuously: - nsKey -> identity 2 tests fail on their own arrayContaining - drop the `...actual` spread 3 tests fail; proves the constants come from the package and NOT from the global src/__tests__/setup.ts mock Also checked: with both env vars unset the suite still skips cleanly (7 skipped, not 0 collected), so the added module-level import does not disturb the CI path. * test(redis): drive the canonical shared mocks from the queues integration suite Fixes the OTHER half of the red unit shards. `Unit tests (1)` and `(2)` are NOT the same failure -- they are two different guards tripped by the same file from54466eb231: Unit tests (1) no-hand-typed-redis-key-constants hand-typed REDIS_*_KEYS Unit tests (2) no-direct-shared-module-mock direct vi.mock of a canonical specifier The previous commit fixed only the first. This fixes both, and supersedes its mechanism for the same file. queues.integration.test.ts directly mocked all three CANONICAL specifiers -- ~/server/db/client, ~/server/redis/client and ~/server/logging/client. Under `isolate: false` a per-file vi.mock of one of those freezes that shape into every later file in the same worker, which is the whole reason the guard exists. There is no exemption available and I did not try to manufacture one: scripts/test-perf/gen-mock-allowlist.mjs refuses on MEMBERSHIP growth (178 -> 179, exit 1), so the allowlist is not a route for a new file. Being an integration test is not an exemption either -- only a constraint on how the seam gets used. The canonical nodes are vi.fn()s, so rather than declaring canned return values this file now points them at the REAL clients it builds in beforeAll: dbMock.dbWrite.$queryRaw / $executeRaw -> the real scratch-schema PrismaClient, forwarding (strings, ...values) so the tagged-template binding this suite exists to prove is still exercised redisMock.sysRedis.<cmd> -> the real redis client, prefixing arg 0 with the run-unique namespace That also subsumes the hand-typed constants: setup.ts spreads the real @civitai/redis/client into the canonical factory, so REDIS_SYS_KEYS and REDIS_SUB_KEYS are production's own values and cannot drift -- no local override needed at all. withSysReadDeadline defaults to the real implementation there too, so queues.ts keeps running the real deadline wrapper. The local ~/server/logging/client stub is dropped because the canonical mock provides exactly logToAxiom. ~/server/redis/fail-open-log stays: it is a PENDING specifier, counted rather than enforced, with no canonical mock to move to. The generated allowlist is deliberately NOT regenerated here. It is not required -- canonical stays at 178 and the file no longer appears there -- and a regen sweeps in six unrelated pending-list entries that accumulated on main, which is churn this PR should not carry. Verified against a real Postgres + Redis (the suite skips without both URLs, so CI has never run it): 7 passed (7) before and after -- behaviour-preserving redis MONITOR identical wire traffic to the pre-rewrite version: HSET "queues-it:<pid>:queues:buckets" ... "queues:buckets:images_v6:Delete:<ts>" SADD "queues-it:<pid>:queues:buckets:images_v6:Delete:<ts>" "1" "2" "3" DEL both keys on cleanup Mutation-tested so the new wiring is not passing vacuously: - nsKey -> identity 2 tests fail on their own assertion - neuter the $queryRaw delegation 6 tests fail Both guards now pass, and the whole services suite that surfaced the second one goes 378 files / 5408 tests green (was 1 failed). Redis dir: 14 files, 124 tests. With both env vars unset the suite still skips cleanly (7 skipped, not 0 collected).
114 lines
6.1 KiB
TypeScript
114 lines
6.1 KiB
TypeScript
import { expect, test } from '@playwright/test';
|
|
import { storageStatePath } from './preview-fixtures';
|
|
import { trpcMutation, uniqueToken } from './preview-trpc';
|
|
|
|
/**
|
|
* Mutation smoke: the core ENGAGEMENT write path — reactions + comments — which is
|
|
* by far the highest-frequency user mutation on the platform and is otherwise
|
|
* untested by the preview suite. A PR that broke `reaction.toggle` or
|
|
* `commentv2.upsert` passes every other preview spec today; this closes that gap.
|
|
*
|
|
* Fully API-driven via tRPC so the flow is isolated per run by a unique token (no
|
|
* collision on the shared dev DB across concurrent previews, no flaky UI menu) —
|
|
* same self-seed-then-act pattern as preview-report.spec.ts.
|
|
*
|
|
* Runs as `tester` (free member that PASSES the preview gate). post.create,
|
|
* reaction.toggle and commentv2.upsert are all guardedProcedures; the ci-smoke
|
|
* `tester` fixture is seeded with onboarding=15 so it clears guardedProcedure.
|
|
*
|
|
* Verified input/return shapes (against origin/main schema + controller files):
|
|
* - post.create (post.router.ts .input(postCreateSchema)) — returns the new post,
|
|
* `.id` is a number (see preview-report.spec.ts).
|
|
* - reaction.toggle (reaction.router.ts:9 guardedProcedure .input(toggleReactionSchema))
|
|
* — toggleReactionSchema (reaction.schema.ts:37): { entityId: number;
|
|
* entityType: enum(reactableEntities incl. 'post'); reaction: enum(ReviewReactions) }.
|
|
* ReviewReactions.Like = 'Like' (enums.ts:355). The mutation is AWAITED and
|
|
* RESOLVES TO THE HANDLER RESULT: `.mutation(toggleReactionHandler)`
|
|
* (reaction.router.ts:15) returns toggleReaction()'s discriminator, one of
|
|
* 'created' | 'removed' | 'noop' (reaction.service.ts:25,30 → controller :286).
|
|
* This post is self-seeded in this same test and `tester` has never reacted to
|
|
* it, so getReaction finds nothing and the create branch runs — the value is
|
|
* deterministically 'created', which is what we pin.
|
|
*
|
|
* 🔴 It used to resolve to null, and this spec used to assert that. `ee376ea7d8`
|
|
* (2026-03) made the router fire-and-forget for latency; #4516 / `1e810875c8`
|
|
* reverted that because detaching the write from the request lifecycle dropped
|
|
* reactions on pod drain. `src/server/routers/__tests__/reaction.router.awaited.test.ts`
|
|
* is the unit-level contract test for the restored shape — keep the two in step.
|
|
* - commentv2.upsert (commentv2.router.ts:62 guardedProcedure .input(upsertCommentv2Schema))
|
|
* — upsertCommentv2Schema (commentv2.schema.ts) extends commentConnectorSchema
|
|
* ({ entityId: number; entityType: enum incl. 'post' }) with a non-empty sanitized
|
|
* `content` (allowed tags incl. 'p'). The isOwnerOrModerator middleware skips its
|
|
* ownership check when `id` is absent (`!!id` is false for a new comment), so a
|
|
* fresh comment is allowed. upsertCommentV2Handler returns the created comment via
|
|
* commentV2Select → `.id` (number) + `.content` (sanitized HTML carrying the token).
|
|
* - commentv2.delete (commentv2.router.ts:68 protectedProcedure .input(getByIdSchema))
|
|
* — { id: number }; owner may delete (isOwnerOrModerator). Used for cleanup.
|
|
*/
|
|
|
|
test.describe('tester self-seeds a post, reacts to it, and comments on it (mutation flow)', () => {
|
|
test.use({ storageState: storageStatePath('tester') });
|
|
|
|
test('reaction.toggle and commentv2.upsert round-trip on a self-seeded post', async ({
|
|
page,
|
|
}) => {
|
|
// Warm the request context against the preview origin so page.request shares the
|
|
// auth cookie + a real navigated origin (the helper stamps Origin/Referer, but
|
|
// navigating once is the safe baseline — mirrors preview-report.spec.ts).
|
|
await page.goto('/', { waitUntil: 'domcontentloaded' });
|
|
|
|
const token = uniqueToken('engagement');
|
|
|
|
// 1. Self-seed a Post carrying the unique token, to react to and comment on.
|
|
const post = await trpcMutation<{ id: number } | null>(page.request, 'post.create', {
|
|
title: token,
|
|
detail: token,
|
|
});
|
|
expect(typeof post?.id, 'post.create should return a numeric post id').toBe('number');
|
|
|
|
// 2. React to that exact post. reaction.toggle is awaited and resolves to the
|
|
// handler's result, so this asserts the WRITE happened, not merely that the call
|
|
// was accepted: a first Like on a post nobody has reacted to takes the create
|
|
// branch and resolves to 'created'. A broken auth/rate-limit/dispatch path would
|
|
// surface as an HTTP/tRPC error before we get here.
|
|
const reaction = await trpcMutation<'created' | 'removed' | 'noop'>(
|
|
page.request,
|
|
'reaction.toggle',
|
|
{ entityId: post!.id, entityType: 'post', reaction: 'Like' }
|
|
);
|
|
// Pinned to the exact discriminator rather than a truthiness check: 'noop' and
|
|
// 'removed' are also non-null, and both would mean the reaction did NOT get
|
|
// created — the regression this spec exists to catch.
|
|
expect(reaction, 'reaction.toggle should create the reaction and say so').toBe('created');
|
|
|
|
// 3. Comment on that exact post. upsert returns the created comment row, so we
|
|
// assert it came back with a numeric id and that our token survived sanitization
|
|
// into its content (proves the write actually persisted, not just 200-OK'd).
|
|
const comment = await trpcMutation<{ id: number; content: string } | null>(
|
|
page.request,
|
|
'commentv2.upsert',
|
|
{
|
|
entityType: 'post',
|
|
entityId: post!.id,
|
|
content: `<p>${token}</p>`,
|
|
}
|
|
);
|
|
expect(typeof comment?.id, 'commentv2.upsert should return a numeric comment id').toBe(
|
|
'number'
|
|
);
|
|
expect(comment?.content, 'the seeded token should survive into the stored comment').toContain(
|
|
token
|
|
);
|
|
|
|
// 4. Clean up our own comment so repeated preview runs don't accrete rows on the
|
|
// shared dev clone. Best-effort: a delete failure must not fail the engagement
|
|
// assertions above, which are the point of this spec.
|
|
try {
|
|
await trpcMutation(page.request, 'commentv2.delete', { id: comment!.id });
|
|
} catch {
|
|
// The reaction + comment writes already passed; leftover-row cleanup is
|
|
// non-critical and intentionally swallowed.
|
|
}
|
|
});
|
|
});
|