Skip to content
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

Port react-conformance tests to testing-library #22009

Merged
merged 10 commits into from
Mar 29, 2022
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "BREAKING CHANGE: Switch from Enzyme to `@testing-library/react` and remove Enzyme-specific APIs",
"packageName": "@fluentui/react-conformance",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Fix cleanup of overridesWin test",
"packageName": "@fluentui/react-conformance-griffel",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,20 @@ import * as path from 'path';
import * as React from 'react';
import { ComponentType, ReactWrapper } from 'enzyme';
import { act } from 'react-dom/test-utils';
import { getDisplayName, mountWithProvider as mount, syntheticEvent, mountWithProvider, consoleUtil } from 'test/utils';
import {
getDisplayName,
mountWithProvider as mount,
syntheticEvent,
consoleUtil,
EmptyThemeProvider,
} from 'test/utils';

import * as FluentUI from 'src/index';
import { getEventTargetComponent, EVENT_TARGET_ATTRIBUTE } from './eventTarget';
import { extraConformanceTests } from './extraConformanceTests';

export interface Conformant<TProps = {}> extends Pick<IsConformantOptions<TProps>, 'disabledTests' | 'testOptions'> {
export interface Conformant<TProps = {}>
extends Pick<IsConformantOptions<TProps>, 'disabledTests' | 'testOptions' | 'getTargetElement'> {
/** Path to the test file. */
testPath: string;
constructorName?: string;
Expand All @@ -27,6 +34,8 @@ export interface Conformant<TProps = {}> extends Pick<IsConformantOptions<TProps
requiredProps?: object;
/** This component uses wrapper slot to wrap the 'meaningful' element. */
wrapperComponent?: React.ElementType;
/** Helpers such as FocusZone and Ref which should be ignored when finding nontrivial children. */
helperComponents?: React.ElementType[];
/** List of autocontrolled props for this component. */
autoControlledProps?: string[];
/** Child component that will receive unhandledProps. */
Expand Down Expand Up @@ -66,23 +75,21 @@ export function isConformant(
} = options;

const defaultConfig: IsConformantOptions = {
customMount: mountWithProvider,
renderOptions: { wrapper: EmptyThemeProvider },
Copy link
Member Author

Choose a reason for hiding this comment

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

componentPath: testPath
.replace(/test[/\\]specs/, 'src')
.replace('-test.', '.')
.replace(/.ts$/, '.tsx'),
Component,
displayName: constructorName,
// TODO enable component-has-root-ref and disable test where necessary.
// List of the components that will either require the test to be disabled or fixed: (https://hackmd.io/OAUn0pF6Qj-vc315wAHXLQ)
// v0 doesn't use the patterns these tests look at
disabledTests: [
'has-top-level-file',
'consistent-callback-args',
// Disabled as v0 has different prefix
'component-has-static-classname',
'component-has-static-classnames-object',
'component-has-static-classname-exported',
],
helperComponents: [Ref, RefFindNode, FocusZone],
extraTests: extraConformanceTests,
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { isConformant } from 'test/specs/commonTests';
import { FormButton } from 'src/components/Form/FormButton';
import { FormButton, formButtonClassName } from 'src/components/Form/FormButton';
import { Button } from 'src/components/Button/Button';

describe('FormButton', () => {
Expand All @@ -8,5 +8,7 @@ describe('FormButton', () => {
constructorName: 'FormButton',
forwardsRefTo: `Button`,
targetComponent: Button,
getTargetElement: (result, attr) =>
attr === 'className' ? result.container.querySelector(`.${formButtonClassName}`) : result.getByRole('button'),
Copy link
Member Author

Choose a reason for hiding this comment

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

Since react-conformance no longer uses enzyme's find method to look for an instance of a particular component, we have to look for where attributes are applied in the DOM instead. Open to suggestions from the northstar team if there's a more appropriate way to do this.

});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { isConformant } from 'test/specs/commonTests';
import { FormCheckbox } from 'src/components/Form/FormCheckbox';
import { FormCheckbox, formCheckboxClassName } from 'src/components/Form/FormCheckbox';
import { Checkbox } from 'src/components/Checkbox/Checkbox';

describe('FormCheckbox', () => {
Expand All @@ -9,5 +9,7 @@ describe('FormCheckbox', () => {
// TODO: point to correct once Checkbox will be using compose
forwardsRefTo: false,
targetComponent: Checkbox,
getTargetElement: (result, attr) =>
attr === 'className' ? result.container.querySelector(`.${formCheckboxClassName}`) : result.getByRole('checkbox'),
});
});
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { isConformant } from 'test/specs/commonTests';
import { FormDatepicker } from 'src/components/Form/FormDatepicker';
import { Datepicker } from 'src/components/Datepicker/Datepicker';
import { FormDatepicker, formDatepickerClassName } from 'src/components/Form/FormDatepicker';
import { Datepicker, datepickerClassName } from 'src/components/Datepicker/Datepicker';

describe('FormDatepicker', () => {
isConformant(FormDatepicker, {
testPath: __filename,
constructorName: 'FormDatepicker',
forwardsRefTo: false,
targetComponent: Datepicker,
getTargetElement: (result, attr) =>
attr === 'className'
? result.container.querySelector(`.${formDatepickerClassName}`)
: result.container.querySelector(`.${datepickerClassName}`),
});
});
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { isConformant } from 'test/specs/commonTests';
import { FormDropdown } from 'src/components/Form/FormDropdown';
import { Dropdown } from 'src/components/Dropdown/Dropdown';
import { FormDropdown, formDropdownClassName } from 'src/components/Form/FormDropdown';
import { Dropdown, dropdownClassName } from 'src/components/Dropdown/Dropdown';

describe('FormDropdown', () => {
isConformant(FormDropdown, {
testPath: __filename,
constructorName: 'FormDropdown',
forwardsRefTo: false,
targetComponent: Dropdown,
getTargetElement: (result, attr) =>
attr === 'className'
? result.container.querySelector(`.${formDropdownClassName}`)
: result.container.querySelector(`.${dropdownClassName}`),
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { isConformant } from 'test/specs/commonTests';
import { FormInput } from 'src/components/Form/FormInput';
import { FormInput, formInputClassName } from 'src/components/Form/FormInput';
import { Input, inputSlotClassNames } from 'src/components/Input/Input';

describe('FormInput', () => {
Expand All @@ -17,5 +17,7 @@ describe('FormInput', () => {
onBlur: 'input',
},
disabledTests: ['component-has-root-ref'],
getTargetElement: (result, attr) =>
attr === 'className' ? result.container.querySelector(`.${formInputClassName}`) : result.getByRole('input'),
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { isConformant } from 'test/specs/commonTests';
import { FormRadioGroup } from 'src/components/Form/FormRadioGroup';
import { FormRadioGroup, formRadioGroupClassName } from 'src/components/Form/FormRadioGroup';
import { RadioGroup } from 'src/components/RadioGroup/RadioGroup';

describe('FormRadioGroup', () => {
Expand All @@ -9,5 +9,9 @@ describe('FormRadioGroup', () => {
// TODO: point to correct once RadioGroup will be using compose
forwardsRefTo: false,
targetComponent: RadioGroup,
getTargetElement: (result, attr) =>
attr === 'className'
? result.container.querySelector(`.${formRadioGroupClassName}`)
: result.getByRole('radiogroup'),
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { isConformant } from 'test/specs/commonTests';
import { FormSlider } from 'src/components/Form/FormSlider';
import { Slider } from 'src/components/Slider/Slider';
import { FormSlider, formSliderClassName } from 'src/components/Form/FormSlider';
import { Slider, sliderClassName } from 'src/components/Slider/Slider';

describe('FormSlider', () => {
isConformant(FormSlider, {
Expand All @@ -15,5 +15,9 @@ describe('FormSlider', () => {
onKeyPress: 'input',
onKeyUp: 'input',
},
getTargetElement: (result, attr) =>
attr === 'className'
? result.container.querySelector(`.${formSliderClassName}`)
: result.container.querySelector(`.${sliderClassName}`),
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { isConformant } from 'test/specs/commonTests';
import { FormTextArea } from 'src/components/Form/FormTextArea';
import { FormTextArea, formTextAreaClassName } from 'src/components/Form/FormTextArea';

import { TextArea } from 'src/components/TextArea/TextArea';

Expand All @@ -9,5 +9,7 @@ describe('FormTextArea', () => {
constructorName: 'FormTextArea',
forwardsRefTo: false,
targetComponent: TextArea,
getTargetElement: (result, attr) =>
attr === 'className' ? result.container.querySelector(`.${formTextAreaClassName}`) : result.getByRole('textbox'),
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe('MenuItem', () => {
testPath: __filename,
constructorName: 'MenuItem',
wrapperComponent: MenuItemWrapper,
getTargetElement: result => result.getByRole('menuitem'),
autoControlledProps: ['menuOpen'],
testOptions: { 'consistent-callback-names': { ignoreProps: ['onActiveChanged'] } },
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import { implementsShorthandProp, isConformant } from 'test/specs/commonTests';

import { Box } from 'src/components/Box/Box';
import { ToolbarMenu } from 'src/components/Toolbar/ToolbarMenu';
import { ToolbarMenuItem } from 'src/components/Toolbar/ToolbarMenuItem';
import { ToolbarMenuItem, toolbarMenuItemClassName } from 'src/components/Toolbar/ToolbarMenuItem';

describe('ToolbarMenuItem', () => {
isConformant(ToolbarMenuItem, {
testPath: __filename,
wrapperComponent: Box,
autoControlledProps: ['menuOpen'],
constructorName: 'ToolbarMenuItem',
getTargetElement: result => result.container.querySelector(`.${toolbarMenuItemClassName}`),
});
implementsShorthandProp(ToolbarMenuItem)('menu', ToolbarMenu, {
implementsPopper: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ describe('ToolbarMenuRadioGroup', () => {
testPath: __filename,
wrapperComponent: ToolbarMenuRadioGroupWrapper,
constructorName: 'ToolbarMenuRadioGroup',
getTargetElement: result => result.container.querySelector(`[data-aa-class="ToolbarMenuRadioGroup"]`),
});

describe('accessibility', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ import * as React from 'react';
import { Accordion } from './Accordion';
import * as renderer from 'react-test-renderer';
import { isConformant } from '../../common/isConformant';
import { AccordionContext } from './AccordionContext';

describe('Accordion', () => {
isConformant({
Component: Accordion,
displayName: 'Accordion',
helperComponents: [AccordionContext.Provider],
// Accordion does not have own styles
disabledTests: ['make-styles-overrides-win'],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { AccordionHeader } from './AccordionHeader';
import { AccordionHeaderProps } from './AccordionHeader.types';
import * as renderer from 'react-test-renderer';
import { isConformant } from '../../common/isConformant';
import { AccordionHeaderContext } from './AccordionHeaderContext';
import { Accordion } from '../Accordion/Accordion';
import { AccordionItem } from '../AccordionItem';
import { AccordionPanel } from '../AccordionPanel';
Expand All @@ -13,7 +12,6 @@ describe('AccordionHeader', () => {
isConformant<AccordionHeaderProps>({
Component: AccordionHeader,
displayName: 'AccordionHeader',
helperComponents: [AccordionHeaderContext.Provider],
testOptions: {
'has-static-classnames': [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ import * as React from 'react';
import { AccordionItem } from './AccordionItem';
import * as renderer from 'react-test-renderer';
import { isConformant } from '../../common/isConformant';
import { AccordionItemContext } from './AccordionItemContext';

describe('AccordionItem', () => {
isConformant({
Component: AccordionItem,
displayName: 'AccordionItem',
helperComponents: [AccordionItemContext.Provider],
// Accordion does not have own styles
disabledTests: ['make-styles-overrides-win'],
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
import * as React from 'react';
import { AccordionPanel } from './AccordionPanel';
import * as renderer from 'react-test-renderer';
import { mount } from 'enzyme';
import { isConformant } from '../../common/isConformant';
import { AccordionItemContext } from '../AccordionItem';
import type { MountRendererProps } from 'enzyme';

describe('AccordionPanel', () => {
const Wrapper: React.FC = props => (
<AccordionItemContext.Provider value={{ open: true, disabled: false, onHeaderClick: () => undefined }}>
{props.children}
</AccordionItemContext.Provider>
);

isConformant({
Component: AccordionPanel,
displayName: 'AccordionPanel',
customMount: (node: React.ReactElement, options?: MountRendererProps) =>
mount(node, {
...options,
wrappingComponent: AccordionItemContext.Provider,
wrappingComponentProps: { value: { open: true } },
}),
renderOptions: { wrapper: Wrapper },
});

/**
Expand Down
4 changes: 3 additions & 1 deletion packages/react-conformance-griffel/src/overridesWin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ export const overridesWin: ConformanceTest = (componentInfo, testInfo) => {
document.body.appendChild(container);
});

afterEach(() => {
afterEach(async () => {
if (container) {
const ReactDOM = await import('react-dom');
ReactDOM.unmountComponentAtNode(container);
Copy link
Member Author

Choose a reason for hiding this comment

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

After removing the "nuclear" document.body.innerHTML = '' cleanup in react-conformance, the incomplete cleanup here was causing subsequent tests to fail if the component rendered anything outside of the container node (such as PopoverSurface rendering a Portal).

document.body.removeChild(container);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/react-conformance-griffel/tsconfig.lib.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"lib": ["DOM", "ES2019"],
"outDir": "dist",
"declaration": true,
"types": ["static-assets", "environment", "jest"],
"types": ["static-assets", "environment", "jest", "node"],
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to add this to make the build pass after enzyme types and their indirect node type leakage were removed

"module": "CommonJS"
},
"exclude": ["**/*.spec.ts", "**/*.spec.tsx", "**/*.test.ts", "**/*.test.tsx"],
Expand Down
7 changes: 0 additions & 7 deletions packages/react-conformance/config/tests.js

This file was deleted.

5 changes: 1 addition & 4 deletions packages/react-conformance/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
const { createConfig } = require('@fluentui/scripts/jest/jest-resources');
const path = require('path');

const config = createConfig({
setupFiles: [path.resolve(path.join(__dirname, 'config', 'tests.js'))],
});
const config = createConfig({});

module.exports = config;
3 changes: 1 addition & 2 deletions packages/react-conformance/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
"peerDependencies": {
"@types/react": ">=16.8.0 <18.0.0",
"@types/react-dom": ">=16.8.0 <18.0.0",
"enzyme": "^3.0.0",
"enzyme-adapter-react-16": "^1.0.0",
"@testing-library/react": "^12.0.0",
"jest": "^26.0.0",
"react": ">=16.8.0 <18.0.0",
"react-dom": ">=16.8.0 <18.0.0",
Expand Down
Loading