Fail SDK unit tests on React act warnings (#83198)

frontend/src/embedding-sdk-bundle/components/private/SdkBreadcrumbs/SdkBreadcrumbs.unit.spec.tsx

@@ -1,4 +1,6 @@
-import { act, screen, waitFor, within } from "__support__/ui";
+import userEvent from "@testing-library/user-event";
+
+import { screen, waitFor, within } from "__support__/ui";
 
 import { setup } from "./setup.spec";
 
@@ -21,7 +23,7 @@ describe("SdkBreadcrumbs", () => {
     );
 
     // Click on the collection browser item.
-    screen.getByText("Test Dashboard").click();
+    await userEvent.click(screen.getByText("Test Dashboard"));
 
     await waitFor(() => {
       expect(screen.getByTestId("current-view-id")).toHaveTextContent("1");
@@ -39,7 +41,9 @@ describe("SdkBreadcrumbs", () => {
     });
 
     // After going back to the root collection, it should no longer show the dashboard on the breadcrumb.
-    within(breadcrumbsContainer).getByText("Our analytics").click();
+    await userEvent.click(
+      within(breadcrumbsContainer).getByText("Our analytics"),
+    );
 
     await waitFor(() => {
       expect(
@@ -57,7 +61,7 @@ describe("SdkBreadcrumbs", () => {
     });
 
     // Click on the question
-    screen.getByText("Test Question").click();
+    await userEvent.click(screen.getByText("Test Question"));
 
     await waitFor(() => {
       expect(screen.getByTestId("current-view-id")).toHaveTextContent("1");
@@ -77,7 +81,9 @@ describe("SdkBreadcrumbs", () => {
     });
 
     // Navigate back to root collection
-    within(breadcrumbsContainer).getByText("Our analytics").click();
+    await userEvent.click(
+      within(breadcrumbsContainer).getByText("Our analytics"),
+    );
 
     // Question should disappear from the breadcrumb
     await waitFor(() => {
@@ -102,9 +108,7 @@ describe("SdkBreadcrumbs", () => {
     ).toBeInTheDocument();
 
     // Click on a nested collection on collection browser
-    act(() => {
-      screen.getByText("Nested Collection").click();
-    });
+    await userEvent.click(screen.getByText("Nested Collection"));
 
     await waitFor(() => {
       expect(screen.getByTestId("current-view-id")).toHaveTextContent("2");
@@ -119,9 +123,9 @@ describe("SdkBreadcrumbs", () => {
     ).toBeInTheDocument();
 
     // Click on the root collection breadcrumb to go back
-    act(() => {
-      within(breadcrumbsContainer).getByText("Our analytics").click();
-    });
+    await userEvent.click(
+      within(breadcrumbsContainer).getByText("Our analytics"),
+    );
 
     await waitFor(() => {
       expect(screen.getByTestId("current-view-id")).toHaveTextContent("root");

frontend/src/embedding-sdk-bundle/components/public/ComponentProvider/ComponentProvider.unit.spec.tsx

@@ -16,11 +16,18 @@ import { render, screen } from "__support__/ui";
 import { useInitSdkTracker } from "embedding-sdk-bundle/analytics/tracker";
 import { isHostReactVersionSupported } from "embedding-sdk-bundle/lib/host-react-version";
 import { renderWithSDKProviders } from "embedding-sdk-bundle/test/__support__/ui";
+import { useLocale } from "metabase/common/hooks/use-locale";
 
 import { ComponentProvider } from "./ComponentProvider";
 
 const mockUseInitSdkTracker = jest.mocked(useInitSdkTracker);
 
+/** Reports when `LocaleProvider` has finished loading the catalogue it asks for. */
+const LocaleProbe = () => {
+  const { isLocaleLoading } = useLocale();
+  return <div>{isLocaleLoading ? "locale loading" : "locale ready"}</div>;
+};
+
 describe("ComponentProvider — unsupported host React", () => {
   let consoleError: jest.SpyInstance;
 
@@ -58,8 +65,8 @@ describe("ComponentProvider — tracker wiring", () => {
     jest.clearAllMocks();
   });
 
-  it("passes locale != null as the third argument when locale is set", () => {
-    renderWithSDKProviders(<div />, {
+  it("passes locale != null as the third argument when locale is set", async () => {
+    renderWithSDKProviders(<LocaleProbe />, {
       componentProviderProps: {
         authConfig: { metabaseInstanceUrl: "https://metabase.example.com" },
         locale: "en",
@@ -71,6 +78,10 @@ describe("ComponentProvider — tracker wiring", () => {
       expect.anything(),
       true,
     );
+
+    // Asking for a locale loads a catalogue, and the provider updates when it
+    // lands. Wait for that rather than leaving it to resolve after the test.
+    expect(await screen.findByText("locale ready")).toBeInTheDocument();
   });
 
   it("passes false as the third argument when locale is not set", () => {

frontend/src/embedding-sdk-bundle/components/public/SdkQuestion/SdkQuestion.unit.spec.tsx

@@ -205,7 +205,12 @@ const setup = async ({
     },
   );
 
-  if (!preventWaitForLoader) {
+  if (preventWaitForLoader) {
+    // These callers assert on the loader, so wait for it to appear rather than
+    // for it to go. The provider settles while this waits, which keeps that
+    // update inside act().
+    await screen.findByTestId("loading-indicator");
+  } else {
     await waitForLoaderToBeRemoved();
   }
 };

frontend/src/embedding-sdk-bundle/components/public/question/shared-tests/alerts-button.spec.tsx

@@ -93,7 +93,9 @@ export function addAlertsButtonTests(
         });
 
         expect(screen.getByText("Custom Layout")).toBeVisible();
-        expect(screen.getByRole("button", { name: "Alerts" })).toBeVisible();
+        expect(
+          await screen.findByRole("button", { name: "Alerts" }),
+        ).toBeVisible();
       });
 
       it("should not show the alert button for custom layouts when withAlerts is false", async () => {

frontend/src/embedding-sdk-shared/jest/setup-after-env.ts

@@ -1,5 +1,49 @@
+import { format } from "util";
+
 import { ensureMetabaseProviderPropsStore } from "embedding-sdk-shared/lib/ensure-metabase-provider-props-store";
 
 afterEach(() => {
   ensureMetabaseProviderPropsStore().cleanup();
 });
+
+// `LocaleProvider` fetches a translation catalogue for whatever locale it is
+// given, including the instance locale that mock settings default to "en". The
+// load resolves after the test, so React reports the state it sets as an update
+// outside act(). A test that exercises localization passes `locale` through
+// `componentProviderProps`, which takes precedence over the instance locale.
+jest.mock("metabase/common/hooks/use-instance-locale", () => ({
+  useInstanceLocale: () => null,
+}));
+
+const ACT_WARNING_PATTERNS = [
+  /was not wrapped in act\(/,
+  /not configured to support act\(/,
+];
+
+const actWarnings: string[] = [];
+const originalConsoleError = console.error;
+
+// Throwing here would land inside React's render, where an error boundary can
+// swallow it, so the warnings are collected and reported after the test.
+console.error = (...args: unknown[]) => {
+  originalConsoleError(...args);
+
+  const [template] = args;
+  const isActWarning =
+    typeof template === "string" &&
+    ACT_WARNING_PATTERNS.some((pattern) => pattern.test(template));
+
+  if (isActWarning) {
+    actWarnings.push(format(...args));
+  }
+};
+
+afterEach(() => {
+  const warnings = actWarnings.splice(0);
+
+  if (warnings.length > 0) {
+    throw new Error(
+      `Test completed with React act() warnings:\n${warnings.join("\n")}`,
+    );
+  }
+});