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

test(integration): mock the mailer everywhere an upload link is issued (#260)

Browse Source 
intake.integration.test.ts, intakeCeiling.integration.test.ts and uploadLinks.integration.test.ts all create upload links, and since #260 that now sends real mail. env.setup.ts never clears SMTP_USER, SMTP_PASSWORD or MAIL_ALLOWLIST, so with those inherited from a developer's shell these three files opened live TLS connections to smtp.gmail.com:465 and, with no allowlist set, actually delivered to sarah@example.com. This is the exact hazard the "Do not add one back" comment in tests/unit/mailOutcome.test.ts already warns about, reintroduced at the integration layer.

All three now mock ../../src/mailer the same way accountDetails.integration.test.ts, favorites.integration.test.ts and resendVerification.integration.test.ts already do. uploadLinks.integration.test.ts is the one place that needs to see specific MailOutcome values come back through the route, so its two outcome tests were restructured to drive the mock's return value directly (sentMail.mockResolvedValueOnce(...)) instead of threading SMTP_USER/MAIL_ALLOWLIST through the real sendMail. That is a cleaner test anyway: it isolates the route's job (reporting whatever outcome sendMail returns) from sendMail's own skip logic, which is already covered hermetically by mailOutcome.test.ts and mailAllowlist.test.ts.

Also addresses the related minor finding that nothing asserted the mail actually carried the working link: required: ['submitUrl'] on the template only guards that the placeholder is present in the body, not that the route supplied a correct value for it. A new test in uploadLinks.integration.test.ts inspects the mock's captured call and asserts the html contains the created token's /submit/ URL, and covers the three submissionsAllowed phrasings (a numeric cap, a cap of exactly one, and uncapped).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
bermudalamb
2026-09-03 18:30:35 -05:00
co-authored by Claude Opus 5
parent 9c5935e8e0
commit 5a9022d8d1
3 changed files with 93 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/integration/intake.integration.test.ts
+8
View File
@@ -4,6 +4,14 @@ import app from '../../src/app';
import { pool } from '../../src/db';
import { resetDb, closeDb } from './setup/testDb';
// Every submission-cap test here issues a link, which now emails it. Mocked
// so the suite never opens a real connection to smtp.gmail.com — see the
// "Do not add one back" warning in tests/unit/mailOutcome.test.ts, which this
// mirrors at the integration layer.
jest.mock('../../src/mailer', () => ({
sendMail: jest.fn().mockResolvedValue('sent')
}));
const UPLOADS_DIR = process.env.UPLOADS_DIR as string;
// The same 1x1 PNG the upload validation suite uses, so the accepted case
backend/tests/integration/intakeCeiling.integration.test.ts
+8
View File
@@ -4,6 +4,14 @@ import { pool } from '../../src/db';
import { resetDb, closeDb } from './setup/testDb';
import { resetAlertThrottleForTests } from '../../src/intake/abuseAlert';
// makeLink() issues a link on every test, which now emails it. Mocked so the
// suite never opens a real connection to smtp.gmail.com — see the "Do not add
// one back" warning in tests/unit/mailOutcome.test.ts, which this mirrors at
// the integration layer.
jest.mock('../../src/mailer', () => ({
sendMail: jest.fn().mockResolvedValue('sent')
}));
const PNG = Buffer.from(
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==',
'base64'
backend/tests/integration/uploadLinks.integration.test.ts
+73 -26
View File
@@ -3,8 +3,33 @@ import app from '../../src/app';
import { pool } from '../../src/db';
import { resetDb, closeDb } from './setup/testDb';
// Every test in this file issues an upload link, which now emails it.
// Mocked exactly like the other integration suites that touch mail (see
// accountDetails.integration.test.ts, favorites.integration.test.ts,
// resendVerification.integration.test.ts) so the suite never opens a real
// connection to smtp.gmail.com — see the "Do not add one back" warning in
// tests/unit/mailOutcome.test.ts, which this mirrors at the integration
// layer.
//
// Unlike those three, the "emailing the link to its recipient" describe
// block below needs to see specific MailOutcome values come back through the
// route rather than a single fixed one. Threading real SMTP_USER /
// MAIL_ALLOWLIST env vars through the real sendMail was the previous
// approach and is exactly the hazard being removed here, so those tests are
// restructured to drive the mock's return value directly instead — that also
// makes them a cleaner test of the route's outcome-reporting (its job),
// separate from sendMail's own skip logic (already covered hermetically by
// mailOutcome.test.ts and mailAllowlist.test.ts).
jest.mock('../../src/mailer', () => ({
sendMail: jest.fn().mockResolvedValue('sent')
}));
import { sendMail, MailOutcome } from '../../src/mailer';
const sentMail = sendMail as jest.MockedFunction<typeof sendMail>;
beforeEach(async () => {
await resetDb();
sentMail.mockReset();
sentMail.mockResolvedValue('sent');
});
afterAll(async () => {
@@ -112,12 +137,6 @@ describe('revoking an upload link', () => {
});
describe('emailing the link to its recipient', () => {
const original = { ...process.env };
afterEach(() => {
process.env = { ...original };
});
it('refuses to create a link with no address', async () => {
const res = await request(app)
.post('/api/admin/upload-links')
@@ -151,37 +170,29 @@ describe('emailing the link to its recipient', () => {
});
// The point of the whole change: QA blocks delivery by design, so a link that
// was not actually emailed must not be reported as though it was.
it('says the mail was not sent when SMTP is not configured', async () => {
delete process.env.SMTP_USER;
delete process.env.SMTP_PASSWORD;
// was not actually emailed must not be reported as though it was. Driven
// through the mock's return value rather than the SMTP_USER /
// MAIL_ALLOWLIST env vars sendMail itself would branch on — see the file
// header comment for why.
it.each<MailOutcome>(['skipped-unconfigured', 'skipped-blocked'])(
'reports a %s outcome from sendMail rather than as though it sent',
async (outcome) => {
sentMail.mockResolvedValueOnce(outcome);
const res = await request(app)
.post('/api/admin/upload-links')
.send({ label: 'Sarah', email: 'sarah@example.com' });
expect(res.status).toBe(201);
expect(res.body.mail).toEqual({ sent: false, outcome: 'skipped-unconfigured' });
});
it('says the mail was not sent when the address is not allowlisted', async () => {
process.env.SMTP_USER = 'user';
process.env.SMTP_PASSWORD = 'password';
process.env.MAIL_ALLOWLIST = 'someone@example.com';
const res = await request(app)
.post('/api/admin/upload-links')
.send({ label: 'Sarah', email: 'sarah@example.com' });
expect(res.status).toBe(201);
expect(res.body.mail).toEqual({ sent: false, outcome: 'skipped-blocked' });
});
expect(res.body.mail).toEqual({ sent: false, outcome });
}
);
// A send that could not happen must never cost the admin the link, because
// the token is shown exactly once and a rollback would hand them a different
// one on the retry.
it('still returns a usable link when the mail did not go', async () => {
delete process.env.SMTP_USER;
sentMail.mockResolvedValueOnce('skipped-unconfigured');
const res = await request(app)
.post('/api/admin/upload-links')
@@ -205,4 +216,40 @@ describe('emailing the link to its recipient', () => {
expect(older.contact_email).toBeNull();
expect(newer.contact_email).toBe('sarah@example.com');
});
// required: ['submitUrl'] on the template is a guard that the placeholder is
// present in the body, not that the route supplied a working value for it.
// A route that passed the bare base URL, or dropped the token, would leave
// every other test here green — so this asserts the actual captured html,
// and covers the submissionsAllowed wording for all three cases (a numeric
// cap, a cap of exactly one, and uncapped) at the same time, since all three
// are the same kind of claim: what the mail says versus what was created.
it('emails a link that actually contains the created token, and states how many items may be sent', async () => {
const capped = await request(app)
.post('/api/admin/upload-links')
.send({ label: 'Sarah', email: 'sarah@example.com', maxSubmissions: 25 });
expect(capped.status).toBe(201);
expect(sentMail).toHaveBeenCalledTimes(1);
const [cappedTo, , cappedHtml] = sentMail.mock.calls[0]!;
expect(cappedTo).toBe('sarah@example.com');
expect(cappedHtml).toContain(`/submit/${capped.body.token}`);
expect(cappedHtml).toContain('25 items');
const single = await request(app)
.post('/api/admin/upload-links')
.send({ label: 'Sarah', email: 'sarah@example.com', maxSubmissions: 1 });
expect(single.status).toBe(201);
const [, , singleHtml] = sentMail.mock.calls[1]!;
expect(singleHtml).toContain(`/submit/${single.body.token}`);
expect(singleHtml).toContain('1 item');
expect(singleHtml).not.toContain('1 items');
const uncapped = await request(app)
.post('/api/admin/upload-links')
.send({ label: 'Sarah', email: 'sarah@example.com', maxSubmissions: null });
expect(uncapped.status).toBe(201);
const [, , uncappedHtml] = sentMail.mock.calls[2]!;
expect(uncappedHtml).toContain(`/submit/${uncapped.body.token}`);
expect(uncappedHtml).toContain('as many items as you like');
});
});