Skip to content

Commit

Permalink
feat: return more data from getSuggestedQuery to support tooling (#608)
Browse files Browse the repository at this point in the history 
* feat: expose suggestion data to support tooling

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.

* feat: update query style to match docs

* chore: add some tests for getSuggestedQuery
  • Loading branch information
@smeijer
smeijer committed Jun 8, 2020
1 parent fb012de commit a029772
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 34 deletions.
102 changes: 72 additions & 30 deletions src/__tests__/suggestions.js
View file
@@ -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})
Expand Down Expand Up @@ -72,7 +72,7 @@ test(`should not suggest if the suggestion would give different results`, () =>
test('should suggest by label over title', () => {
renderIntoDocument(`<label><span>bar</span><input title="foo" /></label>`)

expect(() => screen.getByTitle('foo')).toThrowError(/getByLabelText\("bar"\)/)
expect(() => screen.getByTitle('foo')).toThrowError(/getByLabelText\('bar'\)/)
})

test('should not suggest if there would be mixed suggestions', () => {
Expand All @@ -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 })
<body>
Expand All @@ -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 })
<body>
Expand Down Expand Up @@ -148,18 +148,18 @@ 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 \}\)/,
)
})

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\}\)/,
/getByRole\('img', \{ name: \/incredibles 2 poster\/i \}\)/,
)
})

Expand All @@ -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 \}\)/,
)
})

Expand All @@ -178,7 +178,7 @@ test('should suggest getByLabelText when no role available', () => {
`<label for="foo">Username</label><input data-testid="foo" id="foo" />`,
)
expect(() => screen.getByTestId('foo')).toThrowError(
/getByLabelText\("Username"\)/,
/getByLabelText\('Username'\)/,
)
})

Expand All @@ -191,7 +191,7 @@ test(`should suggest getByLabel on non form elements`, () => {
`)

expect(() => screen.getByTestId('foo')).toThrowError(
/getByLabelText\("Section One"\)/,
/getByLabelText\('Section One'\)/,
)
})

Expand All @@ -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 \}\)/,
)
})

Expand All @@ -230,23 +230,23 @@ test(`should suggest label over placeholder text`, () => {
)

expect(() => screen.getByPlaceholderText('Username')).toThrowError(
/getByLabelText\("Username"\)/,
/getByLabelText\('Username'\)/,
)
})

test(`should suggest getByPlaceholderText`, () => {
renderIntoDocument(`<input data-testid="foo" placeholder="Username" />`)

expect(() => screen.getByTestId('foo')).toThrowError(
/getByPlaceholderText\("Username"\)/,
/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"\)/,
/getByText\('hello there'\)/,
)
})

Expand All @@ -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'\)/,
)
})

Expand All @@ -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'\)/,
)
})

Expand All @@ -285,25 +285,67 @@ test(`should suggest getByTitle`, () => {
</svg>`)

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 <title> 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'\)/)
})

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')`)
})
22 changes: 18 additions & 4 deletions src/suggestions.js
View file
Expand Up @@ -29,14 +29,28 @@ 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}`
const options = queryArgs[1]
? `, { ${Object.entries(queryArgs[1])
.map(([k, v]) => `${k}: ${v}`)
.join(', ')} }`
: ''
return `${variant}By${queryName}("${content}"${options})`
return `${queryMethod}('${content}'${options})`
},
}
}
Expand Down

0 comments on commit a029772

Please sign in to comment.