bermudalamb/redefined-designs
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects 3 Releases Wiki Activity

test(mail): stop the allowlist test from reaching real Gmail (#260)

Browse Source 
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>
This commit is contained in:
bermudalamb
2026-09-03 17:46:29 -05:00
co-authored by Claude Opus 5
parent 8a35c213fc
commit 823796b92a
1 changed files with 11 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/unit/mailOutcome.test.ts
+11 -15
View File
@@ -8,9 +8,17 @@ import { sendMail } from '../../src/mailer';
* #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. 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.
* 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 };
@@ -39,16 +47,4 @@ describe('what sendMail reports', () => {
'skipped-blocked'
);
});
it('does not report blocked for an address that is on the allowlist', async () => {
process.env.SMTP_USER = 'user';
process.env.SMTP_PASSWORD = 'password';
process.env.MAIL_ALLOWLIST = 'allowed@example.com';
// Not asserting 'sent': that would need a live SMTP server. Asserting only
// that the allowlist did not refuse it, which is this test's subject.
await expect(
sendMail('allowed@example.com', 'subject', '<p>body</p>').catch(() => 'threw')
).resolves.not.toBe('skipped-blocked');
});
});