fix(fixture): throw correct error when find* query times out on vis…

…ibility
testing-library · Sep 8, 2022 · 348344b · 348344b
1 parent d4611af
commit 348344b
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 4 deletions.
diff --git a/lib/fixture/locator/queries.ts b/lib/fixture/locator/queries.ts
@@ -44,25 +44,37 @@ const createFindQuery =
       )}`,
     )
 
-    const {state = asyncUtilExpectedState, timeout = asyncUtilTimeout} = waitForElementOptions ?? {}
+    const {state: expectedState = asyncUtilExpectedState, timeout = asyncUtilTimeout} =
+      waitForElementOptions ?? {}
 
     try {
-      await locator.first().waitFor({state, timeout})
+      await locator.first().waitFor({state: expectedState, timeout})
     } catch (error) {
       // In the case of a `waitFor` timeout from Playwright, we want to
       // surface the appropriate error from Testing Library, so run the
       // query one more time as `get*` knowing that it will fail with the
       // error that we want the user to see instead of the `TimeoutError`
       if (error instanceof errors.TimeoutError) {
-        return pageOrLocator
+        const timeoutLocator = pageOrLocator
           .locator(
             `${queryToSelector(findQueryToGetQuery(query))}=${JSON.stringify(
               synchronousOptions,
               replacer,
             )}`,
           )
           .first()
-          .waitFor({state, timeout: 100})
+
+        // Handle case where element is attached, but hidden, and the expected
+        // state is set to `visible`. In this case, dereferencing the
+        // `Locator` instance won't throw a `get*` query error, so just
+        // surface the original Playwright timeout error
+        if (expectedState === 'visible' && !(await timeoutLocator.isVisible())) {
+          throw error
+        }
+
+        // In all other cases, dereferencing the `Locator` instance here should
+        // cause the above `get*` query to throw an error in Testing Library
+        return timeoutLocator.waitFor({state: expectedState, timeout})
       }
 
       throw error

diff --git a/test/fixture/locators.test.ts b/test/fixture/locators.test.ts
@@ -203,6 +203,32 @@ test.describe('lib/fixture.ts (locators)', () => {
       )
     })
 
+    test('throws Playwright error when locator times out for visible state (but is attached)', async ({
+      queries,
+    }) => {
+      const query = async () =>
+        queries.findByText(/Hidden/, undefined, {state: 'visible', timeout: 500})
+
+      await expect(query).rejects.toThrowError(
+        expect.objectContaining({
+          message: expect.stringContaining('500'),
+        }),
+      )
+    })
+
+    test('throws Testing Library error when locator times out for attached state', async ({
+      queries,
+    }) => {
+      const query = async () =>
+        queries.findByText(/Loaded!/, undefined, {state: 'attached', timeout: 500})
+
+      await expect(query).rejects.toThrowError(
+        expect.objectContaining({
+          message: expect.stringContaining('TestingLibraryElementError'),
+        }),
+      )
+    })
+
     test('throws Testing Library error when multi-element locator times out', async ({queries}) => {
       const query = async () => queries.findAllByText(/Hello/, undefined, {timeout: 500})