From 5342c7ae2d0111c2c634764362812866cbe59583 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 19 Dec 2022 11:08:25 -0500 Subject: [PATCH 1/3] Fix flakey firefox tests --- integration/fetcher-layout-test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/integration/fetcher-layout-test.ts b/integration/fetcher-layout-test.ts index 01621555a4a..815fbae6223 100644 --- a/integration/fetcher-layout-test.ts +++ b/integration/fetcher-layout-test.ts @@ -191,6 +191,7 @@ test("fetcher calls layout route action when at index route", async ({ let app = new PlaywrightFixture(appFixture, page); await app.goto("/layout-action"); await app.clickElement("button"); + await page.waitForSelector("#layout-fetcher-data"); let dataElement = await app.getElement("#layout-fetcher-data"); expect(dataElement.text()).toBe("layout action data"); dataElement = await app.getElement("#child-data"); @@ -203,6 +204,7 @@ test("fetcher calls layout route loader when at index route", async ({ let app = new PlaywrightFixture(appFixture, page); await app.goto("/layout-loader"); await app.clickElement("button"); + await page.waitForSelector("#layout-fetcher-data"); let dataElement = await app.getElement("#layout-fetcher-data"); expect(dataElement.text()).toBe("layout loader data"); }); @@ -213,6 +215,7 @@ test("fetcher calls index route action when at index route", async ({ let app = new PlaywrightFixture(appFixture, page); await app.goto("/layout-action"); await app.clickElement("#index-fetcher"); + await page.waitForSelector("#index-fetcher-data"); let dataElement = await app.getElement("#index-fetcher-data"); expect(dataElement.text()).toBe("index action data"); dataElement = await app.getElement("#child-data"); @@ -225,6 +228,7 @@ test("fetcher calls index route loader when at index route", async ({ let app = new PlaywrightFixture(appFixture, page); await app.goto("/layout-loader"); await app.clickElement("#index-fetcher"); + await page.waitForSelector("#index-fetcher-data"); let dataElement = await app.getElement("#index-fetcher-data"); expect(dataElement.text()).toBe("index data"); }); @@ -235,6 +239,7 @@ test("fetcher calls layout route action when at paramaterized route", async ({ let app = new PlaywrightFixture(appFixture, page); await app.goto("/layout-action/foo"); await app.clickElement("button"); + await page.waitForSelector("#layout-fetcher-data"); let dataElement = await app.getElement("#layout-fetcher-data"); expect(dataElement.text()).toBe("layout action data"); dataElement = await app.getElement("#child-data"); @@ -247,6 +252,7 @@ test("fetcher calls layout route loader when at paramaterized route", async ({ let app = new PlaywrightFixture(appFixture, page); await app.goto("/layout-loader/foo"); await app.clickElement("button"); + await page.waitForSelector("#layout-fetcher-data"); let dataElement = await app.getElement("#layout-fetcher-data"); expect(dataElement.text()).toBe("layout loader data"); }); @@ -255,6 +261,7 @@ test("fetcher calls paramaterized route route action", async ({ page }) => { let app = new PlaywrightFixture(appFixture, page); await app.goto("/layout-action/foo"); await app.clickElement("#param-fetcher"); + await page.waitForSelector("#param-fetcher-data"); let dataElement = await app.getElement("#param-fetcher-data"); expect(dataElement.text()).toBe("param action data"); dataElement = await app.getElement("#child-data"); @@ -265,6 +272,7 @@ test("fetcher calls paramaterized route route loader", async ({ page }) => { let app = new PlaywrightFixture(appFixture, page); await app.goto("/layout-loader/foo"); await app.clickElement("#param-fetcher"); + await page.waitForSelector("#param-fetcher-data"); let dataElement = await app.getElement("#param-fetcher-data"); expect(dataElement.text()).toBe("foo"); }); From f0decd5a4162e5283cd22482c4efc028a147e35a Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 19 Dec 2022 11:35:11 -0500 Subject: [PATCH 2/3] Fix flakey upload tests --- integration/file-uploads-test.ts | 5 ++++- integration/upload-test.ts | 25 +++++++++++++++---------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/integration/file-uploads-test.ts b/integration/file-uploads-test.ts index 555fb5cff7b..afcf7493148 100644 --- a/integration/file-uploads-test.ts +++ b/integration/file-uploads-test.ts @@ -61,6 +61,7 @@ test.describe("file-uploads", () => { }; export default function Upload() { + let actionData = useActionData(); return ( <>
@@ -69,7 +70,7 @@ test.describe("file-uploads", () => {
- 
{JSON.stringify(useActionData(), null, 2)}
+ {actionData ? 
{JSON.stringify(actionData, null, 2)}
: null} ); } @@ -100,6 +101,7 @@ test.describe("file-uploads", () => { await app.goto("/file-upload"); await app.uploadFile("#file", uploadFile); await app.clickSubmitButton("/file-upload"); + await page.waitForSelector("pre"); expect(await app.getHtml("pre")).toBe(`
 {
   "name": "underLimit.txt",
@@ -126,6 +128,7 @@ test.describe("file-uploads", () => {
     await app.goto("/file-upload");
     await app.uploadFile("#file", uploadFile);
     await app.clickSubmitButton("/file-upload");
+    await page.waitForSelector("pre");
     expect(await app.getHtml("pre")).toBe(`
 {
   "errorMessage": "Field \\"file\\" exceeded upload size of 10000 bytes."
diff --git a/integration/upload-test.ts b/integration/upload-test.ts
index 290e55ca7a2..491f49b8f02 100644
--- a/integration/upload-test.ts
+++ b/integration/upload-test.ts
@@ -184,6 +184,7 @@ test("can upload a file with createFileUploadHandler", async ({ page }) => {
   await app.goto("/file-upload-handler");
   await app.uploadFile("#file", path.resolve(__dirname, "assets/toupload.txt"));
   await app.clickSubmitButton("/file-upload-handler");
+  await page.waitForSelector("#message");
 
   expect(await app.getHtml("#message")).toMatch(">SUCCESS<");
   expect(await app.getHtml("#size")).toMatch(">13<");
@@ -200,6 +201,7 @@ test("can catch MaxPartSizeExceededError when file is too big with createFileUpl
     path.resolve(__dirname, "assets/touploadtoobig.txt")
   );
   await app.clickSubmitButton("/file-upload-handler");
+  await page.waitForSelector("#message");
 
   expect(await app.getHtml("#message")).toMatch(">FILE_TOO_LARGE<");
   expect(await app.getHtml("#size")).toMatch(">13<");
@@ -210,6 +212,7 @@ test("can upload a file with createMemoryUploadHandler", async ({ page }) => {
   await app.goto("/memory-upload-handler");
   await app.uploadFile("#file", path.resolve(__dirname, "assets/toupload.txt"));
   await app.clickSubmitButton("/memory-upload-handler");
+  await page.waitForSelector("#message");
 
   expect(await app.getHtml("#message")).toMatch(">SUCCESS<");
   expect(await app.getHtml("#size")).toMatch(">13<");
@@ -220,6 +223,7 @@ test("can upload a file with a passthrough handler", async ({ page }) => {
   await app.goto("/passthrough-upload-handler");
   await app.uploadFile("#file", path.resolve(__dirname, "assets/toupload.txt"));
   await app.clickSubmitButton("/passthrough-upload-handler");
+  await page.waitForSelector("#message");
 
   expect(await app.getHtml("#message")).toMatch(">SUCCESS<");
 });
@@ -234,6 +238,7 @@ test("can catch MaxPartSizeExceededError when file is too big with createMemoryU
     path.resolve(__dirname, "assets/touploadtoobig.txt")
   );
   await app.clickSubmitButton("/memory-upload-handler");
+  await page.waitForSelector("#message");
 
   expect(await app.getHtml("#message")).toMatch(">FILE_TOO_LARGE<");
   expect(await app.getHtml("#size")).toMatch(">13<");
@@ -249,8 +254,8 @@ test.describe("without javascript", () => {
       "#file",
       path.resolve(__dirname, "assets/toupload.txt")
     );
-
-    await Promise.all([page.click("#submit"), page.waitForNavigation()]);
+    await page.click("#submit");
+    await page.waitForSelector("#message");
 
     expect(await app.getHtml("#message")).toMatch(">SUCCESS<");
     expect(await app.getHtml("#size")).toMatch(">13<");
@@ -265,8 +270,8 @@ test.describe("without javascript", () => {
       "#file",
       path.resolve(__dirname, "assets/touploadtoobig.txt")
     );
-
-    await Promise.all([page.click("#submit"), page.waitForNavigation()]);
+    await page.click("#submit");
+    await page.waitForSelector("#message");
 
     expect(await app.getHtml("#message")).toMatch(">FILE_TOO_LARGE<");
     expect(await app.getHtml("#size")).toMatch(">13<");
@@ -279,8 +284,8 @@ test.describe("without javascript", () => {
       "#file",
       path.resolve(__dirname, "assets/toupload.txt")
     );
-
-    await Promise.all([page.click("#submit"), page.waitForNavigation()]);
+    await page.click("#submit");
+    await page.waitForSelector("#message");
 
     expect(await app.getHtml("#message")).toMatch(">SUCCESS<");
     expect(await app.getHtml("#size")).toMatch(">13<");
@@ -293,8 +298,8 @@ test.describe("without javascript", () => {
       "#file",
       path.resolve(__dirname, "assets/toupload.txt")
     );
-
-    await Promise.all([page.click("#submit"), page.waitForNavigation()]);
+    await page.click("#submit");
+    await page.waitForSelector("#message");
 
     expect(await app.getHtml("#message")).toMatch(">SUCCESS<");
   });
@@ -308,8 +313,8 @@ test.describe("without javascript", () => {
       "#file",
       path.resolve(__dirname, "assets/touploadtoobig.txt")
     );
-
-    await Promise.all([page.click("#submit"), page.waitForNavigation()]);
+    await page.click("#submit");
+    await page.waitForSelector("#message");
 
     expect(await app.getHtml("#message")).toMatch(">FILE_TOO_LARGE<");
     expect(await app.getHtml("#size")).toMatch(">13<");

From b2d88275e23b875baf45ff2113d25012f68a13e3 Mon Sep 17 00:00:00 2001
From: Matt Brophy 
Date: Mon, 19 Dec 2022 12:22:36 -0500
Subject: [PATCH 3/3] Add integration tests for useLoaderData/ErrorBoundary

---
 integration/error-boundary-test.ts | 239 +++++++++++++++++++++++++++++
 1 file changed, 239 insertions(+)

diff --git a/integration/error-boundary-test.ts b/integration/error-boundary-test.ts
index b925d9dbc83..d567effab31 100644
--- a/integration/error-boundary-test.ts
+++ b/integration/error-boundary-test.ts
@@ -1,4 +1,5 @@
 import { test, expect } from "@playwright/test";
+import { ServerMode } from "@remix-run/server-runtime/mode";
 
 import { createAppFixture, createFixture, js } from "./helpers/create-fixture";
 import type { Fixture, AppFixture } from "./helpers/create-fixture";
@@ -636,3 +637,241 @@ test.describe("ErrorBoundary", () => {
     });
   });
 });
+
+test.describe("loaderData in ErrorBoundary", () => {
+  let fixture: Fixture;
+  let appFixture: AppFixture;
+  let consoleErrors: string[];
+  let oldConsoleError: () => void;
+
+  test.beforeAll(async () => {
+    fixture = await createFixture({
+      files: {
+        "app/root.jsx": js`
+          import { Links, Meta, Outlet, Scripts } from "@remix-run/react";
+
+          export default function Root() {
+            return (
+              
+                
+                  
+                  
+                
+                
+                  

+                    
+                  

+                  
+                
+              
+            );
+          }
+        `,
+
+        "app/routes/parent.jsx": js`
+          import { Outlet, useLoaderData, useMatches } from "@remix-run/react";
+
+          export function loader() {
+            return "PARENT";
+          }
+
+          export default function () {
+            return (
+              

+                
{useLoaderData()}

+                
+              

+            )
+          }
+
+          export function ErrorBoundary({ error }) {
+            return (
+              <>
+                
{useLoaderData()}

+                

+                  {useMatches().find(m => m.id === 'routes/parent').data}
+                

+                
{error.message}

+              
+            );
+          }
+        `,
+
+        "app/routes/parent/child-with-boundary.jsx": js`
+          import { Form, useLoaderData } from "@remix-run/react";
+
+          export function loader() {
+            return "CHILD";
+          }
+
+          export function action() {
+            throw new Error("Broken!");
+          }
+
+          export default function () {
+            return (
+              <>
+                
{useLoaderData()}

+                

+                  
+                

+              
+            )
+          }
+
+          export function ErrorBoundary({ error }) {
+            return (
+              <>
+                
{useLoaderData()}

+                
{error.message}

+              
+            );
+          }
+        `,
+
+        "app/routes/parent/child-without-boundary.jsx": js`
+          import { Form, useLoaderData } from "@remix-run/react";
+
+          export function loader() {
+            return "CHILD";
+          }
+
+          export function action() {
+            throw new Error("Broken!");
+          }
+
+          export default function () {
+            return (
+              <>
+                
{useLoaderData()}

+                

+                  
+                

+              
+            )
+          }
+        `,
+      },
+    });
+
+    appFixture = await createAppFixture(fixture, ServerMode.Development);
+  });
+
+  test.afterAll(() => {
+    appFixture.close();
+  });
+
+  test.beforeEach(({ page }) => {
+    oldConsoleError = console.error;
+    console.error = () => {};
+    consoleErrors = [];
+    // Listen for all console events and handle errors
+    page.on("console", (msg) => {
+      if (msg.type() === "error") {
+        consoleErrors.push(msg.text());
+      }
+    });
+  });
+
+  test.afterEach(() => {
+    console.error = oldConsoleError;
+  });
+
+  test.describe("without JavaScript", () => {
+    test.use({ javaScriptEnabled: false });
+    runBoundaryTests();
+  });
+
+  test.describe("with JavaScript", () => {
+    test.use({ javaScriptEnabled: true });
+    runBoundaryTests();
+  });
+
+  function runBoundaryTests() {
+    test("Prevents useLoaderData in self ErrorBoundary", async ({
+      page,
+      javaScriptEnabled,
+    }) => {
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/parent/child-with-boundary");
+
+      expect(await app.getHtml("#parent-data")).toEqual(
+        '
PARENT
'
+      );
+      expect(await app.getHtml("#child-data")).toEqual(
+        '
CHILD
'
+      );
+      expect(consoleErrors).toEqual([]);
+
+      await app.clickSubmitButton("/parent/child-with-boundary");
+      await page.waitForSelector("#child-error");
+
+      expect(await app.getHtml("#child-error")).toEqual(
+        '
Broken!
'
+      );
+      expect(await app.getHtml("#parent-data")).toEqual(
+        '
PARENT
'
+      );
+      expect(await app.getHtml("#child-data")).toEqual(
+        '
'
+      );
+
+      // Only look for this message.  Chromium browsers will also log the
+      // network error but firefox does not
+      //   "Failed to load resource: the server responded with a status of 500 (Internal Server Error)",
+      let msg =
+        "You cannot `useLoaderData` in an errorElement (routeId: routes/parent/child-with-boundary)";
+      if (javaScriptEnabled) {
+        expect(consoleErrors.filter((m) => m === msg)).toEqual([msg]);
+      } else {
+        // We don't get the useLoaderData message in the client when JS is disabled
+        expect(consoleErrors.filter((m) => m === msg)).toEqual([]);
+      }
+    });
+
+    test("Prevents useLoaderData in bubbled ErrorBoundary", async ({
+      page,
+      javaScriptEnabled,
+    }) => {
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/parent/child-without-boundary");
+
+      expect(await app.getHtml("#parent-data")).toEqual(
+        '
PARENT
'
+      );
+      expect(await app.getHtml("#child-data")).toEqual(
+        '
CHILD
'
+      );
+      expect(consoleErrors).toEqual([]);
+
+      await app.clickSubmitButton("/parent/child-without-boundary");
+      await page.waitForSelector("#parent-error");
+
+      expect(await app.getHtml("#parent-error")).toEqual(
+        '
Broken!
'
+      );
+      expect(await app.getHtml("#parent-matches-data")).toEqual(
+        '
'
+      );
+      expect(await app.getHtml("#parent-data")).toEqual(
+        '
'
+      );
+
+      // Only look for this message.  Chromium browsers will also log the
+      // network error but firefox does not
+      //   "Failed to load resource: the server responded with a status of 500 (Internal Server Error)",
+      let msg =
+        "You cannot `useLoaderData` in an errorElement (routeId: routes/parent)";
+      if (javaScriptEnabled) {
+        expect(consoleErrors.filter((m) => m === msg)).toEqual([msg]);
+      } else {
+        // We don't get the useLoaderData message in the client when JS is disabled
+        expect(consoleErrors.filter((m) => m === msg)).toEqual([]);
+      }
+    });
+  }
+});