Skip to content

feat: ...byRole type queries accepts second arg #875

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

Closed
wants to merge 17 commits into from
Closed
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@testing-library/react-native",
"version": "9.1.0",
"version": "9.0.0-alpha.0",
"description": "Simple and complete React Native testing utilities that encourage good testing practices.",
"main": "dist/index.js",
"types": "dist/index.d.ts",
Expand Down
81 changes: 77 additions & 4 deletions src/__tests__/a11yAPI.test.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import * as React from 'react';
import { TouchableOpacity, Text } from 'react-native';
import { TouchableOpacity, Text, View } from 'react-native';
import { render } from '../index';

const BUTTON_LABEL = 'cool button';
const BUTTON_HINT = 'click this button';
const TEXT_LABEL = 'cool text';
const TEXT_HINT = 'static text';
const ONE_OCCURANCE = 'more words';
const TWO_OCCURANCE = 'cooler text';
// Little hack to make all the methods happy with type
const NO_MATCHES_TEXT: any = 'not-existent-element';
const FOUND_TWO_INSTANCES = 'Expected 1 but found 2 instances';
Expand Down Expand Up @@ -69,6 +71,22 @@ function Section() {
);
}

function ButtonsWithText() {
return (
<>
<TouchableOpacity accessibilityRole="button">
<Text>{TWO_OCCURANCE}</Text>
</TouchableOpacity>
<TouchableOpacity accessibilityRole="button">
<Text>{TWO_OCCURANCE}</Text>
</TouchableOpacity>
<TouchableOpacity accessibilityRole="button">
<Text>{ONE_OCCURANCE}</Text>
</TouchableOpacity>
</>
);
}

