bermudalambandClaude Opus 5
823796b92a
Review of the Task 1 commit approved the implementation but flagged the third test in mailOutcome.test.ts as a defect carried over from the brief: it set SMTP_USER, SMTP_PASSWORD, and an allowlist the recipient satisfied, so sendMail fell through both early-return guards and reached the real transporter, opening a live TLS connection to smtp.gmail.com:465. The .catch(() => 'threw') wrapper hid a fast auth rejection, a slow timeout, or an accidental real send equally, and on a restricted CI runner it would hang to the Jest timeout rather than fail fast. The fix deletes that test rather than replacing it. What it was trying to prove — that an allowlisted recipient is not blocked — is already covered hermetically by backend/tests/unit/mailAllowlist.test.ts, which exercises isAllowedRecipient directly across exact matches, plus-suffixes, domains, and refusals. The other two tests in mailOutcome.test.ts are untouched; they cover the two paths that return early, which is the entire point of the change, and neither one reaches the transporter. The file's top doc comment is updated to match: it now says only the two skip paths are covered here, names mailAllowlist.test.ts as where the allowlist's own behaviour is tested, and spells out why a third test that reaches the transporter does not belong in this file, so nobody adds one back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
51 lines
1.9 KiB
TypeScript
51 lines
1.9 KiB
TypeScript
import { sendMail } from '../../src/mailer';
|
|
|
|
/**
|
|
* What sendMail says it did.
|
|
*
|
|
* It returns early in two cases that are indistinguishable from success at the
|
|
* call site — SMTP unconfigured, and the recipient not on MAIL_ALLOWLIST — and
|
|
* #260 needs to tell them apart so the admin is not told a link was emailed
|
|
* when it was not.
|
|
*
|
|
* Only the two skip paths are covered here. A real send needs an SMTP server,
|
|
* which a unit test has no business starting; the integration test in Task 4
|
|
* covers the route's behaviour instead.
|
|
*
|
|
* The allowlist's own behaviour — exact matches, plus-suffixes, domains,
|
|
* refusals — is covered directly and hermetically in
|
|
* backend/tests/unit/mailAllowlist.test.ts, against isAllowedRecipient itself.
|
|
* There is deliberately no third test here that sets SMTP_USER/SMTP_PASSWORD
|
|
* and an allowed recipient: that combination falls through both guards and
|
|
* reaches the real transporter, which opens a live TLS connection to
|
|
* smtp.gmail.com:465. Do not add one back.
|
|
*/
|
|
describe('what sendMail reports', () => {
|
|
const original = { ...process.env };
|
|
|
|
afterEach(() => {
|
|
process.env = { ...original };
|
|
});
|
|
|
|
it('says so when SMTP is not configured', async () => {
|
|
delete process.env.SMTP_USER;
|
|
delete process.env.SMTP_PASSWORD;
|
|
|
|
await expect(sendMail('someone@example.com', 'subject', '<p>body</p>')).resolves.toBe(
|
|
'skipped-unconfigured'
|
|
);
|
|
});
|
|
|
|
// The case that matters most: QA restricts delivery, and a blocked address
|
|
// previously returned exactly as though it had sent.
|
|
it('says so when the recipient is not on the allowlist', async () => {
|
|
process.env.SMTP_USER = 'user';
|
|
process.env.SMTP_PASSWORD = 'password';
|
|
process.env.MAIL_ALLOWLIST = 'allowed@example.com';
|
|
|
|
await expect(sendMail('someone-else@example.com', 'subject', '<p>body</p>')).resolves.toBe(
|
|
'skipped-blocked'
|
|
);
|
|
});
|
|
});
|