From 12c7ed1193577dcb4fe4c084b85735ddbfd2bc9d Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Fri, 23 Mar 2018 07:57:21 -0600 Subject: [PATCH] feat(render): add new query capabilities for improved tests (#17) **What**: Add the following methods - queryByText - getByText - queryByPlaceholderText - getByPlaceholderText - queryByLabelText - getByLabelText **Why**: Closes #16 These will really improve the usability of this module. These also align much better with the guiding principles :+1: **How**: - Created a `queries.js` file where we have all the logic for the queries and their associated getter functions - Migrate tests where it makes sense - Update docs considerably. **Checklist**: * [x] Documentation * [x] Tests * [x] Ready to be merged * [ ] Added myself to contributors table N/A --- README.md | 225 +++++++++++++++--- package.json | 1 + .../__snapshots__/element-queries.js.snap | 24 +- src/__tests__/__snapshots__/fetch.js.snap | 8 +- src/__tests__/element-queries.js | 73 ++++-- src/__tests__/fetch.js | 14 +- src/__tests__/forms.js | 60 +++++ src/__tests__/mock.react-transition-group.js | 16 +- src/__tests__/react-redux.js | 22 +- src/__tests__/react-router.js | 12 +- .../shallow.react-transition-group.js | 10 +- src/__tests__/stopwatch.js | 10 +- src/__tests__/text-matchers.js | 25 ++ src/index.js | 30 +-- src/queries.js | 143 +++++++++++ typings/index.d.ts | 6 + 16 files changed, 541 insertions(+), 138 deletions(-) create mode 100644 src/__tests__/forms.js create mode 100644 src/__tests__/text-matchers.js create mode 100644 src/queries.js diff --git a/README.md b/README.md index 9aca0618..41f85a7d 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,8 @@ components. It provides light utility functions on top of `react-dom` and * [`Simulate`](#simulate) * [`flushPromises`](#flushpromises) * [`render`](#render) -* [More on `data-testid`s](#more-on-data-testids) +* [`TextMatch`](#textmatch) +* [`query` APIs](#query-apis) * [Examples](#examples) * [FAQ](#faq) * [Other Solutions](#other-solutions) @@ -76,7 +77,7 @@ This library has a `peerDependencies` listing for `react-dom`. import React from 'react' import {render, Simulate, flushPromises} from 'react-testing-library' import axiosMock from 'axios' -import Fetch from '../fetch' +import Fetch from '../fetch' // see the tests for a full implementation test('Fetch makes an API call and displays the greeting when load-greeting is clicked', async () => { // Arrange @@ -86,10 +87,10 @@ test('Fetch makes an API call and displays the greeting when load-greeting is cl }), ) const url = '/greeting' - const {getByTestId, container} = render() + const {getByText, getByTestId, container} = render() // Act - Simulate.click(getByTestId('load-greeting')) + Simulate.click(getByText('Load Greeting')) // let's wait for our mocked `get` request promise to resolve await flushPromises() @@ -146,39 +147,115 @@ unmount() // your component has been unmounted and now: container.innerHTML === '' ``` +#### `getByLabelText(text: TextMatch, options: {selector: string = '*'}): HTMLElement` + +This will search for the label that matches the given [`TextMatch`](#textmatch), +then find the element associated with that label. + +```javascript +const inputNode = getByLabelText('Username') + +// this would find the input node for the following DOM structures: +// The "for" attribute (NOTE: in JSX with React you'll write "htmlFor" rather than "for") +// +// +// +// The aria-labelledby attribute +// +// +// +// Wrapper labels +// +// +// It will NOT find the input node for this: +// +// +// For this case, you can provide a `selector` in the options: +const inputNode = getByLabelText('username-input', {selector: 'input'}) +// and that would work +``` + +> Note: This method will throw an error if it cannot find the node. If you don't +> want this behavior (for example you wish to assert that it doesn't exist), +> then use `queryByLabelText` instead. + +#### `getByPlaceholderText(text: TextMatch): HTMLElement` + +This will search for all elements with a placeholder attribute and find one +that matches the given [`TextMatch`](#textmatch). + +```javascript +// +const inputNode = getByPlaceholderText('Username') +``` + +> NOTE: a placeholder is not a good substitute for a label so you should +> generally use `getByLabelText` instead. + +#### `getByText(text: TextMatch): HTMLElement` + +This will search for all elements that have a text node with `textContent` +matching the given [`TextMatch`](#textmatch). + +```javascript +// About ℹ️ +const aboutAnchorNode = getByText('about') +``` + #### `getByTestId` -A shortcut to `` container.querySelector(`[data-testid="${yourId}"]`) `` except -that it will throw an Error if no matching element is found. Read more about -`data-testid`s below. +A shortcut to `` container.querySelector(`[data-testid="${yourId}"]`) ``. ```javascript +// const usernameInputElement = getByTestId('username-input') -usernameInputElement.value = 'new value' -Simulate.change(usernameInputElement) ``` -#### `queryByTestId` +> In the spirit of [the guiding principles](#guiding-principles), it is +> recommended to use this only after `getByLabel`, `getByPlaceholderText` or +> `getByText` don't work for your use case. Using data-testid attributes do +> not resemble how your software is used and should be avoided if possible. +> That said, they are _way_ better than querying based on DOM structure. +> Learn more about `data-testid`s from the blog post +> ["Making your UI tests resilient to change"][data-testid-blog-post] -A shortcut to `` container.querySelector(`[data-testid="${yourId}"]`) `` -(Note: just like `querySelector`, this could return null if no matching element -is found, which may lead to harder-to-understand error messages). Read more about -`data-testid`s below. +## `TextMatch` + +Several APIs accept a `TextMatch` which can be a `string`, `regex` or a +`function` which returns `true` for a match and `false` for a mismatch. + +Here's an example ```javascript -// assert something doesn't exist -// (you couldn't do this with `getByTestId`) -expect(queryByTestId('username-input')).toBeNull() +//
Hello World
+// all of the following will find the div +getByText('Hello World') // full match +getByText('llo worl') // substring match +getByText('hello world') // strings ignore case +getByText(/Hello W?oRlD/i) // regex +getByText((content, element) => content.startsWith('Hello')) // function + +// all of the following will NOT find the div +getByText('Goodbye World') // non-string match +getByText(/hello world/) // case-sensitive regex with different case +// function looking for a span when it's actually a div +getByText((content, element) => { + return element.tagName.toLowerCase() === 'span' && content.startsWith('Hello') +}) ``` -## More on `data-testid`s +## `query` APIs -The `getByTestId` and `queryByTestId` utilities refer to the practice of using `data-testid` -attributes to identify individual elements in your rendered component. This is -one of the practices this library is intended to encourage. +Each of the `get` APIs listed in [the `render`](#render) section above have a +complimentary `query` API. The `get` APIs will throw errors if a proper node +cannot be found. This is normally the desired effect. However, if you want to +make an assertion that an element is _not_ present in the DOM, then you can use +the `query` API instead: -Learn more about this practice in the blog post: -["Making your UI tests resilient to change"](https://blog.kentcdodds.com/making-your-ui-tests-resilient-to-change-d37a6ee37269) +```javascript +const submitButton = queryByText('submit') +expect(submitButton).toBeNull() // it doesn't exist +``` ## Examples @@ -193,7 +270,61 @@ Feel free to contribute more! ## FAQ -**How do I update the props of a rendered component?** +
+ +Which get method should I use? + +Based on [the Guiding Principles](#guiding-principles), your test should +resemble how your code (component, page, etc.) as much as possible. With this +in mind, we recommend this order of priority: + +1. `getByLabelText`: Only really good for form fields, but this is the number 1 + method a user finds those elements, so it should be your top preference. +2. `getByPlaceholderText`: [A placeholder is not a substitute for a label](https://www.nngroup.com/articles/form-design-placeholders/). + But if that's all you have, then it's better than alternatives. +3. `getByText`: Not useful for forms, but this is the number 1 method a user + finds other elements (like buttons to click), so it should be your top + preference for non-form elements. +4. `getByTestId`: The user cannot see (or hear) these, so this is only + recommended for cases where you can't match by text or it doesn't make sense + (the text is dynamic). + +Other than that, you can also use the `container` to query the rendered +component as well (using the regular +[`querySelector` API](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector)). + +
+ +
+ +Can I write unit tests with this library? + +Definitely yes! You can write unit and integration tests with this library. +See below for more on how to mock dependencies (because this library +intentionally does NOT support shallow rendering) if you want to unit test a +high level component. The tests in this project show several examples of +unit testing with this library. + +As you write your tests, keep in mind: + +> The more your tests resemble the way your software is used, the more confidence they can give you. - [17 Feb 2018][guiding-principle] + +
+ +
+ +What if my app is localized and I don't have access to the text in test? + +This is fairly common. Our first bit of advice is to try to get the default +text used in your tests. That will make everything much easier (more than just +using this utility). If that's not possible, then you're probably best +to just stick with `data-testid`s (which is not bad anyway). + +
+ +
+ +How do I update the props of a rendered component? It'd probably be better if you test the component that's doing the prop updating to ensure that the props are being updated correctly (see @@ -215,7 +346,11 @@ expect(getByTestId('number-display').textContent).toBe('2') [Open the tests](https://github.com/kentcdodds/react-testing-library/blob/master/src/__tests__/number-display.js) for a full example of this. -**If I can't use shallow rendering, how do I mock out components in tests?** +
+ +
+ +If I can't use shallow rendering, how do I mock out components in tests? In general, you should avoid mocking out components (see [the Guiding Principles section](#guiding-principles)). However if you need to, @@ -265,7 +400,11 @@ something more Learn more about how Jest mocks work from my blog post: ["But really, what is a JavaScript mock?"](https://blog.kentcdodds.com/but-really-what-is-a-javascript-mock-10d060966f7d) -**What if I want to verify that an element does NOT exist?** +
+ +
+ +What if I want to verify that an element does NOT exist? You typically will get access to rendered elements using the `getByTestId` utility. However, that function will throw an error if the element isn't found. If you want to specifically test for the absence of an element, then you should use the `queryByTestId` utility which will return the element if found or `null` if not. @@ -273,7 +412,11 @@ You typically will get access to rendered elements using the `getByTestId` utili expect(queryByTestId('thing-that-does-not-exist')).toBeNull() ``` -**I don't want to use `data-testid` attributes for everything. Do I have to?** +
+ +
+ +I really don't like data-testids, but none of the other queries make sense. Do I have to use a data-testid? Definitely not. That said, a common reason people don't like the `data-testid` attribute is they're concerned about shipping that to production. I'd suggest @@ -298,7 +441,11 @@ const allLisInDiv = container.querySelectorAll('div li') const rootElement = container.firstChild ``` -**What if I’m iterating over a list of items that I want to put the data-testid="item" attribute on. How do I distinguish them from each other?** +
+ +
+ +What if I’m iterating over a list of items that I want to put the data-testid="item" attribute on. How do I distinguish them from each other? You can make your selector just choose the one you want by including :nth-child in the selector. @@ -322,8 +469,12 @@ const {getByTestId} = render(/* your component with the items */) const thirdItem = getByTestId(`item-${items[2].id}`) ``` -**What about enzyme is "bloated with complexity and features" and "encourage poor testing -practices"?** +
+ +
+ +What about enzyme is "bloated with complexity and features" and "encourage +poor testing practices"? Most of the damaging features have to do with encouraging testing implementation details. Primarily, these are @@ -334,7 +485,7 @@ state/properties) (most of enzyme's wrapper APIs allow this). The guiding principle for this library is: -> The less your tests resemble the way your software is used, the less confidence they can give you. - [17 Feb 2018](https://twitter.com/kentcdodds/status/965052178267176960) +> The more your tests resemble the way your software is used, the more confidence they can give you. - [17 Feb 2018][guiding-principle] Because users can't directly interact with your app's component instances, assert on their internal state or what components they render, or call their @@ -345,7 +496,11 @@ That's not to say that there's never a use case for doing those things, so they should be possible to accomplish, just not the default and natural way to test react components. -**How does `flushPromises` work and why would I need it?** +
+ +
+ +How does flushPromises work and why would I need it? As mentioned [before](#flushpromises), `flushPromises` uses [`setImmediate`][set-immediate] to schedule resolving a promise after any pending @@ -366,6 +521,8 @@ that this is only effective if you've mocked out your async requests to resolve immediately (like the `axios` mock we have in the examples). It will not `await` for promises that are not already resolved by the time you attempt to flush them. +
+ ## Other Solutions In preparing this project, @@ -378,7 +535,7 @@ this one instead. ## Guiding Principles -> [The less your tests resemble the way your software is used, the less confidence they can give you.](https://twitter.com/kentcdodds/status/965052178267176960) +> [The more your tests resemble the way your software is used, the more confidence they can give you.][guiding-principle] We try to only expose methods and utilities that encourage you to write tests that closely resemble how your react components are used. @@ -443,3 +600,5 @@ MIT [emojis]: https://github.com/kentcdodds/all-contributors#emoji-key [all-contributors]: https://github.com/kentcdodds/all-contributors [set-immediate]: https://developer.mozilla.org/en-US/docs/Web/API/Window/setImmediate +[guiding-principle]: https://twitter.com/kentcdodds/status/977018512689455106 +[data-testid-blog-post]: https://blog.kentcdodds.com/making-your-ui-tests-resilient-to-change-d37a6ee37269 diff --git a/package.json b/package.json index 35939e06..06c161a7 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "@types/react-dom": "^16.0.4", "axios": "^0.18.0", "history": "^4.7.2", + "jest-in-case": "^1.0.2", "kcd-scripts": "^0.36.1", "react": "^16.2.0", "react-dom": "^16.2.0", diff --git a/src/__tests__/__snapshots__/element-queries.js.snap b/src/__tests__/__snapshots__/element-queries.js.snap index 722e413e..ccbe9bc6 100644 --- a/src/__tests__/__snapshots__/element-queries.js.snap +++ b/src/__tests__/__snapshots__/element-queries.js.snap @@ -1,15 +1,13 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`getByTestId finds matching element 1`] = ` - -`; - -exports[`getByTestId throws error when no matching element exists 1`] = `"Unable to find element by [data-testid=\\"unknown-data-testid\\"]"`; - -exports[`queryByTestId finds matching element 1`] = ` - -`; +exports[`get throws a useful error message 1`] = `"Unable to find a label with the text of: LucyRicardo"`; + +exports[`get throws a useful error message 2`] = `"Unable to find an element with the placeholder text of: LucyRicardo"`; + +exports[`get throws a useful error message 3`] = `"Unable to find an element with the text: LucyRicardo. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible."`; + +exports[`get throws a useful error message 4`] = `"Unable to find an element by: [data-testid=\\"LucyRicardo\\"]"`; + +exports[`label with no form control 1`] = `"Found a label with the text of: alone, however no form control was found associated to that label. Make sure you're using the \\"for\\" attribute or \\"aria-labelledby\\" attribute correctly."`; + +exports[`totally empty label 1`] = `"Found a label with the text of: , however no form control was found associated to that label. Make sure you're using the \\"for\\" attribute or \\"aria-labelledby\\" attribute correctly."`; diff --git a/src/__tests__/__snapshots__/fetch.js.snap b/src/__tests__/__snapshots__/fetch.js.snap index 6de283ad..69e0e57b 100644 --- a/src/__tests__/__snapshots__/fetch.js.snap +++ b/src/__tests__/__snapshots__/fetch.js.snap @@ -2,14 +2,10 @@ exports[`Fetch makes an API call and displays the greeting when load-greeting is clicked 1`] = `
- - + hello there
diff --git a/src/__tests__/element-queries.js b/src/__tests__/element-queries.js index 270865fe..cec3f1ab 100644 --- a/src/__tests__/element-queries.js +++ b/src/__tests__/element-queries.js @@ -1,26 +1,69 @@ import React from 'react' import {render} from '../' -const TestComponent = () => +test('query can return null', () => { + const { + queryByLabelText, + queryByPlaceholderText, + queryByText, + queryByTestId, + } = render(
) + expect(queryByTestId('LucyRicardo')).toBeNull() + expect(queryByLabelText('LucyRicardo')).toBeNull() + expect(queryByPlaceholderText('LucyRicardo')).toBeNull() + expect(queryByText('LucyRicardo')).toBeNull() +}) -test('queryByTestId finds matching element', () => { - const {queryByTestId} = render() - expect(queryByTestId('test-component')).toMatchSnapshot() +test('get throws a useful error message', () => { + const {getByLabelText, getByPlaceholderText, getByText, getByTestId} = render( +
, + ) + expect(() => getByLabelText('LucyRicardo')).toThrowErrorMatchingSnapshot() + expect(() => + getByPlaceholderText('LucyRicardo'), + ).toThrowErrorMatchingSnapshot() + expect(() => getByText('LucyRicardo')).toThrowErrorMatchingSnapshot() + expect(() => getByTestId('LucyRicardo')).toThrowErrorMatchingSnapshot() }) -test('queryByTestId returns null when no matching element exists', () => { - const {queryByTestId} = render() - expect(queryByTestId('unknown-data-testid')).toBeNull() +test('get can get form controls by label text', () => { + const {getByLabelText} = render( +
+ +
+ + +
+
+ + +
+
, + ) + expect(getByLabelText('1st').id).toBe('first-id') + expect(getByLabelText('2nd').id).toBe('second-id') + expect(getByLabelText('3rd').id).toBe('third-id') }) -test('getByTestId finds matching element', () => { - const {getByTestId} = render() - expect(getByTestId('test-component')).toMatchSnapshot() +test('get can get form controls by placeholder', () => { + const {getByPlaceholderText} = render( + , + ) + expect(getByPlaceholderText('username').id).toBe('username-id') }) -test('getByTestId throws error when no matching element exists', () => { - const {getByTestId} = render() - expect(() => - getByTestId('unknown-data-testid'), - ).toThrowErrorMatchingSnapshot() +test('label with no form control', () => { + const {getByLabelText, queryByLabelText} = render() + expect(queryByLabelText('alone')).toBeNull() + expect(() => getByLabelText('alone')).toThrowErrorMatchingSnapshot() +}) + +test('totally empty label', () => { + const {getByLabelText, queryByLabelText} = render(