Skip to content

fix(breaking): getByTestId should work only for native elements #324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/__tests__/getByApi.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// @flow
import React from 'react';
import { View, Text, TextInput, Button } from 'react-native';
import { render } from '..';

const MyComponent = () => {
return <Text>My Component</Text>;
};

test('getByTestId returns only native elements', () => {
const { getByTestId, getAllByTestId } = render(
<View>
<Text testID="text">Text</Text>
<TextInput testID="textInput" />
<View testID="view" />
<Button testID="button" title="Button" onPress={jest.fn()} />
<MyComponent testID="myComponent" />
</View>
);

expect(getByTestId('text')).toBeTruthy();
expect(getByTestId('textInput')).toBeTruthy();
expect(getByTestId('view')).toBeTruthy();
expect(getByTestId('button')).toBeTruthy();

expect(getAllByTestId('text')).toHaveLength(1);
expect(getAllByTestId('textInput')).toHaveLength(1);
expect(getAllByTestId('view')).toHaveLength(1);
expect(getAllByTestId('button')).toHaveLength(1);

expect(() => getByTestId('myComponent')).toThrowError(
'No instances found with testID: myComponent'
);
expect(() => getAllByTestId('myComponent')).toThrowError(
'No instances found with testID: myComponent'
);
});
4 changes: 2 additions & 2 deletions src/helpers/findByAPI.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @flow
import waitForElement from '../waitForElement';
import {
fixedGetByTestId,
getByTestId,
getAllByTestId,
getByText,
getAllByText,
Expand Down Expand Up @@ -31,7 +31,7 @@ const makeFindQuery = <Text, Result>(
export const findByTestId = (instance: ReactTestInstance) => (
testId: string,
waitForOptions: WaitForOptions = {}
) => makeFindQuery(instance, fixedGetByTestId, testId, waitForOptions);
) => makeFindQuery(instance, getByTestId, testId, waitForOptions);

export const findAllByTestId = (instance: ReactTestInstance) => (
testId: string,
Expand Down
9 changes: 0 additions & 9 deletions src/helpers/getByAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,6 @@ export const getByProps = (instance: ReactTestInstance, warnFn?: Function) =>
};

export const getByTestId = (instance: ReactTestInstance) =>
function getByTestIdFn(testID: string) {
try {
return instance.findByProps({ testID });
} catch (error) {
throw new ErrorWithStack(prepareErrorMessage(error), getByTestIdFn);
}
};

export const fixedGetByTestId = (instance: ReactTestInstance) =>
function getByTestIdFn(testID: string) {
try {
const results = getAllByTestId(instance)(testID);
Expand Down
9 changes: 2 additions & 7 deletions website/docs/Queries.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ title: Queries

`findAllBy` queries return a promise which resolves to an array when any matching elements are found. The promise is rejected if no elements match after a default timeout of 4500ms.


:::info
`findBy` and `findAllBy` queries accept optional `waitForOptions` object argument which can contain `timeout` and `interval` properies which have the same meaning as respective arguments to [`waitForElement`](https://callstack.github.io/react-native-testing-library/docs/api#waitforelement) function.
`findBy` and `findAllBy` queries accept optional `waitForOptions` object argument which can contain `timeout` and `interval` properies which have the same meaning as respective arguments to [`waitForElement`](https://callstack.github.io/react-native-testing-library/docs/api#waitforelement) function.
:::

## Queries
Expand Down Expand Up @@ -105,11 +104,7 @@ const element = getByTestId('unique-id');
```

:::caution
Please be mindful when using these API and **treat it as an escape hatch**. Your users can't interact with `testID` anyhow, so you may end up writing tests that provide false sense of security. Favor text and accessibility queries instead.
:::

:::danger
Current implementation of `getByTestId` and `queryByTestId` has a serious flaw, which results in finding more IDs than there really would be present in native React host components. Fixing it may break some of your tests so we'll do it in next major release (v2). As a temporary workaround, please use `getAllByTestId('your-id')[0]` or `queryAllByTestId('your-id')[0]` or migrate off testing with testID, which is considered to be an escape hatch.
Please be mindful when using these API and **treat it as an escape hatch**. Your users can't interact with `testID` anyhow, so you may end up writing tests that provide false sense of security. Favor text and accessibility queries instead.
:::

### `ByA11yLabel`, `ByAccessibilityLabel`
Expand Down