refactor(tests): wrong usage of waitFor (#9606)

* refactor(tests): to use / not use waitFor properly

* refactor(tests): more

* refactor(tests): more

* refactor: deprecate renderWithHookWrappersTL

* feat: enable eslint-plugin-testing-library and add rules

* chore: refactor

* fix: remove a11y label and fix test

.eslintrc.js

@@ -16,6 +16,7 @@ module.exports = {
   ],
   extends: [
     "eslint:recommended",
+    "plugin:testing-library/react",
     "plugin:@typescript-eslint/recommended-requiring-type-checking",
     "plugin:@typescript-eslint/recommended",
     "plugin:import/recommended",
@@ -58,6 +59,13 @@ module.exports = {
     "unused-imports/no-unused-imports": OFF, // look below
     "no-autofix/unused-imports/no-unused-imports": ERR,
 
+    /**
+     * Rules for tests see https://github.com/testing-library/eslint-plugin-testing-library#supported-rules for details
+     * on default enabled rules or to remove rules.
+     */
+
+    "testing-library/no-global-regexp-flag-in-query": OFF,
+
     /**
      * Warnings
      */
@@ -143,5 +151,9 @@ module.exports = {
         "@typescript-eslint/no-non-null-assertion": OFF,
       },
     },
+    {
+      files: ["**/__tests__/**/*.[jt]s?(x)", "**/?(*.)+(spec|test).[jt]s?(x)"],
+      extends: ["plugin:testing-library/react"],
+    },
   ],
 }

package.json

@@ -294,7 +294,7 @@
     "eslint-plugin-prettier": "4.2.1",
     "eslint-plugin-react": "7.32.1",
     "eslint-plugin-react-hooks": "4.6.0",
-    "eslint-plugin-testing-library": "5.9.1",
+    "eslint-plugin-testing-library": "6.2.0",
     "eslint-plugin-unused-imports": "2.0.0",
     "graphql": "16.8.1",
     "graphql-request": "5.1.0",

src/app/Scenes/MyCollection/Screens/Artwork/Components/Field.tests.tsx

