From edc68fea58dd03aef68166f810ec1f009a6b437c Mon Sep 17 00:00:00 2001
From: BilalG1 <bg2002@gmail.com>
Date: Fri, 12 Sep 2025 10:41:53 -0700
Subject: [PATCH] test fixes (#893)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!--

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>
for 90b7bdad01fc73edbaa725e08450d0a1b9b4be26. 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](https://github.com/stack-auth/stack-auth/compare/d14317c78713e383ef058973ee545ac88eb63d3a...f42dfc53512356aa80c0d170adbc361d919e2705)_

| Severity | Location | Issue | Action |
|----------|----------|-------|--------|
| ![Medium](https://img.shields.io/badge/Medium-yellow?style=plastic) |
[apps/e2e/tests/helpers.ts:235](https://github.com/stack-auth/stack-auth/pull/893#discussion_r2342704515)
| Async function call not wrapped with runAsynchronously |
[![Dismiss](https://img.shields.io/badge/Dismiss-lightgray?style=plastic)](https://squash-322339097191.europe-west3.run.app/interactive/94b4ba1ab226246ff7102274bc246d6555e6dc4b3287aedac73fe998f4900bc9/?repo_owner=stack-auth&repo_name=stack-auth&pr_number=893)
|

<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>

[![Need help? Join our
Discord](https://img.shields.io/badge/Need%20help%3F%20Join%20our%20Discord-5865F2?style=plastic&logo=discord&logoColor=white)](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 -->
---
 .../endpoints/api/v1/send-email.test.ts       | 47 +++++++++----------
 .../endpoints/api/v1/unsubscribe-link.test.ts | 11 ++---
 apps/e2e/tests/helpers.ts                     | 18 ++++++-
 3 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts
index abc39981a..ebccc89dd 100644
--- a/apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts
+++ b/apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts
@@ -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");
   });
 });
diff --git a/apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts
index 613b964f9..e937a8a93 100644
--- a/apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts
+++ b/apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts
@@ -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, &quot;Apple Color Emoji&quot;, &quot;Segoe UI Emoji&quot;, &quot;Segoe UI Symbol&quot;, &quot;Noto Color Emoji&quot;;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>"`);
 });
diff --git a/apps/e2e/tests/helpers.ts b/apps/e2e/tests/helpers.ts
index e914cc096..183bcd1e7 100644
--- a/apps/e2e/tests/helpers.ts
+++ b/apps/e2e/tests/helpers.ts
@@ -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`);
+    };
   }
 }