Skip to content

feat(breaking): add text match options a.k.a string precision API #554

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
merged 31 commits into from
Nov 23, 2021
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
34ffda8
Add TextMatch options to getByAPI
RafikiTiki Sep 9, 2020
d95ff2f
Add TextMatch options to queryByAPI
RafikiTiki Sep 9, 2020
b616319
Add TextMatch options to findByAPI
RafikiTiki Sep 9, 2020
3857870
Add tests covering new TextMatch options
RafikiTiki Sep 9, 2020
7c4c074
Add documentation and examples for TextMatch options
RafikiTiki Sep 9, 2020
f9e7d8d
Implement text match normalization customization
RafikiTiki Sep 14, 2020
e3719bd
Implement tests for normalization options
RafikiTiki Sep 14, 2020
73c6aed
Add text normalization docs
RafikiTiki Sep 14, 2020
0884d86
Refactor typings
RafikiTiki Sep 14, 2020
c14446c
Add export for getDefaultNormalizer function
RafikiTiki Sep 14, 2020
677a78e
Remove undocumented config for normalizer function
RafikiTiki Sep 14, 2020
cb6d122
Update website/docs/Queries.md
thymikee Sep 15, 2020
b7f0b74
BREAKING: make findBy* queries arguments order compatible with RTL
RafikiTiki Sep 15, 2020
5f1d266
Fix tests for findBy queries after changing their API
RafikiTiki Sep 15, 2020
e5e977b
Fix matching nested Text components with queryOptions exact set to true
RafikiTiki Sep 15, 2020
4118a23
Remove recursion matching for *byText APIs
AugustinLF Sep 28, 2020
4087aff
Reuse getAllByText in getByText
AugustinLF Sep 28, 2020
6d8caf4
Add Textmatch options to *byTestId
AugustinLF Oct 7, 2020
5a8c62b
fix: types for getDefaultNormalizer
May 17, 2021
901d3e8
refactor: remove makeNormalizer and use getDefaultNormalizer() instead
May 17, 2021
e1ad2b4
refactor: change params position and description for matches
May 17, 2021
ff10a3a
Merge branch 'master' into AugustinLF/non-recursive-text-match-options
May 19, 2021
6eb071a
refactor: move & remove duplicated tests after merge
May 19, 2021
3bafdff
Merge remote-tracking branch 'origin/master' into non-recursive-text-…
AugustinLF Oct 30, 2021
d2f9f33
Make waitForOptions param change non breaking change
AugustinLF Oct 30, 2021
af62597
chore: cleanup and type fixups
thymikee Nov 23, 2021
5aa5e50
Merge remote-tracking branch 'origin/main' into pr/554
thymikee Nov 23, 2021
864155a
tests: update snapshots
thymikee Nov 23, 2021
ed34a69
chore: use default arg value for normalizer
thymikee Nov 23, 2021
223247b
Restore mock after use
AugustinLF Nov 23, 2021
bd17c86
fix: wrong snapshot update with outdated deps
thymikee Nov 23, 2021
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
12 changes: 6 additions & 6 deletions src/__tests__/findByApi.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,19 @@ test('findBy queries work asynchronously', async () => {
} = render(<View />);
await expect(findByTestId('aTestId', options)).rejects.toBeTruthy();
await expect(findAllByTestId('aTestId', options)).rejects.toBeTruthy();
await expect(findByText('Some Text', options)).rejects.toBeTruthy();
await expect(findAllByText('Some Text', options)).rejects.toBeTruthy();
await expect(findByText('Some Text', {}, options)).rejects.toBeTruthy();
await expect(findAllByText('Some Text', {}, options)).rejects.toBeTruthy();
await expect(
findByPlaceholderText('Placeholder Text', options)
findByPlaceholderText('Placeholder Text', {}, options)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the second breaking change I mentioned

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could provide an easy migration path by looking for waitFor options (timeout, interval) also in the queryOptions only for findBy queries and only till next major release. While displaying a console warning to inform the developer. Wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea that I missed, I fixed it in the latest commit d2f9f33

).rejects.toBeTruthy();
await expect(
findAllByPlaceholderText('Placeholder Text', options)
findAllByPlaceholderText('Placeholder Text', {}, options)
).rejects.toBeTruthy();
await expect(
findByDisplayValue('Display Value', options)
findByDisplayValue('Display Value', {}, options)
).rejects.toBeTruthy();
await expect(
findAllByDisplayValue('Display Value', options)
findAllByDisplayValue('Display Value', {}, options)
).rejects.toBeTruthy();

setTimeout(
Expand Down
113 changes: 112 additions & 1 deletion src/__tests__/getByApi.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @flow
import React from 'react';
import { View, Text, TextInput, Button } from 'react-native';
import { render } from '..';
import { render, getDefaultNormalizer } from '..';

const MyComponent = () => {
return <Text>My Component</Text>;
Expand Down Expand Up @@ -50,3 +50,114 @@ test('supports a regex matcher', () => {
expect(getByTestId(/view/)).toBeTruthy();
expect(getAllByTestId(/text/)).toHaveLength(2);
});

describe('Supports a TextMatch options', () => {
test('getByText, getAllByText', () => {
const { getByText, getAllByText } = render(
<View>
<Text testID="text">Text and details</Text>
<Button
testID="button"
title="Button and a detail"
onPress={jest.fn()}
/>
</View>
);

expect(getByText('details', { exact: false })).toBeTruthy();
expect(getAllByText('detail', { exact: false })).toHaveLength(2);
});

test('getByPlaceholderText, getAllByPlaceholderText', () => {
const { getByPlaceholderText, getAllByPlaceholderText } = render(
<View>
<TextInput placeholder={'Placeholder with details'} />
<TextInput placeholder={'Placeholder with a DETAIL'} />
</View>
);

expect(getByPlaceholderText('details', { exact: false })).toBeTruthy();
expect(getAllByPlaceholderText('detail', { exact: false })).toHaveLength(2);
});

test('getByDisplayValue, getAllByDisplayValue', () => {
const { getByDisplayValue, getAllByDisplayValue } = render(
<View>
<TextInput value={'Value with details'} />
<TextInput value={'Value with a detail'} />
</View>
);

expect(getByDisplayValue('details', { exact: false })).toBeTruthy();
expect(getAllByDisplayValue('detail', { exact: false })).toHaveLength(2);
});

test('with TextMatch option exact === false text search is NOT case sensitive', () => {
const { getByText, getAllByText } = render(
<View>
<Text testID="text">Text and details</Text>
<Button
testID="button"
title="Button and a DeTAil"
onPress={jest.fn()}
/>
</View>
);

expect(getByText('DeTaIlS', { exact: false })).toBeTruthy();
expect(getAllByText('detail', { exact: false })).toHaveLength(2);
});
});

describe('Supports normalization', () => {
test('trims and collapses whitespace by default', () => {
const { getByText } = render(
<View>
<Text>{` Text and


whitespace`}</Text>
</View>
);

expect(getByText('Text and whitespace')).toBeTruthy();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that one 👍

});

test('trim and collapseWhitespace is customizable by getDefaultNormalizer param', () => {
const testTextWithWhitespace = ` Text and


whitespace`;
const { getByText } = render(
<View>
<Text>{testTextWithWhitespace}</Text>
</View>
);

expect(
getByText(testTextWithWhitespace, {
normalizer: getDefaultNormalizer({
trim: false,
collapseWhitespace: false,
}),
})
).toBeTruthy();
});

test('normalizer function is customisable', () => {
const testText = 'A TO REMOVE text';
const normalizerFn = (textToNormalize: string) =>
textToNormalize.replace('TO REMOVE ', '');
const { getByText } = render(
<View>
<Text>{testText}</Text>
</View>
);

expect(
getByText('A text', {
normalizer: normalizerFn,
})
).toBeTruthy();
});
});
58 changes: 45 additions & 13 deletions src/__tests__/queryByApi.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ test('queryByText not found', () => {
).toBeFalsy();
});

test('queryByText nested text across multiple <Text> in <Text>', () => {
test('queryByText does not match nested text across multiple <Text> in <Text>', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we're regressing this functionality to be compatible with Testing Library, which also doesn't support this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I checked, testing library doesn't match in this case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this PR change this functionality? Finding the text in nodes still recurses through children

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must be missing something but how does the logic added to getNodeByText end up not matching in this case? As I see it the the logic in getChildrenAsText will return ['Hello', ' ', 'World', '!'] which when joined will be "Hello World!", which is an exact match for what you are looking for.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the change is to keep matching component with multiple children, but not <Text>Hello <Text>World</Text></Text, because now getChildrenAsText don't read its children's children

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so if the test render looked like this:

<Text>
  <Text>Hello</Text>
  World!
</Text>

Would this then match the outer text?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This currently match is the current released version, but wouldn't match in my current implementation. It's the topic of discussion in #554 (comment)

const { queryByText } = render(
<Text nativeID="1">
Hello{' '}
Expand All @@ -59,7 +59,7 @@ test('queryByText nested text across multiple <Text> in <Text>', () => {
</Text>
);

expect(queryByText('Hello World!')?.props.nativeID).toBe('1');
expect(queryByText('Hello World!')).toBe(null);
});

test('queryByText with nested Text components return the closest Text', () => {
Expand All @@ -72,6 +72,7 @@ test('queryByText with nested Text components return the closest Text', () => {
const { queryByText } = render(<NestedTexts />);

expect(queryByText('My text')?.props.nativeID).toBe('2');
expect(queryByText('My text', { exact: false })?.props.nativeID).toBe('2');
});

test('queryByText with nested Text components each with text return the lowest one', () => {
Expand All @@ -87,18 +88,26 @@ test('queryByText with nested Text components each with text return the lowest o
expect(queryByText('My text')?.props.nativeID).toBe('2');
});

test('queryByText nested <CustomText> in <Text>', () => {
const CustomText = ({ children }) => {
return <Text>{children}</Text>;
};
test('queryByText with nested Text components: not-exact text match returns the most deeply nested common component', () => {
const { queryByText: queryByTextFirstCase } = render(
<Text nativeID="1">
bob
<Text nativeID="2">My </Text>
<Text nativeID="3">text</Text>
</Text>
);

const { queryByText: queryByTextSecondCase } = render(
<Text nativeID="1">
bob
<Text nativeID="2">My text for test</Text>
</Text>
);

expect(queryByTextFirstCase('My text')).toBe(null);
expect(
render(
<Text>
Hello <CustomText>World!</CustomText>
</Text>
).queryByText('Hello World!')
).toBeTruthy();
queryByTextSecondCase('My text', { exact: false })?.props.nativeID
).toBe('2');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does seem strange a bit, that we use one component structure with the default exact: true and other component structure with exact: false. If we were to contast exact true/false IMO we should use the same component structure.

Looks like we should do one of the following (in order of preference):

  1. merge queryByTextFirstCase + queryByTextSecondCase
  2. add queryByTextFirstCase + exact: false & queryByTextSecondCase + exact: true
  3. split the test into two unreleated tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do overall believe that tests should be restructured, a lot of cases are tested only because the internal APIs are the same (a lot of edge cases for findByText are only tested in getByText for instance), so I'd like to have more tests, and make them more verbose. But I believe it's out of the scope of this PR.

But this specific case makes sense, I'll follow your suggestion, I got a bit lazy and monkey patched some tests without really reconsidering them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those could contain additional describe there as it contains 2 different scenarios.

});

test('queryByText nested deep <CustomText> in <Text>', () => {
Expand All @@ -112,5 +121,28 @@ test('queryByText nested deep <CustomText> in <Text>', () => {
<CustomText>Hello</CustomText> <CustomText>World!</CustomText>
</Text>
).queryByText('Hello World!')
).toBeTruthy();
).toBe(null);
});

test('queryAllByText does not match several times the same text', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

const allMatched = render(
<Text nativeID="1">
Start
<Text nativeID="2">This is a long text</Text>
</Text>
).queryAllByText('long text', { exact: false });
expect(allMatched.length).toBe(1);
expect(allMatched[0].props.nativeID).toBe('2');
});

test('queryAllByText matches all the matching nodes', () => {
const allMatched = render(
<Text nativeID="1">
Start
<Text nativeID="2">This is a long text</Text>
<Text nativeID="3">This is another long text</Text>
</Text>
).queryAllByText('long text', { exact: false });
expect(allMatched.length).toBe(2);
expect(allMatched.map((node) => node.props.nativeID)).toEqual(['2', '3']);
});
1 change: 1 addition & 0 deletions src/helpers/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export function printDeprecationWarning(functionName: string) {
return;
}

// eslint-disable-next-line no-console
console.warn(`
Deprecation Warning:
Use of ${functionName} is not recommended and will be deleted in future versions of @testing-library/react-native.
Expand Down
58 changes: 48 additions & 10 deletions src/helpers/findByAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,54 +12,92 @@ import {
getAllByDisplayValue,
} from './getByAPI';
import { throwRenamedFunctionError } from './errors';
import type { TextMatchOptions } from './getByAPI';

const makeFindQuery = <Text, Result>(
instance: ReactTestInstance,
getQuery: (instance: ReactTestInstance) => (text: Text) => Result,
getQuery: (
instance: ReactTestInstance
) => (text: Text, options?: TextMatchOptions) => Result,
text: Text,
queryOptions?: TextMatchOptions,
waitForOptions: WaitForOptions
): Promise<Result> => waitFor(() => getQuery(instance)(text), waitForOptions);
): Promise<Result> =>
waitFor(() => getQuery(instance)(text, queryOptions), waitForOptions);

export const findByTestId = (instance: ReactTestInstance) => (
testId: string | RegExp,
waitForOptions: WaitForOptions = {}
) => makeFindQuery(instance, getByTestId, testId, waitForOptions);
) => makeFindQuery(instance, getByTestId, testId, {}, waitForOptions);

export const findAllByTestId = (instance: ReactTestInstance) => (
testId: string | RegExp,
waitForOptions: WaitForOptions = {}
) => makeFindQuery(instance, getAllByTestId, testId, waitForOptions);
) => makeFindQuery(instance, getAllByTestId, testId, {}, waitForOptions);

export const findByText = (instance: ReactTestInstance) => (
text: string | RegExp,
queryOptions?: TextMatchOptions,
waitForOptions: WaitForOptions = {}
) => makeFindQuery(instance, getByText, text, waitForOptions);
) => makeFindQuery(instance, getByText, text, queryOptions, waitForOptions);

export const findAllByText = (instance: ReactTestInstance) => (
text: string | RegExp,
queryOptions?: TextMatchOptions,
waitForOptions: WaitForOptions = {}
) => makeFindQuery(instance, getAllByText, text, waitForOptions);
) => makeFindQuery(instance, getAllByText, text, queryOptions, waitForOptions);

export const findByPlaceholderText = (instance: ReactTestInstance) => (
placeholder: string | RegExp,
queryOptions?: TextMatchOptions,
waitForOptions: WaitForOptions = {}
) => makeFindQuery(instance, getByPlaceholderText, placeholder, waitForOptions);
) =>
makeFindQuery(
instance,
getByPlaceholderText,
placeholder,
queryOptions,
waitForOptions
);

export const findAllByPlaceholderText = (instance: ReactTestInstance) => (
placeholder: string | RegExp,
queryOptions?: TextMatchOptions,
waitForOptions: WaitForOptions = {}
) =>
makeFindQuery(instance, getAllByPlaceholderText, placeholder, waitForOptions);
makeFindQuery(
instance,
getAllByPlaceholderText,
placeholder,
queryOptions,
waitForOptions
);

export const findByDisplayValue = (instance: ReactTestInstance) => (
value: string | RegExp,
queryOptions?: TextMatchOptions,
waitForOptions: WaitForOptions = {}
) => makeFindQuery(instance, getByDisplayValue, value, waitForOptions);
) =>
makeFindQuery(
instance,
getByDisplayValue,
value,
queryOptions,
waitForOptions
);

export const findAllByDisplayValue = (instance: ReactTestInstance) => (
value: string | RegExp,
queryOptions?: TextMatchOptions,
waitForOptions: WaitForOptions = {}
) => makeFindQuery(instance, getAllByDisplayValue, value, waitForOptions);
) =>
makeFindQuery(
instance,
getAllByDisplayValue,
value,
queryOptions,
waitForOptions
);

export const findByAPI = (instance: ReactTestInstance) => ({
findByTestId: findByTestId(instance),
Expand Down
Loading