From 0015996cd96992a4eaf4e7022046fb34738f9dff Mon Sep 17 00:00:00 2001 From: Zachary Lowden Date: Thu, 30 Jul 2026 11:09:59 -0500 Subject: [PATCH] fix(tests): repair stale approve-gate assertion after the icon+cover floor (#3392) (#3468) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `preview / smoke-tests` check has been red on every open PR for days on a STALE test assertion, not a real regression. #3392 (81fe8f7c90, 2026-07-26) swapped the live approve gate from `assertListingAssetsComplete` to `assertListingMeetsFloor`, changing the emitted message from "Listing is missing required assets: ..." to "Listing needs at least an icon and cover before it can be published (missing: icon, cover)." `tests/preview-apps-external-approve.spec.ts` was last touched 2026-07-23 (76706c367d, #3317) and still pinned the OLD sentence verbatim, so it failed. The gate is working correctly; the test was wrong. Changes (test-only, no production code): - Assert the gate's INTENT — the error names each missing FLOOR asset (`icon` and `cover`) — instead of pinning the sentence verbatim, so a future reword does not re-break it. - Fix the now-wrong comments naming `assertListingAssetsComplete` as the live approve gate (spec header + the inline comment above the assertion). - Sibling sweep: the `preview-apps-external-delist.spec.ts` header made the same stale claim (naming the old gate and "icon+cover+>=1 screenshot"); corrected to the floor gate + the scan-clean gate. Its conclusion (approve is unreachable in a preview) still holds and is unchanged. Deliberately NOT changed: `app-listings.router.offsite-authz.test.ts` also contains "missing required assets", but that is a unit test asserting against its OWN mocked error string — it is self-consistent and not stale. Co-authored-by: Claude Opus 5 (1M context) --- tests/preview-apps-external-approve.spec.ts | 23 +++++++++++++-------- tests/preview-apps-external-delist.spec.ts | 13 ++++++------ 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/tests/preview-apps-external-approve.spec.ts b/tests/preview-apps-external-approve.spec.ts index 7a30192e44..9eedabced7 100644 --- a/tests/preview-apps-external-approve.spec.ts +++ b/tests/preview-apps-external-approve.spec.ts @@ -12,8 +12,8 @@ import { trpcMutation, trpcQuery } from './preview-trpc'; * (reject is terminal + releases the slug → self-cleaning, no leftover row). * * (2) APPROVE-GATE: submit (mod, NO assets — no icon/cover/screenshot) → - * `approveExternalRequest` → asserts a BAD_REQUEST (the dark P1 - * `assertListingAssetsComplete` gate fires: "missing required assets"), + * `approveExternalRequest` → asserts a BAD_REQUEST (the publish-FLOOR gate + * `assertListingMeetsFloor` fires, naming the missing `icon` + `cover`), * proving approve is BLOCKED without assets. Then `withdrawExternalRequest` * cleans the still-pending draft. * @@ -207,9 +207,10 @@ test.describe('App Blocks P3a PR-b: off-site approve/reject (mod, self-cleaning) ); publishRequestId = result.publishRequestId; - // APPROVE with NO icon/cover/screenshot attached → the assertListingAssetsComplete - // gate MUST fire (the router maps it to a BAD_REQUEST / HTTP 400 whose message - // names the missing assets). trpcMutation throws on a non-2xx response. + // APPROVE with NO icon/cover/screenshot attached → the publish-FLOOR gate + // `assertListingMeetsFloor` MUST fire (the router maps it to a BAD_REQUEST / + // HTTP 400 whose message names the missing floor assets). trpcMutation throws + // on a non-2xx response. let approveError: Error | null = null; try { await trpcMutation(request, 'appListings.approveExternalRequest', { @@ -220,10 +221,14 @@ test.describe('App Blocks P3a PR-b: off-site approve/reject (mod, self-cleaning) approveError = err as Error; } expect(approveError, 'approve without assets must be rejected by the gate').not.toBeNull(); - expect( - approveError?.message ?? '', - 'the gate error names the missing required assets' - ).toMatch(/missing required assets/i); + // Assert the gate's INTENT — the error names each missing FLOOR asset — rather + // than pinning the sentence verbatim. The exact wording has already churned + // once (#3392 swapped the live approve gate from `assertListingAssetsComplete` + // → `assertListingMeetsFloor`, silently breaking a verbatim match here), and the + // wording is not the contract under test; naming what's missing is. + const approveMessage = approveError?.message ?? ''; + expect(approveMessage, 'the gate error names the missing icon').toMatch(/icon/i); + expect(approveMessage, 'the gate error names the missing cover').toMatch(/cover/i); // The gate fired BEFORE any mutation → the request is still pending. const stillPending = await findPendingBySlug(request, SLUG); diff --git a/tests/preview-apps-external-delist.spec.ts b/tests/preview-apps-external-delist.spec.ts index ae6e6b6abc..624061c01e 100644 --- a/tests/preview-apps-external-delist.spec.ts +++ b/tests/preview-apps-external-delist.spec.ts @@ -11,13 +11,14 @@ import { trpcMutation, trpcQuery } from './preview-trpc'; * approve-success → store-render → delist/claim round-trip: * * The full "approve an off-site listing, see it in the store, delist/claim it, - * purge to self-clean" round-trip requires a SUCCESSFUL approve, which the dark P1 - * asset gate (`assertListingAssetsComplete`) blocks unless the draft has an - * icon+cover+≥1 screenshot whose backing Image is `ingestion = Scanned`. In a PR - * preview the external image scanner is UNREACHABLE, so uploaded images stay + * purge to self-clean" round-trip requires a SUCCESSFUL approve, which two approve + * gates block: the publish FLOOR (`assertListingMeetsFloor` — icon+cover required, + * screenshots optional since #3392) and the go-live SCAN-CLEAN gate, which requires + * every attached asset's backing Image to be `ingestion = Scanned`. In a PR preview + * the external image scanner is UNREACHABLE, so uploaded images stay * `Pending` forever (see `tests/preview-post-images.spec.ts`: "image ingestion … - * is unreachable in preview so the row stays Pending"). An attach of a Pending - * image is rejected ("scan is not complete"), so an off-site listing can NOT reach + * is unreachable in preview so the row stays Pending"). A Pending asset therefore + * never clears the scan-clean gate, so an off-site listing can NOT reach * `approved`/`removed` in preview — i.e. a CLAIMABLE-state listing is not * constructible here. So the claim HAPPY-PATH (approved/removed → reassigned + * audit event) is covered EXHAUSTIVELY in the unit tests instead