@@ -1,47 +1,44 @@
-import { fireEvent, waitFor } from "@testing-library/react-native"
+import { fireEvent, screen } from "@testing-library/react-native"
 import { Field } from "app/Scenes/MyCollection/Screens/Artwork/Components/Field"
 import { renderWithWrappers } from "app/utils/tests/renderWithWrappers"
 
 describe("Field", () => {
   it("Value is truncated when truncateLimit is set", () => {
-    const { queryByText } = renderWithWrappers(
-      <Field label="Test" value={longText} truncateLimit={5} />
-    )
-    expect(queryByText(longText)).toBeNull()
+    renderWithWrappers(<Field label="Test" value={longText} truncateLimit={5} />)
 
-    expect(queryByText("Lorem")).not.toBeNull()
+    expect(screen.queryByText(longText)).not.toBeOnTheScreen()
+    expect(screen.getByText("Lorem")).toBeOnTheScreen()
   })
 
   it("Value is NOT truncated when truncateLimit is not given", () => {
-    const { queryByText } = renderWithWrappers(<Field label="Test" value={longText} />)
-    expect(queryByText(longText)).not.toBeNull()
+    renderWithWrappers(<Field label="Test" value={longText} />)
+
+    expect(screen.getByText(longText)).toBeOnTheScreen()
   })
 
-  it("Read More button is only present if value can be expanded", () => {
-    const { queryByText: queryByTextOne } = renderWithWrappers(
-      <Field label="Test" value={longText} truncateLimit={longText.length} />
-    )
-    expect(queryByTextOne("Read More")).toBeNull()
+  describe("Read More button is only present if value can be expanded", () => {
+    it("Read More button is not present if truncateLimit is not given", () => {
+      renderWithWrappers(<Field label="Test" value={longText} truncateLimit={longText.length} />)
+      expect(screen.queryByText("Read More")).not.toBeOnTheScreen()
+    })
 
-    const { queryByText: queryByTextTwo } = renderWithWrappers(
-      <Field label="Test" value={longText} truncateLimit={longText.length - 20} />
-    )
-    expect(queryByTextTwo("Read More")).not.toBeNull()
+    it("Read More button is not present if truncateLimit is given", () => {
+      renderWithWrappers(
+        <Field label="Test" value={longText} truncateLimit={longText.length - 20} />
+      )
+      expect(screen.getByText("Read More")).toBeOnTheScreen()
+    })
   })
 
   it('Pressing "Read More" expands the text value', async () => {
-    const { findByTestId, queryByText } = renderWithWrappers(
-      <Field label="Test" value={longText} truncateLimit={10} />
-    )
+    renderWithWrappers(<Field label="Test" value={longText} truncateLimit={10} />)
 
-    expect(queryByText(longText)).toBeNull()
+    expect(screen.queryByText(longText)).not.toBeOnTheScreen()
 
-    const button = await findByTestId("ReadMoreButton")
+    const button = screen.getByTestId("ReadMoreButton")
 
-    waitFor(() => {
-      fireEvent.press(button)
-      expect(queryByText(longText)).not.toBeNull()
-    })
+    fireEvent.press(button)
+    expect(screen.getByText(longText)).toBeOnTheScreen()
   })
 })
 

src/app/Scenes/SavedSearchAlert/Components/SavedSearchFilterAdditionalGeneIDs.tests.tsx

@@ -1,5 +1,5 @@
 import { OwnerType } from "@artsy/cohesion"
-import { fireEvent, waitFor } from "@testing-library/react-native"
+import { fireEvent, screen } from "@testing-library/react-native"
 import { SavedSearchFilterAdditionalGeneIDs } from "app/Scenes/SavedSearchAlert/Components/SavedSearchFilterAdditionalGeneIDs"
 import {
   SavedSearchModel,
@@ -13,22 +13,22 @@ const black100Hex = "#000000"
 
 describe("SavedSearchFilterAdditionalGeneIDs", () => {
   it("shows all available categories unselected", () => {
-    const { getByText } = renderWithWrappers(
+    renderWithWrappers(
       <SavedSearchStoreProvider runtimeModel={initialData}>
         <SavedSearchFilterAdditionalGeneIDs />
       </SavedSearchStoreProvider>
     )
 
     gravityArtworkMediumCategories.slice(0, 7).forEach((option) => {
-      expect(getByText(option.label)).toBeDefined()
-      expect(getByText(option.label)).toHaveStyle({
+      expect(screen.getByText(option.label)).toBeOnTheScreen()
+      expect(screen.getByText(option.label)).toHaveStyle({
         color: black100Hex,
       })
     })
   })
 
   it("shows the right selected categories", () => {
-    const { getByText } = renderWithWrappers(
+    renderWithWrappers(
       <SavedSearchStoreProvider
         runtimeModel={{ ...initialData, attributes: { additionalGeneIDs: ["painting"] } }}
       >
@@ -38,34 +38,32 @@ describe("SavedSearchFilterAdditionalGeneIDs", () => {
 
     gravityArtworkMediumCategories.slice(0, 7).forEach((option) => {
       if (option.value === "painting") {
-        expect(getByText("Painting")).not.toHaveStyle({ color: black100Hex })
+        expect(screen.getByText("Painting")).not.toHaveStyle({ color: black100Hex })
       } else {
-        expect(getByText(option.label)).toBeDefined()
-        expect(getByText(option.label)).toHaveStyle({
+        expect(screen.getByText(option.label)).toBeOnTheScreen()
+        expect(screen.getByText(option.label)).toHaveStyle({
           color: black100Hex,
         })
       }
     })
   })
 
   it("Updates selected categories filters on press", () => {
-    const { getByText } = renderWithWrappers(
+    renderWithWrappers(
       <SavedSearchStoreProvider runtimeModel={initialData}>
         <SavedSearchFilterAdditionalGeneIDs />
       </SavedSearchStoreProvider>
     )
 
-    expect(getByText("Work on Paper")).toHaveStyle({ color: black100Hex })
+    expect(screen.getByText("Work on Paper")).toHaveStyle({ color: black100Hex })
 
-    fireEvent(getByText("Work on Paper"), "onPress")
+    fireEvent(screen.getByText("Work on Paper"), "onPress")
 
-    waitFor(() => {
-      expect(getByText("Work on Paper")).not.toHaveStyle({ color: black100Hex })
-    })
+    expect(screen.getByText("Work on Paper")).not.toHaveStyle({ color: black100Hex })
   })
 
   it("Shows all categories if the user has already selected mediums", () => {
-    const { getByText } = renderWithWrappers(
+    renderWithWrappers(
       <SavedSearchStoreProvider
         runtimeModel={{
           ...initialData,
@@ -79,7 +77,7 @@ describe("SavedSearchFilterAdditionalGeneIDs", () => {
     )
 
     gravityArtworkMediumCategories.forEach((option) => {
-      expect(getByText(option.label)).toBeDefined()
+      expect(screen.getByText(option.label)).toBeDefined()
     })
   })
 })

src/app/Scenes/SavedSearchAlert/Components/SavedSearchFilterColor.tests.tsx

@@ -1,5 +1,5 @@
 import { OwnerType } from "@artsy/cohesion"
-import { fireEvent, waitFor } from "@testing-library/react-native"
+import { fireEvent } from "@testing-library/react-native"
 import {
   COLORS_INDEXED_BY_VALUE,
   COLOR_OPTIONS,
@@ -56,9 +56,7 @@ describe("SavedSearchFilterColor", () => {
 
     fireEvent(getByText("Red"), "onPress")
 
-    waitFor(() => {
-      expect(getByTestId("check-icon-Red")).toBeDefined()
-    })
+    expect(getByTestId("check-icon-Red")).toBeDefined()
   })
 })
 

src/app/Scenes/SavedSearchAlert/Components/SavedSearchFilterPriceRange.tests.tsx

@@ -1,5 +1,5 @@
 import { OwnerType } from "@artsy/cohesion"
-import { fireEvent, waitFor } from "@testing-library/react-native"
+import { fireEvent, screen, waitForElementToBeRemoved } from "@testing-library/react-native"
 import { SavedSearchFilterPriceRangeQR } from "app/Scenes/SavedSearchAlert/Components/SavedSearchFilterPriceRange"
 import {
   SavedSearchModel,
@@ -9,7 +9,7 @@ import {
 import { setupTestWrapper } from "app/utils/tests/setupTestWrapper"
 
 describe("SavedSearchFilterPriceRange", () => {
-  it("shows the right price range when available", () => {
+  it("shows the right price range when available", async () => {
     const { renderWithRelay } = setupTestWrapper({
       Component: () => (
         <SavedSearchStoreProvider runtimeModel={initialData}>
@@ -18,17 +18,17 @@ describe("SavedSearchFilterPriceRange", () => {
       ),
     })
 
-    const { getByText } = renderWithRelay({
+    renderWithRelay({
       Artist: () => ({
         internalID: "artistID",
         name: "Banksy",
       }),
     })
 
-    waitFor(() => {
-      expect(getByText("200")).toBeDefined()
-      expect(getByText("3000")).toBeDefined()
-    })
+    await waitForElementToBeRemoved(() => screen.getByTestId("loading-skeleton"))
+
+    expect(screen.getByLabelText("Minimum Price Range Input")).toHaveProp("value", "200")
+    expect(screen.getByLabelText("Maximum Price Range Input")).toHaveProp("value", "3000")
   })
 
   it("Updates the price range appropriately", async () => {
@@ -40,23 +40,23 @@ describe("SavedSearchFilterPriceRange", () => {
       ),
     })
 
-    const { getByTestId, getByText } = renderWithRelay({
+    renderWithRelay({
       Artist: () => ({
         internalID: "artistID",
         name: "Banksy",
       }),
     })
 
-    waitFor(() => {
-      expect(getByText("200")).toBeDefined()
-      expect(getByText("3000")).toBeDefined()
+    await waitForElementToBeRemoved(() => screen.getByTestId("loading-skeleton"))
 
-      fireEvent.changeText(getByTestId("price-min-input"), "300")
-      fireEvent.changeText(getByTestId("price-max-input"), "5000")
+    expect(screen.getByLabelText("Minimum Price Range Input")).toHaveProp("value", "200")
+    expect(screen.getByLabelText("Maximum Price Range Input")).toHaveProp("value", "3000")
 
-      expect(getByText("300")).toBeDefined()
-      expect(getByText("5000")).toBeDefined()
-    })
+    fireEvent.changeText(screen.getByTestId("price-min-input"), "300")
+    fireEvent.changeText(screen.getByTestId("price-max-input"), "5000")
+
+    expect(screen.getByLabelText("Minimum Price Range Input")).toHaveProp("value", "300")
+    expect(screen.getByLabelText("Maximum Price Range Input")).toHaveProp("value", "5000")
   })
 })
 

src/app/Scenes/SavedSearchAlert/Components/SavedSearchFilterRarity.tests.tsx

@@ -1,5 +1,5 @@
 import { OwnerType } from "@artsy/cohesion"
-import { fireEvent, waitFor } from "@testing-library/react-native"
+import { fireEvent } from "@testing-library/react-native"
 import { KNOWN_ATTRIBUTION_CLASS_OPTIONS } from "app/Components/ArtworkFilter/Filters/AttributionClassOptions"
 import { SavedSearchFilterRarity } from "app/Scenes/SavedSearchAlert/Components/SavedSearchFilterRarity"
 import {
@@ -41,20 +41,16 @@ describe("SavedSearchFilterRarity", () => {
     expect(getByText("Open Edition")).toHaveStyle({ color: black100Hex })
   })
 
-  it("Updates selected filters on press", () => {
+  it("Updates selected filters on press", async () => {
     const { getByText } = renderWithWrappers(
       <SavedSearchStoreProvider runtimeModel={initialData}>
         <SavedSearchFilterRarity />
       </SavedSearchStoreProvider>
     )
 
-    expect(getByText("Unique")).toHaveStyle({ color: black100Hex })
-
     fireEvent(getByText("Unique"), "onPress")
 
-    waitFor(() => {
-      expect(getByText("Unique")).not.toHaveStyle({ color: black100Hex })
-    })
+    expect(getByText("Unique")).not.toHaveStyle({ color: black100Hex })
   })
 })
 

src/app/Scenes/SavedSearchAlert/Components/SavedSearchFilterWaysToBuy.tests.tsx

@@ -1,5 +1,5 @@
 import { OwnerType } from "@artsy/cohesion"
-import { fireEvent, waitFor } from "@testing-library/react-native"
+import { fireEvent } from "@testing-library/react-native"
 import { WAYS_TO_BUY_OPTIONS } from "app/Components/ArtworkFilter/Filters/WaysToBuyOptions"
 import { SavedSearchFilterWaysToBuy } from "app/Scenes/SavedSearchAlert/Components/SavedSearchFilterWaysToBuy"
 import {
@@ -58,9 +58,7 @@ describe("SavedSearchFilterWaysToBuy", () => {
 
     fireEvent(getByText("Bid"), "onPress")
 
-    waitFor(() => {
-      expect(getByText("Bid")).not.toHaveStyle({ color: black100Hex })
-    })
+    expect(getByText("Bid")).not.toHaveStyle({ color: black100Hex })
   })
 })
 

src/app/Scenes/SavedSearchAlert/SavedSearchAlertForm.tests.tsx

@@ -1,5 +1,5 @@
 import { OwnerType } from "@artsy/cohesion"
-import { fireEvent, waitFor, waitForElementToBeRemoved } from "@testing-library/react-native"
+import { fireEvent, screen, waitFor } from "@testing-library/react-native"
 import { Aggregations } from "app/Components/ArtworkFilter/ArtworkFilterHelpers"
 import {
   SavedSearchEntity,
@@ -542,7 +542,7 @@ describe("SavedSearchAlertForm", () => {
       expect(getByText("Prints")).toBeTruthy()
     })
 
-    it("should have removable filter pills", () => {
+    it("should have removable filter pills", async () => {
       const { getByText } = renderWithWrappers(<TestRenderer />)
       // artist pill should appear and not be removable
       expect(getByText("artistName")).toBeTruthy()
@@ -551,8 +551,8 @@ describe("SavedSearchAlertForm", () => {
       fireEvent.press(getByText("Prints"))
       fireEvent.press(getByText("Photography"))
 
-      waitForElementToBeRemoved(() => getByText("Prints"))
-      waitForElementToBeRemoved(() => getByText("Photography"))
+      expect(screen.queryByText("Prints")).not.toBeOnTheScreen()
+      expect(screen.queryByText("Photography")).not.toBeOnTheScreen()
     })
   })