test('getByLabelText, queryByLabelText, findByLabelText', async () => {
const { getByLabelText, queryByLabelText, findByLabelText } = render(
<Section />
Expand Down Expand Up @@ -183,7 +201,7 @@ test('getByRole, queryByRole, findByRole', async () => {

const asyncButton = await findByRole('button');
expect(asyncButton.props.accessibilityRole).toEqual('button');
await expect(findByRole(NO_MATCHES_TEXT, waitForOptions)).rejects.toThrow(
await expect(findByRole(NO_MATCHES_TEXT, {}, waitForOptions)).rejects.toThrow(
getNoInstancesFoundMessage('accessibilityRole')
);
await expect(findByRole('link')).rejects.toThrow(FOUND_TWO_INSTANCES);
Expand All @@ -201,9 +219,64 @@ test('getAllByRole, queryAllByRole, findAllByRole', async () => {
expect(queryAllByRole(NO_MATCHES_TEXT)).toEqual([]);

await expect(findAllByRole('link')).resolves.toHaveLength(2);
await expect(findAllByRole(NO_MATCHES_TEXT, waitForOptions)).rejects.toThrow(
getNoInstancesFoundMessage('accessibilityRole')
await expect(
findAllByRole(NO_MATCHES_TEXT, {}, waitForOptions)
).rejects.toThrow(getNoInstancesFoundMessage('accessibilityRole'));
});

describe('findBy options deprecations', () => {
let warnSpy: jest.SpyInstance;
beforeEach(() => {
warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
});
afterEach(() => {
warnSpy.mockRestore();
});

test('findByText queries warn on deprecated use of WaitForOptions', async () => {
const options = { timeout: 10 };
// mock implementation to avoid warning in the test suite
const { rerender, findByText } = render(<View />);
await expect(findByText('Some Text', options)).rejects.toBeTruthy();

setTimeout(
() =>
rerender(
<View>
<Text>Some Text</Text>
</View>
),
20
);

await expect(findByText('Some Text')).resolves.toBeTruthy();

expect(warnSpy).toHaveBeenCalledWith(
expect.stringContaining('Use of option "timeout"')
);
}, 20000);
});

test('getAllByRole, queryAllByRole, findAllByRole with name', async () => {
const { getAllByRole, queryAllByRole, findAllByRole } = render(
<ButtonsWithText />
);

expect(getAllByRole('button', { name: TWO_OCCURANCE })).toHaveLength(2);
expect(getAllByRole('button', { name: ONE_OCCURANCE })).toHaveLength(1);
expect(queryAllByRole('button', { name: TWO_OCCURANCE })).toHaveLength(2);

expect(() => getAllByRole('button', { name: NO_MATCHES_TEXT })).toThrow(
getNoInstancesFoundMessage('accessibilityRole', 'button')
);
expect(queryAllByRole('button', { name: NO_MATCHES_TEXT })).toEqual([]);

await expect(
findAllByRole('button', { name: TWO_OCCURANCE })
).resolves.toHaveLength(2);
await expect(
findAllByRole('button', { name: NO_MATCHES_TEXT })
).rejects.toThrow(getNoInstancesFoundMessage('accessibilityRole', 'button'));
});

// TODO: accessibilityStates was removed from RN 0.62
Expand Down
60 changes: 53 additions & 7 deletions src/helpers/a11yAPI.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { ReactTestInstance } from 'react-test-renderer';
import type { AccessibilityRole, AccessibilityState } from 'react-native';
import type { AccessibilityState } from 'react-native';
import type { WaitForOptions } from '../waitFor';
import type { TextMatch } from '../matches';
import makeA11yQuery from './makeA11yQuery';
Expand All @@ -11,6 +11,34 @@ type A11yValue = {
now?: number;
text?: string;
};
type A11yRole =
| 'none'
| 'button'
| 'link'
| 'search'
| 'image'
| 'keyboardkey'
| 'text'
| 'adjustable'
| 'imagebutton'
| 'header'
| 'summary'
| 'alert'
| 'checkbox'
| 'combobox'
| 'menu'
| 'menubar'
| 'menuitem'
| 'progressbar'
| 'radio'
| 'radiogroup'
| 'scrollbar'
| 'spinbutton'
| 'switch'
| 'tab'
| 'tablist'
| 'timer'
| 'toolbar';

type GetReturn = ReactTestInstance;
type GetAllReturn = Array<ReactTestInstance>;
Expand All @@ -19,6 +47,10 @@ type QueryAllReturn = Array<ReactTestInstance>;
type FindReturn = Promise<GetReturn>;
type FindAllReturn = Promise<GetAllReturn>;

export type QueryOptions = {
Copy link
Member

Choose a reason for hiding this comment

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

This type should be renamed to ByRoleOptions bacause this type of option is only relevant to byRole queries.

name?: string | RegExp;
};

export type A11yAPI = {
// Label
getByLabelText: (label: TextMatch) => GetReturn;
Expand Down Expand Up @@ -61,16 +93,30 @@ export type A11yAPI = {
) => FindAllReturn;

// Role
getByRole: (role: AccessibilityRole | RegExp) => GetReturn;
getAllByRole: (role: AccessibilityRole | RegExp) => GetAllReturn;
queryByRole: (role: AccessibilityRole | RegExp) => QueryReturn;
queryAllByRole: (role: AccessibilityRole | RegExp) => QueryAllReturn;
getByRole: (
role: A11yRole | RegExp,
queryOptions?: QueryOptions
) => GetReturn;
getAllByRole: (
role: A11yRole | RegExp,
queryOptions?: QueryOptions
) => GetAllReturn;
queryByRole: (
role: A11yRole | RegExp,
queryOptions?: QueryOptions
) => QueryReturn;
queryAllByRole: (
role: A11yRole | RegExp,
queryOptions?: QueryOptions
) => QueryAllReturn;
findByRole: (
role: AccessibilityRole,
role: A11yRole,
queryOptions?: QueryOptions & WaitForOptions,
waitForOptions?: WaitForOptions
) => FindReturn;
findAllByRole: (
role: AccessibilityRole,
role: A11yRole,
queryOptions?: QueryOptions & WaitForOptions,
waitForOptions?: WaitForOptions
) => FindAllReturn;

Expand Down
109 changes: 96 additions & 13 deletions src/helpers/makeA11yQuery.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { ReactTestInstance } from 'react-test-renderer';
import waitFor from '../waitFor';
import { getQueriesForElement } from '../within';
import type { WaitForOptions } from '../waitFor';
import {
ErrorWithStack,
Expand All @@ -17,6 +18,39 @@ function makeAliases(aliases: Array<string>, query: Function) {
.reduce((acc, query) => ({ ...acc, ...query }), {});
}

// The WaitForOptions has been moved to the third param of findBy* methods with the addition of QueryOptions.
// To make the migration easier and to avoid a breaking change, keep reading these options from second param
// but warn.
const deprecatedKeys: (keyof WaitForOptions)[] = [
'timeout',
'interval',
'stackTraceError',
];
const warnDeprectedWaitForOptionsUsage = (queryOptions?: WaitForOptions) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const warnDeprectedWaitForOptionsUsage = (queryOptions?: WaitForOptions) => {
const warnDeprecatedWaitForOptionsUsage = (queryOptions?: WaitForOptions) => {

Copy link
Member

Choose a reason for hiding this comment

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

Also pls move it to helpers/errors.ts, as this looks like reusable function that can be used with other findByXxx queries if they decide to go with query options.

if (queryOptions) {
const waitForOptions: WaitForOptions = {
timeout: queryOptions.timeout,
interval: queryOptions.interval,
stackTraceError: queryOptions.stackTraceError,
};
deprecatedKeys.forEach((key) => {
if (queryOptions[key]) {
// eslint-disable-next-line no-console
console.warn(
`Use of option "${key}" in a findBy* query's second parameter, QueryOptions, is deprecated. Please pass this option in the third, WaitForOptions, parameter.
Example:
findByText(text, {}, { ${key}: ${queryOptions[key]} })`
);
}
});
return waitForOptions;
}
};

type QueryOptions = {
name: string | RegExp;
};

type QueryNames = {
getBy: Array<string>;
getAllBy: Array<string>;
Expand All @@ -31,8 +65,27 @@ const makeA11yQuery = <P extends unknown, M extends unknown>(
queryNames: QueryNames,
matcherFn: (prop: P, value: M) => boolean
) => (instance: ReactTestInstance) => {
const getBy = (matcher: M) => {
const filterWithName = (
Copy link
Member

Choose a reason for hiding this comment

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

Suggested name filterByAccessibilityName

node: ReactTestInstance,
options: QueryOptions,
matcher: M
) => {
const matchesRole =
isNodeValid(node) && matcherFn(node.props[name], matcher);

return (
matchesRole && !!getQueriesForElement(node).queryByText(options.name)
Copy link
Member

Choose a reason for hiding this comment

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

it looks like we should match for either queryByText(options.name) or queryByLabelText(options.name)

);
};

const getBy = (matcher: M, queryOptions?: QueryOptions) => {
try {
if (queryOptions?.name) {
return instance.find((node) =>
filterWithName(node, queryOptions, matcher)
);
}

return instance.find(
(node) => isNodeValid(node) && matcherFn(node.props[name], matcher)
);
Expand All @@ -44,10 +97,18 @@ const makeA11yQuery = <P extends unknown, M extends unknown>(
}
};

const getAllBy = (matcher: M) => {
const results = instance.findAll(
(node) => isNodeValid(node) && matcherFn(node.props[name], matcher)
);
const getAllBy = (matcher: M, options?: QueryOptions) => {
let results = [];

if (options?.name) {
results = instance.findAll((node) =>
filterWithName(node, options, matcher)
);
} else {
results = instance.findAll(
(node) => isNodeValid(node) && matcherFn(node.props[name], matcher)
);
}

if (results.length === 0) {
throw new ErrorWithStack(
Expand All @@ -59,28 +120,50 @@ const makeA11yQuery = <P extends unknown, M extends unknown>(
return results;
};

const queryBy = (matcher: M) => {
const queryBy = (matcher: M, options?: QueryOptions) => {
try {
return getBy(matcher);
return getBy(matcher, options);
} catch (error) {
return createQueryByError(error, queryBy);
}
};

const queryAllBy = (matcher: M) => {
const queryAllBy = (matcher: M, options?: QueryOptions) => {
try {
return getAllBy(matcher);
return getAllBy(matcher, options);
} catch (error) {
return [];
}
};

const findBy = (matcher: M, waitForOptions?: WaitForOptions) => {
return waitFor(() => getBy(matcher), waitForOptions);
const findBy = (
matcher: M,
queryOptions?: QueryOptions & WaitForOptions,
waitForOptions?: WaitForOptions
) => {
const deprecatedWaitForOptions = warnDeprectedWaitForOptionsUsage(
queryOptions
);

return waitFor(() => getBy(matcher, queryOptions), {
...deprecatedWaitForOptions,
...waitForOptions,
});
};

const findAllBy = (matcher: M, waitForOptions?: WaitForOptions) => {
return waitFor(() => getAllBy(matcher), waitForOptions);
const findAllBy = (
matcher: M,
queryOptions?: QueryOptions & WaitForOptions,
waitForOptions?: WaitForOptions
) => {
const deprecatedWaitForOptions = warnDeprectedWaitForOptionsUsage(
queryOptions
);

return waitFor(() => getAllBy(matcher, queryOptions), {
...deprecatedWaitForOptions,
...waitForOptions,
});
};

return {
Expand Down