From 91092fbb314f753f5c6f3a1c83ae35e4e9af9630 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 06:53:40 +0000 Subject: [PATCH] test(plugin-email): assert the over-limit attachment by identity, not deep equality The over-limit test's stated subject is the comment above it -- "delivered inline and delivered WHOLE". Deep equality is the WEAKER reading of that: it passes for a copy too, so it cannot tell an untouched buffer from one the service re-encoded and rebuilt to the same bytes. `toBe` on the captured call argument proves the exact instance the caller allocated travelled through the over-limit path untouched, which is what "whole" means. Removing the O(n) walk over the 256 KiB + 1 fixture is a consequence of that, not the reason for it. Measured in this container, under the shared verify lock, `--reporter=verbose`: target test 674 ms -> 1 ms its file, tests total 800 ms -> 125 ms (21 siblings total 126 ms) whole package 2287 ms -> 1537 ms across the same 468 tests The fixture size is unchanged (the over-limit boundary IS the subject), no timeout was raised, and nothing was skipped. The transport fake in this one test now declares its parameter so the captured call is typed rather than cast -- `vi.fn(async () => ...)` has an empty parameter tuple, which makes `mock.calls[0][0]` a type error under the package's test-layer typecheck. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --- .../src/email-service.queue-delivery.test.ts | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/packages/plugins/plugin-email/src/email-service.queue-delivery.test.ts b/packages/plugins/plugin-email/src/email-service.queue-delivery.test.ts index e4597eb04e..5ebab21fe0 100644 --- a/packages/plugins/plugin-email/src/email-service.queue-delivery.test.ts +++ b/packages/plugins/plugin-email/src/email-service.queue-delivery.test.ts @@ -16,7 +16,7 @@ import { describe, it, expect, vi } from 'vitest'; import { createHash } from 'node:crypto'; -import type { IQueueService } from '@objectstack/spec/contracts'; +import type { IQueueService, NormalizedEmailMessage } from '@objectstack/spec/contracts'; import { EmailService, EMAIL_SEND_QUEUE, @@ -277,7 +277,10 @@ describe('EmailService — queue delivery on', () => { }); it('still refuses the queue for attachments OVER the limit, and stores nothing (#5177)', async () => { - const transport = { send: vi.fn(async () => ({ messageId: '' })) }; + // The parameter is DECLARED (unlike this file's other transport fakes) so + // the captured call below is typed rather than cast — see the identity + // assertion and its note further down. + const transport = { send: vi.fn(async (_message: NormalizedEmailMessage) => ({ messageId: '' })) }; const queue = makeQueue(); const { p, rows } = makePersistence(); const logger = makeLogger(); @@ -289,11 +292,24 @@ describe('EmailService — queue delivery on', () => { const res = await svc.send({ ...MSG, attachments: [{ filename: 'big.bin', content: huge }] }); // Pre-#5177 behaviour, unchanged: delivered inline and delivered WHOLE. + // + // "WHOLE" is asserted by IDENTITY, not by deep equality (#16506). Deep + // equality is the weaker claim: it passes for a *copy* too, so it cannot + // tell an untouched buffer from one the service re-encoded, sliced and + // rebuilt to the same bytes. `toBe` proves the exact instance the caller + // allocated travelled through the over-limit path untouched — which is + // what "whole" means here, and it is what the inline path promises when + // it declines the queue. Do NOT "simplify" this back to + // `toHaveBeenCalledWith(objectContaining({ attachments: [...] }))`: that + // asserts less AND deep-compares a 256 KiB + 1 buffer, which cost 674 ms + // of this file's 800 ms — its 21 siblings total 126 ms between them — and + // ejected two PRs from the merge queue on vitest's 5000 ms default. expect(res.status).toBe('sent'); expect(queue.published).toHaveLength(0); - expect(transport.send).toHaveBeenCalledWith(expect.objectContaining({ - attachments: [{ filename: 'big.bin', content: huge }], - })); + const [delivered] = transport.send.mock.calls[0]!; + expect(delivered.attachments).toHaveLength(1); + expect(delivered.attachments![0]!.filename).toBe('big.bin'); + expect(delivered.attachments![0]!.content).toBe(huge); // The row must stay bounded: over-limit content never lands in the column. expect(rows.get(res.id)!.attachments_json).toBeUndefined(); const info = logger.info.mock.calls.map((c) => String(c[0])).join('\n');