feat: suggestions for which query to use (#586)

* feat: suggestions for which query to use * coverage bumped * more tests, label text working * more cases for labelText * removed commented out code * added *ByDisplayValue * all queries supported now * cleanup * added types for suggestions * export suggestions from index * fixed a couple lint warnings * Update src/__tests__/suggestions.js Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> * Update src/config.js haha Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> * Update src/__tests__/suggestions.js Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> * Update src/config.js Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> * Update src/suggestions.js Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> * Update src/query-helpers.js * PR feedback * refactor to getLabelFor * formatting * added support for suggest:false * case ignored regex * using full query name for get & getAll & query * suggest on labeltext * suggest on queryAllBy * more tests * rename showSuggs to throwSuggs * PR feedback * matches.d.ts * Update types/matches.d.ts Co-authored-by: Ben Monro <benjamin.monro@walmartlabs.com> Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
testing-library · May 29, 2020 · 839dca3 · 839dca3
1 parent 709044b
commit 839dca3
Show file tree

Hide file tree

Showing 17 changed files with 560 additions and 42 deletions.
diff --git a/src/__tests__/suggestions.js b/src/__tests__/suggestions.js
@@ -0,0 +1,256 @@
+import {configure} from '../config'
+import {screen} from '..'
+import {renderIntoDocument} from './helpers/test-utils'
+
+beforeAll(() => {
+  configure({throwSuggestions: true})
+})
+
+afterAll(() => {
+  configure({throwSuggestions: false})
+})
+
+test('does not suggest when using getByRole', () => {
+  renderIntoDocument(`<button data-testid="foo">submit</button>`)
+
+  expect(() => screen.getByRole('button', {name: /submit/i})).not.toThrowError()
+})
+
+test('should not suggest when nothing available', () => {
+  renderIntoDocument(`<span data-testid="foo" />`)
+
+  expect(() => screen.queryByTestId('foo')).not.toThrowError()
+})
+
+test(`should not suggest if the suggestion would give different results`, () => {
+  renderIntoDocument(`
+    <input type="text" data-testid="foo" /><span data-testid="foo" />
+  `)
+
+  expect(() =>
+    screen.getAllByTestId('foo', {suggest: false}),
+  ).not.toThrowError()
+})
+
+test('should not suggest if there would be mixed suggestions', () => {
+  renderIntoDocument(`
+  <button data-testid="foo">submit</button>
+  <label for="foo">Username</label><input data-testid="foo" id="foo" />`)
+
+  expect(() => screen.getAllByTestId('foo')).not.toThrowError()
+})
+
+test('should not suggest when suggest is turned off for a query', () => {
+  renderIntoDocument(`
+  <button data-testid="foo">submit</button>
+  <button data-testid="foot">another</button>`)
+
+  expect(() => screen.getByTestId('foo', {suggest: false})).not.toThrowError()
+  expect(() =>
+    screen.getAllByTestId(/foo/, {suggest: false}),
+  ).not.toThrowError()
+})
+
+test('should suggest getByRole when used with getBy', () => {
+  renderIntoDocument(`<button data-testid="foo">submit</button>`)
+
+  expect(() => screen.getByTestId('foo')).toThrowErrorMatchingInlineSnapshot(`
+"A better query is available, try this:
+getByRole("button", {name: /submit/i})
+
+
+<body>
+  <button
+    data-testid="foo"
+  >
+    submit
+  </button>
+</body>"
+`)
+})
+
+test('should suggest getAllByRole when used with getAllByTestId', () => {
+  renderIntoDocument(`
+    <button data-testid="foo">submit</button>
+    <button data-testid="foo">submit</button>`)
+
+  expect(() => screen.getAllByTestId('foo'))
+    .toThrowErrorMatchingInlineSnapshot(`
+"A better query is available, try this:
+getAllByRole("button", {name: /submit/i})
+
+
+<body>
+  
+    
+  <button
+    data-testid="foo"
+  >
+    submit
+  </button>
+  
+    
+  <button
+    data-testid="foo"
+  >
+    submit
+  </button>
+</body>"
+`)
+})
+test('should suggest findByRole when used with findByTestId', async () => {
+  renderIntoDocument(`
+  <button data-testid="foo">submit</button>
+  <button data-testid="foot">submit</button>
+  `)
+
+  await expect(screen.findByTestId('foo')).rejects.toThrowError(
+    /findByRole\("button", \{name: \/submit\/i\}\)/,
+  )
+  await expect(screen.findAllByTestId(/foo/)).rejects.toThrowError(
+    /findAllByRole\("button", \{name: \/submit\/i\}\)/,
+  )
+})
+
+test('should suggest img role w/ alt text', () => {
+  renderIntoDocument(`<img data-testid="img" alt="Incredibles 2 Poster"  />`)
+
+  expect(() => screen.getByAltText('Incredibles 2 Poster')).toThrowError(
+    /getByRole\("img", \{name: \/incredibles 2 poster\/i\}\)/,
+  )
+})
+
+test('should suggest getByLabelText when no role available', () => {
+  renderIntoDocument(
+    `<label for="foo">Username</label><input data-testid="foo" id="foo" />`,
+  )
+  expect(() => screen.getByTestId('foo')).toThrowError(
+    /getByLabelText\("Username"\)/,
+  )
+})
+
+test(`should suggest getByLabel on non form elements`, () => {
+  renderIntoDocument(`
+  <div data-testid="foo" aria-labelledby="section-one-header">
+    <span id="section-one-header">Section One</span>
+    <p>some content</p>
+  </div>
+  `)
+
+  expect(() => screen.getByTestId('foo')).toThrowError(
+    /getByLabelText\("Section One"\)/,
+  )
+})
+
+test.each([
+  `<label id="username-label">Username</label><input aria-labelledby="username-label" type="text" />`,
+  `<label><span>Username</span><input type="text" /></label>`,
+  `<label for="foo">Username</label><input id="foo" type="text" />`,
+])('%s\nshould suggest getByRole over', async html => {
+  renderIntoDocument(html)
+
+  expect(() => screen.getByLabelText('Username')).toThrowError(
+    /getByRole\("textbox", \{name: \/username\/i\}\)/,
+  )
+  expect(() => screen.getAllByLabelText('Username')).toThrowError(
+    /getAllByRole\("textbox", \{name: \/username\/i\}\)/,
+  )
+
+  expect(() => screen.queryByLabelText('Username')).toThrowError(
+    /queryByRole\("textbox", \{name: \/username\/i\}\)/,
+  )
+  expect(() => screen.queryAllByLabelText('Username')).toThrowError(
+    /queryAllByRole\("textbox", \{name: \/username\/i\}\)/,
+  )
+
+  await expect(screen.findByLabelText('Username')).rejects.toThrowError(
+    /findByRole\("textbox", \{name: \/username\/i\}\)/,
+  )
+  await expect(screen.findAllByLabelText(/Username/)).rejects.toThrowError(
+    /findAllByRole\("textbox", \{name: \/username\/i\}\)/,
+  )
+})
+
+test(`should suggest label over placeholder text`, () => {
+  renderIntoDocument(
+    `<label for="foo">Username</label><input id="foo" data-testid="foo" placeholder="Username" />`,
+  )
+
+  expect(() => screen.getByPlaceholderText('Username')).toThrowError(
+    /getByLabelText\("Username"\)/,
+  )
+})
+
+test(`should suggest getByPlaceholderText`, () => {
+  renderIntoDocument(`<input data-testid="foo" placeholder="Username" />`)
+
+  expect(() => screen.getByTestId('foo')).toThrowError(
+    /getByPlaceholderText\("Username"\)/,
+  )
+})
+
+test(`should suggest getByText for simple elements`, () => {
+  renderIntoDocument(`<div data-testid="foo">hello there</div>`)
+
+  expect(() => screen.getByTestId('foo')).toThrowError(
+    /getByText\("hello there"\)/,
+  )
+})
+
+test(`should suggest getByDisplayValue`, () => {
+  renderIntoDocument(`<input id="lastName" data-testid="lastName" />`)
+
+  document.getElementById('lastName').value = 'Prine' // RIP John Prine
+
+  expect(() => screen.getByTestId('lastName')).toThrowError(
+    /getByDisplayValue\("Prine"\)/,
+  )
+})
+
+test(`should suggest getByAltText`, () => {
+  renderIntoDocument(`
+    <input data-testid="input" alt="last name" />
+    <map name="workmap">
+      <area data-testid="area" shape="rect" coords="34,44,270,350" alt="Computer">
+    </map>
+    `)
+
+  expect(() => screen.getByTestId('input')).toThrowError(
+    /getByAltText\("last name"\)/,
+  )
+  expect(() => screen.getByTestId('area')).toThrowError(
+    /getByAltText\("Computer"\)/,
+  )
+})
+
+test(`should suggest getByTitle`, () => {
+  renderIntoDocument(`
+  <span title="Delete" data-testid="delete"></span>
+  <svg>
+    <title data-testid="svg">Close</title>
+    <g><path /></g>
+  </svg>`)
+
+  expect(() => screen.getByTestId('delete')).toThrowError(
+    /getByTitle\("Delete"\)/,
+  )
+  expect(() => screen.getAllByTestId('delete')).toThrowError(
+    /getAllByTitle\("Delete"\)/,
+  )
+  expect(() => screen.queryByTestId('delete')).toThrowError(
+    /queryByTitle\("Delete"\)/,
+  )
+  expect(() => screen.queryAllByTestId('delete')).toThrowError(
+    /queryAllByTitle\("Delete"\)/,
+  )
+  expect(() => screen.queryAllByTestId('delete')).toThrowError(
+    /queryAllByTitle\("Delete"\)/,
+  )
+  expect(() => screen.queryAllByTestId('delete')).toThrowError(
+    /queryAllByTitle\("Delete"\)/,
+  )
+
+  // Since `ByTitle` and `ByText` will both return the <title> element
+  // `getByText` will always be the suggested query as it is higher up the list.
+  expect(() => screen.getByTestId('svg')).toThrowError(/getByText\("Close"\)/)
+})
diff --git a/src/config.js b/src/config.js
@@ -16,9 +16,12 @@ let config = {
   asyncWrapper: cb => cb(),
   // default value for the `hidden` option in `ByRole` queries
   defaultHidden: false,
-  //showOriginalStackTrace flag to show the full error stack traces for async errors
+  // showOriginalStackTrace flag to show the full error stack traces for async errors
   showOriginalStackTrace: false,
 
+  // throw errors w/ suggestions for better queries. Opt in so off by default.
+  throwSuggestions: false,
+
   // called when getBy* queries fail. (message, container) => Error
   getElementError(message, container) {
     const error = new Error(

diff --git a/src/index.js b/src/index.js
@@ -16,6 +16,7 @@ export * from './query-helpers'
 export {getRoles, logRoles, isInaccessible} from './role-helpers'
 export * from './pretty-dom'
 export {configure} from './config'
+export * from './suggestions'
 
 export {
   // "within" reads better in user-code

diff --git a/src/queries/alt-text.js b/src/queries/alt-text.js
@@ -1,4 +1,5 @@
 import {matches, fuzzyMatches, makeNormalizer, buildQueries} from './all-utils'
+import {wrapAllByQueryWithSuggestion} from '../query-helpers'
 
 function queryAllByAltText(
   container,
@@ -16,6 +17,12 @@ const getMultipleError = (c, alt) =>
   `Found multiple elements with the alt text: ${alt}`
 const getMissingError = (c, alt) =>
   `Unable to find an element with the alt text: ${alt}`
+
+const queryAllByAltTextWithSuggestions = wrapAllByQueryWithSuggestion(
+  queryAllByAltText,
+  queryAllByAltText.name,
+  'queryAll',
+)
 const [
   queryByAltText,
   getAllByAltText,
@@ -26,7 +33,7 @@ const [
 
 export {
   queryByAltText,
-  queryAllByAltText,
+  queryAllByAltTextWithSuggestions as queryAllByAltText,
   getByAltText,
   getAllByAltText,
   findAllByAltText,

diff --git a/src/queries/display-value.js b/src/queries/display-value.js
@@ -5,6 +5,7 @@ import {
   makeNormalizer,
   buildQueries,
 } from './all-utils'
+import {wrapAllByQueryWithSuggestion} from '../query-helpers'
 
 function queryAllByDisplayValue(
   container,
@@ -33,6 +34,13 @@ const getMultipleError = (c, value) =>
   `Found multiple elements with the display value: ${value}.`
 const getMissingError = (c, value) =>
   `Unable to find an element with the display value: ${value}.`
+
+const queryAllByDisplayValueWithSuggestions = wrapAllByQueryWithSuggestion(
+  queryAllByDisplayValue,
+  queryAllByDisplayValue.name,
+  'queryAll',
+)
+
 const [
   queryByDisplayValue,
   getAllByDisplayValue,
@@ -43,7 +51,7 @@ const [
 
 export {
   queryByDisplayValue,
-  queryAllByDisplayValue,
+  queryAllByDisplayValueWithSuggestions as queryAllByDisplayValue,
   getByDisplayValue,
   getAllByDisplayValue,
   findAllByDisplayValue,