mirror of
https://github.com/stack-auth/stack.git
synced 2026-06-13 21:01:21 +08:00
test fixes (#893)
<!-- Make sure you've read the CONTRIBUTING.md guidelines: https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md --> <!-- RECURSEML_SUMMARY:START --> ## High-level PR Summary This PR improves email testing in the end-to-end test suite by adding a new `waitForMessagesWithSubject` helper method to the `Mailbox` class. This method replaces the previous pattern of using arbitrary wait times (e.g., `wait(2000)`) followed by fetching and finding messages, which could lead to flaky tests. The new approach implements a polling mechanism that waits until messages with the specified subject appear, with a reasonable timeout. The PR updates all email-related tests to use this new helper method, making the tests more reliable and less dependent on timing assumptions. ⏱️ Estimated Review Time: 0h 15m <details> <summary>💡 Review Order Suggestion</summary> | Order | File Path | |-------|-----------| | 1 | `apps/e2e/tests/helpers.ts` | | 2 | `apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts` | | 3 | `apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts` | </details> <!-- RECURSEML_SUMMARY:END --> <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Improves email test reliability by adding `waitForMessagesWithSubject` in `Mailbox` and updating tests to use it. > > - **Behavior**: > - Introduces `waitForMessagesWithSubject` in `Mailbox` class in `helpers.ts` to replace arbitrary wait times with a polling mechanism. > - Updates email-related tests in `send-email.test.ts` and `unsubscribe-link.test.ts` to use `waitForMessagesWithSubject` for more reliable email verification. > - **Tests**: > - Replaces `wait()` calls with `waitForMessagesWithSubject()` in `send-email.test.ts` and `unsubscribe-link.test.ts`. > - Ensures emails are verified by subject, reducing flakiness in tests. > - **Misc**: > - Minor refactoring in `helpers.ts` to support new functionality. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=stack-auth%2Fstack-auth&utm_source=github&utm_medium=referral)<sup> for90b7bdad01. You can [customize](https://app.ellipsis.dev/stack-auth/settings/summaries) this summary. It will automatically update as commits are pushed.</sup> ---- <!-- ELLIPSIS_HIDDEN --> <!-- RECURSEML_ANALYSIS:START --> ## Review by RecurseML _🔍 Review performed on [d14317c..f42dfc5](d14317c787...f42dfc5351)_ | Severity | Location | Issue | Action | |----------|----------|-------|--------| |  | [apps/e2e/tests/helpers.ts:235](https://github.com/stack-auth/stack-auth/pull/893#discussion_r2342704515) | Async function call not wrapped with runAsynchronously | [ | <details> <summary>✅ Files analyzed, no issues (2)</summary> • `apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts` • `apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts` </details> [](https://discord.gg/n3SsVDAW6U) <!-- RECURSEML_ANALYSIS:END --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * More reliable email end-to-end tests by waiting for messages by subject instead of fixed delays and manual polling. * Multi-recipient scenarios now independently confirm delivery for each user. * Switched to snapshot-style assertions capturing full message metadata for clearer, more stable verification. * Unsubscribe email tests now wait deterministically for delivery before validating links. * **Chores** * Mailbox handling improved to reduce flakiness and repeated network calls. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
d14317c787
commit
edc68fea58
@ -294,11 +294,22 @@ it("should return 200 and send email successfully", async ({ expect }) => {
|
||||
`);
|
||||
|
||||
// Verify the email was actually sent by checking the mailbox
|
||||
await wait(2000);
|
||||
const messages = await user.mailbox.fetchMessages();
|
||||
const sentEmail = messages.find(msg => msg.subject === "Custom Test Email Subject");
|
||||
expect(sentEmail).toBeDefined();
|
||||
expect(sentEmail!.body?.html).toMatchInlineSnapshot(`"http://localhost:8102/api/v1/emails/unsubscribe-link?code=%3Cstripped+query+param%3E"`);
|
||||
const messages = await user.mailbox.waitForMessagesWithSubject("Custom Test Email Subject");
|
||||
expect(messages).toMatchInlineSnapshot(`
|
||||
[
|
||||
MailboxMessage {
|
||||
"attachments": [],
|
||||
"body": {
|
||||
"html": "http://localhost:8102/api/v1/emails/unsubscribe-link?code=%3Cstripped+query+param%3E",
|
||||
"text": "http://localhost:8102/api/v1/emails/unsubscribe-link?code=%3Cstripped+query+param%3E",
|
||||
},
|
||||
"from": "Test Project <test@example.com>",
|
||||
"subject": "Custom Test Email Subject",
|
||||
"to": ["<unindexed-mailbox--<stripped UUID>@stack-generated.example.com>"],
|
||||
<some fields may have been hidden>,
|
||||
},
|
||||
]
|
||||
`);
|
||||
});
|
||||
|
||||
it("should handle user that does not exist", async ({ expect }) => {
|
||||
@ -403,11 +414,7 @@ export function EmailTemplate({ user, project }: Props) {
|
||||
expect(sendRes.status).toBe(200);
|
||||
expect(sendRes.body.results).toHaveLength(1);
|
||||
|
||||
await wait(3000);
|
||||
const messages = await user.mailbox.fetchMessages();
|
||||
const sentEmail = messages.find(m => m.subject === "Overridden Subject");
|
||||
expect(sentEmail).toBeDefined();
|
||||
|
||||
await user.mailbox.waitForMessagesWithSubject("Overridden Subject");
|
||||
const getDraftRes = await niceBackendFetch(`/api/v1/internal/email-drafts/${draftId}`, {
|
||||
method: "GET",
|
||||
accessType: "admin",
|
||||
@ -590,13 +597,9 @@ describe("all users", () => {
|
||||
}
|
||||
`);
|
||||
|
||||
await wait(2000);
|
||||
const messagesA = await userA.mailbox.fetchMessages();
|
||||
const messagesB = await userB.mailbox.fetchMessages();
|
||||
const messagesC = await userC.mailbox.fetchMessages();
|
||||
expect(messagesA.find(m => m.subject === subject)).toBeDefined();
|
||||
expect(messagesB.find(m => m.subject === subject)).toBeDefined();
|
||||
expect(messagesC.find(m => m.subject === subject)).toBeDefined();
|
||||
await userA.mailbox.waitForMessagesWithSubject(subject);
|
||||
await userB.mailbox.waitForMessagesWithSubject(subject);
|
||||
await userC.mailbox.waitForMessagesWithSubject(subject);
|
||||
});
|
||||
});
|
||||
|
||||
@ -740,10 +743,7 @@ describe("notification categories", () => {
|
||||
`);
|
||||
|
||||
// Verify the email was sent
|
||||
await wait(2000);
|
||||
const messages = await user.mailbox.fetchMessages();
|
||||
const sentEmail = messages.find(msg => msg.subject === "Transactional Test Subject");
|
||||
expect(sentEmail).toBeDefined();
|
||||
await user.mailbox.waitForMessagesWithSubject("Transactional Test Subject");
|
||||
});
|
||||
|
||||
it("should default to Transactional category when notification_category_name is not provided", async ({ expect }) => {
|
||||
@ -782,9 +782,6 @@ describe("notification categories", () => {
|
||||
`);
|
||||
|
||||
// Verify the email was sent
|
||||
await wait(2000);
|
||||
const messages = await user.mailbox.fetchMessages();
|
||||
const sentEmail = messages.find(msg => msg.subject === "Default Category Test Subject");
|
||||
expect(sentEmail).toBeDefined();
|
||||
await user.mailbox.waitForMessagesWithSubject("Default Category Test Subject");
|
||||
});
|
||||
});
|
||||
|
||||
@ -48,10 +48,8 @@ it("unsubscribe link should be sent and update notification preference", async (
|
||||
`);
|
||||
|
||||
// Verify the email was actually sent by checking the mailbox
|
||||
await wait(2000);
|
||||
const messages = await user.mailbox.fetchMessages();
|
||||
const sentEmail = messages.find(msg => msg.subject === "Custom Test Email Subject");
|
||||
expect(sentEmail).toBeDefined();
|
||||
const messages = await user.mailbox.waitForMessagesWithSubject("Custom Test Email Subject");
|
||||
const sentEmail = messages[0];
|
||||
expect(sentEmail!.body?.html).toMatchInlineSnapshot(`"http://localhost:8102/api/v1/emails/unsubscribe-link?code=%3Cstripped+query+param%3E"`);
|
||||
|
||||
// Extract the unsubscribe link and fetch it
|
||||
@ -142,9 +140,8 @@ it("unsubscribe link should not be sent for emails with transactional notificati
|
||||
}
|
||||
`);
|
||||
|
||||
await wait(2000);
|
||||
const messages = await user.mailbox.fetchMessages();
|
||||
const sentEmail = messages.find(msg => msg.subject === "Custom Test Email Subject");
|
||||
const messages = await user.mailbox.waitForMessagesWithSubject("Custom Test Email Subject");
|
||||
const sentEmail = messages[0];
|
||||
expect(sentEmail).toBeDefined();
|
||||
expect(sentEmail!.body?.html).toMatchInlineSnapshot(`"<!DOCTYPE html PUBLIC \\"-//W3C//DTD XHTML 1.0 Transitional//EN\\" \\"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\\"><html dir=\\"ltr\\" lang=\\"en\\"><head><meta content=\\"text/html; charset=UTF-8\\" http-equiv=\\"Content-Type\\"/><meta name=\\"x-apple-disable-message-reformatting\\"/></head><body style=\\"background-color:rgb(250,251,251);font-family:ui-sans-serif, system-ui, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Noto Color Emoji";font-size:1rem;line-height:1.5rem\\"><!--$--><table align=\\"center\\" width=\\"100%\\" border=\\"0\\" cellPadding=\\"0\\" cellSpacing=\\"0\\" role=\\"presentation\\" style=\\"background-color:rgb(255,255,255);padding:45px;border-radius:0.5rem;max-width:37.5em\\"><tbody><tr style=\\"width:100%\\"><td><div><h1>Test Email</h1><p>This is a test email with HTML content.</p></div></td></tr></tbody></table><!--7--><!--/$--></body></html>"`);
|
||||
});
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
import { generateSecureRandomString } from "@stackframe/stack-shared/dist/utils/crypto";
|
||||
import { StackAssertionError } from "@stackframe/stack-shared/dist/utils/errors";
|
||||
import { filterUndefined, omit } from "@stackframe/stack-shared/dist/utils/objects";
|
||||
import { wait } from "@stackframe/stack-shared/dist/utils/promises";
|
||||
import { Nicifiable } from "@stackframe/stack-shared/dist/utils/strings";
|
||||
import { AsyncLocalStorage } from "node:async_hooks";
|
||||
// eslint-disable-next-line no-restricted-imports
|
||||
@ -129,7 +130,7 @@ export class NiceResponse implements Nicifiable {
|
||||
public readonly headers: Headers,
|
||||
public readonly body: any,
|
||||
public readonly fromRequestInit?: NiceRequestInit,
|
||||
) {}
|
||||
) { }
|
||||
|
||||
getNicifiableKeys(): string[] {
|
||||
// reorder the keys for nicer printing
|
||||
@ -198,6 +199,7 @@ export const generatedEmailRegex = /[a-zA-Z0-9_.+\-]+@stack-generated\.example\.
|
||||
|
||||
export class Mailbox {
|
||||
public readonly fetchMessages: (options?: { noBody?: boolean }) => Promise<MailboxMessage[]>;
|
||||
public readonly waitForMessagesWithSubject: (subject: string) => Promise<MailboxMessage[]>;
|
||||
|
||||
constructor(
|
||||
disclaimer: "USE_CREATE_MAILBOX_FUNCTION_INSTEAD",
|
||||
@ -205,6 +207,7 @@ export class Mailbox {
|
||||
) {
|
||||
const mailboxName = emailAddress.split("@")[0];
|
||||
const fullMessageCache = new Map<string, any>();
|
||||
|
||||
this.fetchMessages = async ({ noBody } = {}) => {
|
||||
const res = await niceFetch(new URL(`/api/v1/mailbox/${encodeURIComponent(mailboxName)}`, INBUCKET_API_URL));
|
||||
return await Promise.all((res.body as any[]).map(async (message) => {
|
||||
@ -220,6 +223,19 @@ export class Mailbox {
|
||||
return new MailboxMessage(messagePart);
|
||||
}));
|
||||
};
|
||||
|
||||
this.waitForMessagesWithSubject = async (subject: string) => {
|
||||
const maxRetries = 20;
|
||||
for (let i = 0; i < maxRetries; i++) {
|
||||
const messages = await this.fetchMessages();
|
||||
const withSubject = messages.filter(m => m.subject === subject);
|
||||
if (withSubject.length > 0) {
|
||||
return withSubject;
|
||||
}
|
||||
await wait(200);
|
||||
}
|
||||
throw new Error(`Message with subject ${subject} not found`);
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user