From 5cea4fbc6cb33ae60a88de7b87635e4c6c6382b6 Mon Sep 17 00:00:00 2001 From: Stephan Meijer Date: Sun, 7 Jun 2020 19:24:04 +0200 Subject: [PATCH 1/3] feat: expose suggestion data to support tooling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I've added more data to the returned object, to support the development of tooling. Before this change, the returned value would be something like `{ queryName: 'Role' }`, and only by calling the `toString` method, the caller would get more information (the full formatted expression). With this change, the caller gets more useful data, to build tooling around. Example result: ``` { queryName: 'Role', queryMethod: 'getByRole', queryArgs: ['button', { name: /submit/i }], variant: 'get' } ``` The variant now defaults to `get`, to fix a bug that was causing suggestions like `undefinedByRole(…)` to be generated. --- src/suggestions.js | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/suggestions.js b/src/suggestions.js index d67b373b..5301cb20 100644 --- a/src/suggestions.js +++ b/src/suggestions.js @@ -29,16 +29,29 @@ function getLabelTextFor(element) { function escapeRegExp(string) { return string.replace(/[.*+\-?^${}()|[\]\\]/g, '\\$&') // $& means the whole matched string } -function makeSuggestion(queryName, content, {variant, name}) { + +function makeSuggestion(queryName, content, { + variant = 'get', + name +}) { + const queryArgs = [content]; + + if (name) { + queryArgs.push({ name: new RegExp(escapeRegExp(name.toLowerCase()), 'i') }) + } + + const queryMethod = `${variant}By${queryName}`; + return { queryName, + queryMethod, + queryArgs, + variant, toString() { - const options = name - ? `, {name: /${escapeRegExp(name.toLowerCase())}/i}` - : '' - return `${variant}By${queryName}("${content}"${options})` - }, - } + const options = queryArgs[1] ? `, { ${Object.entries(queryArgs[1]).map(([k, v]) => `${k}: ${v}`).join(', ')} }` : ''; + return `${queryMethod}("${content}"${options})`; + } + }; } export function getSuggestedQuery(element, variant) { From 72efd82565bb8bcb8ea63857e3e932c291966454 Mon Sep 17 00:00:00 2001 From: Stephan Meijer Date: Sun, 7 Jun 2020 20:18:01 +0200 Subject: [PATCH 2/3] feat: update query style to match docs --- src/__tests__/suggestions.js | 56 ++++++++++++++++++------------------ src/suggestions.js | 23 ++++++++------- 2 files changed, 40 insertions(+), 39 deletions(-) diff --git a/src/__tests__/suggestions.js b/src/__tests__/suggestions.js index e0ecd51c..6f6deec0 100644 --- a/src/__tests__/suggestions.js +++ b/src/__tests__/suggestions.js @@ -72,7 +72,7 @@ test(`should not suggest if the suggestion would give different results`, () => test('should suggest by label over title', () => { renderIntoDocument(``) - expect(() => screen.getByTitle('foo')).toThrowError(/getByLabelText\("bar"\)/) + expect(() => screen.getByTitle('foo')).toThrowError(/getByLabelText\('bar'\)/) }) test('should not suggest if there would be mixed suggestions', () => { @@ -99,7 +99,7 @@ test('should suggest getByRole when used with getBy', () => { expect(() => screen.getByTestId('foo')).toThrowErrorMatchingInlineSnapshot(` "A better query is available, try this: -getByRole("button", {name: /submit/i}) +getByRole('button', { name: /submit/i }) @@ -120,7 +120,7 @@ test('should suggest getAllByRole when used with getAllByTestId', () => { expect(() => screen.getAllByTestId('foo')) .toThrowErrorMatchingInlineSnapshot(` "A better query is available, try this: -getAllByRole("button", {name: /submit/i}) +getAllByRole('button', { name: /submit/i }) @@ -148,10 +148,10 @@ test('should suggest findByRole when used with findByTestId', async () => { `) await expect(screen.findByTestId('foo')).rejects.toThrowError( - /findByRole\("button", \{name: \/submit\/i\}\)/, + /findByRole\('button', \{ name: \/submit\/i \}\)/, ) await expect(screen.findAllByTestId(/foo/)).rejects.toThrowError( - /findAllByRole\("button", \{name: \/submit\/i\}\)/, + /findAllByRole\('button', \{ name: \/submit\/i \}\)/, ) }) @@ -159,7 +159,7 @@ test('should suggest img role w/ alt text', () => { renderIntoDocument(`Incredibles 2 Poster`) expect(() => screen.getByAltText('Incredibles 2 Poster')).toThrowError( - /getByRole\("img", \{name: \/incredibles 2 poster\/i\}\)/, + /getByRole\('img', \{ name: \/incredibles 2 poster\/i \}\)/, ) }) @@ -169,7 +169,7 @@ test('escapes regular expressions in suggestion', () => { ) expect(() => screen.getByTestId('foo')).toThrowError( - /getByRole\("img", \{name: \/the problem \\\(picture of a question mark\\\)\/i\}\)/, + /getByRole\('img', \{ name: \/the problem \\\(picture of a question mark\\\)\/i \}\)/, ) }) @@ -178,7 +178,7 @@ test('should suggest getByLabelText when no role available', () => { ``, ) expect(() => screen.getByTestId('foo')).toThrowError( - /getByLabelText\("Username"\)/, + /getByLabelText\('Username'\)/, ) }) @@ -191,7 +191,7 @@ test(`should suggest getByLabel on non form elements`, () => { `) expect(() => screen.getByTestId('foo')).toThrowError( - /getByLabelText\("Section One"\)/, + /getByLabelText\('Section One'\)/, ) }) @@ -203,24 +203,24 @@ test.each([ renderIntoDocument(html) expect(() => screen.getByLabelText('Username')).toThrowError( - /getByRole\("textbox", \{name: \/username\/i\}\)/, + /getByRole\('textbox', \{ name: \/username\/i \}\)/, ) expect(() => screen.getAllByLabelText('Username')).toThrowError( - /getAllByRole\("textbox", \{name: \/username\/i\}\)/, + /getAllByRole\('textbox', \{ name: \/username\/i \}\)/, ) expect(() => screen.queryByLabelText('Username')).toThrowError( - /queryByRole\("textbox", \{name: \/username\/i\}\)/, + /queryByRole\('textbox', \{ name: \/username\/i \}\)/, ) expect(() => screen.queryAllByLabelText('Username')).toThrowError( - /queryAllByRole\("textbox", \{name: \/username\/i\}\)/, + /queryAllByRole\('textbox', \{ name: \/username\/i \}\)/, ) await expect(screen.findByLabelText('Username')).rejects.toThrowError( - /findByRole\("textbox", \{name: \/username\/i\}\)/, + /findByRole\('textbox', \{ name: \/username\/i \}\)/, ) await expect(screen.findAllByLabelText(/Username/)).rejects.toThrowError( - /findAllByRole\("textbox", \{name: \/username\/i\}\)/, + /findAllByRole\('textbox', \{ name: \/username\/i \}\)/, ) }) @@ -230,7 +230,7 @@ test(`should suggest label over placeholder text`, () => { ) expect(() => screen.getByPlaceholderText('Username')).toThrowError( - /getByLabelText\("Username"\)/, + /getByLabelText\('Username'\)/, ) }) @@ -238,7 +238,7 @@ test(`should suggest getByPlaceholderText`, () => { renderIntoDocument(``) expect(() => screen.getByTestId('foo')).toThrowError( - /getByPlaceholderText\("Username"\)/, + /getByPlaceholderText\('Username'\)/, ) }) @@ -246,7 +246,7 @@ test(`should suggest getByText for simple elements`, () => { renderIntoDocument(`
hello there
`) expect(() => screen.getByTestId('foo')).toThrowError( - /getByText\("hello there"\)/, + /getByText\('hello there'\)/, ) }) @@ -256,7 +256,7 @@ test(`should suggest getByDisplayValue`, () => { document.getElementById('lastName').value = 'Prine' // RIP John Prine expect(() => screen.getByTestId('lastName')).toThrowError( - /getByDisplayValue\("Prine"\)/, + /getByDisplayValue\('Prine'\)/, ) }) @@ -269,10 +269,10 @@ test(`should suggest getByAltText`, () => { `) expect(() => screen.getByTestId('input')).toThrowError( - /getByAltText\("last name"\)/, + /getByAltText\('last name'\)/, ) expect(() => screen.getByTestId('area')).toThrowError( - /getByAltText\("Computer"\)/, + /getByAltText\('Computer'\)/, ) }) @@ -285,25 +285,25 @@ test(`should suggest getByTitle`, () => { `) expect(() => screen.getByTestId('delete')).toThrowError( - /getByTitle\("Delete"\)/, + /getByTitle\('Delete'\)/, ) expect(() => screen.getAllByTestId('delete')).toThrowError( - /getAllByTitle\("Delete"\)/, + /getAllByTitle\('Delete'\)/, ) expect(() => screen.queryByTestId('delete')).toThrowError( - /queryByTitle\("Delete"\)/, + /queryByTitle\('Delete'\)/, ) expect(() => screen.queryAllByTestId('delete')).toThrowError( - /queryAllByTitle\("Delete"\)/, + /queryAllByTitle\('Delete'\)/, ) expect(() => screen.queryAllByTestId('delete')).toThrowError( - /queryAllByTitle\("Delete"\)/, + /queryAllByTitle\('Delete'\)/, ) expect(() => screen.queryAllByTestId('delete')).toThrowError( - /queryAllByTitle\("Delete"\)/, + /queryAllByTitle\('Delete'\)/, ) // Since `ByTitle` and `ByText` will both return the element // `getByText` will always be the suggested query as it is higher up the list. - expect(() => screen.getByTestId('svg')).toThrowError(/getByText\("Close"\)/) + expect(() => screen.getByTestId('svg')).toThrowError(/getByText\('Close'\)/) }) diff --git a/src/suggestions.js b/src/suggestions.js index 5301cb20..ff960d68 100644 --- a/src/suggestions.js +++ b/src/suggestions.js @@ -30,17 +30,14 @@ function escapeRegExp(string) { return string.replace(/[.*+\-?^${}()|[\]\\]/g, '\\$&') // $& means the whole matched string } -function makeSuggestion(queryName, content, { - variant = 'get', - name -}) { - const queryArgs = [content]; +function makeSuggestion(queryName, content, {variant = 'get', name}) { + const queryArgs = [content] if (name) { - queryArgs.push({ name: new RegExp(escapeRegExp(name.toLowerCase()), 'i') }) + queryArgs.push({name: new RegExp(escapeRegExp(name.toLowerCase()), 'i')}) } - const queryMethod = `${variant}By${queryName}`; + const queryMethod = `${variant}By${queryName}` return { queryName, @@ -48,10 +45,14 @@ function makeSuggestion(queryName, content, { queryArgs, variant, toString() { - const options = queryArgs[1] ? `, { ${Object.entries(queryArgs[1]).map(([k, v]) => `${k}: ${v}`).join(', ')} }` : ''; - return `${queryMethod}("${content}"${options})`; - } - }; + const options = queryArgs[1] + ? `, { ${Object.entries(queryArgs[1]) + .map(([k, v]) => `${k}: ${v}`) + .join(', ')} }` + : '' + return `${queryMethod}('${content}'${options})` + }, + } } export function getSuggestedQuery(element, variant) { From fcdf49b251b3272915d60e63b991ada4f8981ba3 Mon Sep 17 00:00:00 2001 From: Stephan Meijer <stephan.meijer@gmail.com> Date: Mon, 8 Jun 2020 10:28:15 +0200 Subject: [PATCH 3/3] chore: add some tests for getSuggestedQuery --- src/__tests__/suggestions.js | 46 ++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/src/__tests__/suggestions.js b/src/__tests__/suggestions.js index 6f6deec0..581f3994 100644 --- a/src/__tests__/suggestions.js +++ b/src/__tests__/suggestions.js @@ -1,6 +1,6 @@ import {configure} from '../config' -import {screen} from '..' -import {renderIntoDocument} from './helpers/test-utils' +import {screen, getSuggestedQuery} from '..' +import {renderIntoDocument, render} from './helpers/test-utils' beforeAll(() => { configure({throwSuggestions: true}) @@ -307,3 +307,45 @@ test(`should suggest getByTitle`, () => { // `getByText` will always be the suggested query as it is higher up the list. expect(() => screen.getByTestId('svg')).toThrowError(/getByText\('Close'\)/) }) + +test('getSuggestedQuery handles `variant` and defaults to `get`', () => { + const button = render(`<button>submit</button>`).container.firstChild + + expect(getSuggestedQuery(button).toString()).toMatch(/getByRole/) + expect(getSuggestedQuery(button, 'get').toString()).toMatch(/getByRole/) + expect(getSuggestedQuery(button, 'getAll').toString()).toMatch(/getAllByRole/) + expect(getSuggestedQuery(button, 'query').toString()).toMatch(/queryByRole/) + expect(getSuggestedQuery(button, 'queryAll').toString()).toMatch( + /queryAllByRole/, + ) + expect(getSuggestedQuery(button, 'find').toString()).toMatch(/findByRole/) + expect(getSuggestedQuery(button, 'findAll').toString()).toMatch( + /findAllByRole/, + ) +}) + +test('getSuggestedQuery returns rich data for tooling', () => { + const button = render(`<button>submit</button>`).container.firstChild + + expect(getSuggestedQuery(button)).toMatchObject({ + queryName: 'Role', + queryMethod: 'getByRole', + queryArgs: ['button', {name: /submit/i}], + variant: 'get', + }) + + expect(getSuggestedQuery(button).toString()).toEqual( + `getByRole('button', { name: /submit/i })`, + ) + + const div = render(`<a>cancel</a>`).container.firstChild + + expect(getSuggestedQuery(div)).toMatchObject({ + queryName: 'Text', + queryMethod: 'getByText', + queryArgs: ['cancel'], + variant: 'get', + }) + + expect(getSuggestedQuery(div).toString()).toEqual(`getByText('cancel')`) +})