From c49b86759b9237f6a23ccbfa5292497ed5e7b49e Mon Sep 17 00:00:00 2001 From: "Tim J. Baumann" Date: Sun, 31 Oct 2021 23:40:36 +0100 Subject: [PATCH] feat: Improve performance of findBy*/findAllBy* by not generating informative error messages in the function passed to waitFor. The work of generating these error messages almost entirely wasted because all of them, except possibly the last one (if the test is red), will be discarded by waitFor. Instead, if the call to waitFor failed, another attempt to find the element(s) is made, this time generating a useful error message if it also fails. --- src/__tests__/role.js | 9 +++++---- src/queries/label-text.ts | 22 ++++++++++++++++++++++ src/query-helpers.ts | 39 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/src/__tests__/role.js b/src/__tests__/role.js index a87bc000..f1047da2 100644 --- a/src/__tests__/role.js +++ b/src/__tests__/role.js @@ -1,5 +1,6 @@ import {configure, getConfig} from '../config' import {getQueriesForElement} from '../get-queries-for-element' +import {waitFor} from '../wait-for' import {render, renderIntoDocument} from './helpers/test-utils' test('by default logs accessible roles when it fails', () => { @@ -356,11 +357,11 @@ test('does not include the container in the queryable roles', () => { }) test('has no useful error message in findBy', async () => { - const {findByRole} = render(`
  • `) + const {getByRole} = render(`
  • `) - await expect(findByRole('option', {timeout: 1})).rejects.toThrow( - 'Unable to find role="option"', - ) + await expect( + waitFor(() => getByRole('option'), {timeout: 1}), + ).rejects.toThrow('Unable to find role="option"') }) test('explicit role is most specific', () => { diff --git a/src/queries/label-text.ts b/src/queries/label-text.ts index 39e766d5..04dd9624 100644 --- a/src/queries/label-text.ts +++ b/src/queries/label-text.ts @@ -191,12 +191,34 @@ const findAllByLabelText = makeFindQuery( // @ts-expect-error -- See `wrapAllByQueryWithSuggestion` Argument constraint comment [labelText: Matcher, options?: SelectorMatcherOptions] >(getAllByLabelText, getAllByLabelText.name, 'findAll'), + wrapAllByQueryWithSuggestion( + (container, text, options) => { + const elements = queryAllByLabelText(container, text, options) + if (elements.length === 0) { + throw new Error('no element found') + } + return elements + }, + getAllByLabelText.name, + 'findAll', + ), ) const findByLabelText = makeFindQuery( wrapSingleQueryWithSuggestion< // @ts-expect-error -- See `wrapAllByQueryWithSuggestion` Argument constraint comment [labelText: Matcher, options?: SelectorMatcherOptions] >(getByLabelText, getAllByLabelText.name, 'find'), + wrapSingleQueryWithSuggestion( + (container, text, options) => { + const elements = queryAllByLabelText(container, text, options) + if (elements.length !== 1) { + throw new Error('no element or more than one elements found') + } + return elements[0] + }, + getAllByLabelText.name, + 'find', + ), ) const getAllByLabelTextWithSuggestions = wrapAllByQueryWithSuggestion< diff --git a/src/query-helpers.ts b/src/query-helpers.ts index 155210e1..c9bdedeb 100644 --- a/src/query-helpers.ts +++ b/src/query-helpers.ts @@ -118,11 +118,16 @@ function makeGetAllQuery( // this accepts a getter query function and returns a function which calls // waitFor and passing a function which invokes the getter. function makeFindQuery( - getter: ( + getterWithInformativeErrorMessage: ( container: HTMLElement, text: Matcher, options: MatcherOptions, ) => QueryFor, + getterWithQuickToGenerateErrorMessage: ( + container: HTMLElement, + text: Matcher, + options: MatcherOptions, + ) => QueryFor = getterWithInformativeErrorMessage, ) { return ( container: HTMLElement, @@ -132,9 +137,17 @@ function makeFindQuery( ) => { return waitFor( () => { - return getter(container, text, options) + // Don't waste time generating an informative error message since they will be discarded + // anyway. + return getterWithQuickToGenerateErrorMessage(container, text, options) }, {container, ...waitForOptions}, + ).then( + x => x, + () => { + // Try one last time, this time generating an informative error message in case of failure + return getterWithInformativeErrorMessage(container, text, options) + }, ) } } @@ -242,9 +255,31 @@ function buildQueries( const findAllBy = makeFindQuery( wrapAllByQueryWithSuggestion(getAllBy, queryAllBy.name, 'findAll'), + wrapAllByQueryWithSuggestion( + (container, text, options) => { + const elements = queryAllBy(container, text, options) + if (elements.length === 0) { + throw new Error('no element found') + } + return elements + }, + queryAllBy.name, + 'findAll', + ), ) const findBy = makeFindQuery( wrapSingleQueryWithSuggestion(getBy, queryAllBy.name, 'find'), + wrapSingleQueryWithSuggestion( + (container, text, options) => { + const elements = queryAllBy(container, text, options) + if (elements.length !== 1) { + throw new Error('no element or more than one elements found') + } + return elements[0] + }, + queryAllBy.name, + 'find', + ), ) return [