diff --git a/change/@fluentui-react-conformance-7d889654-5e80-427c-b5ac-8437beabc7a1.json b/change/@fluentui-react-conformance-7d889654-5e80-427c-b5ac-8437beabc7a1.json new file mode 100644 index 00000000000000..05573da80d2d68 --- /dev/null +++ b/change/@fluentui-react-conformance-7d889654-5e80-427c-b5ac-8437beabc7a1.json @@ -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": "elcraig@microsoft.com", + "dependentChangeType": "none" +} diff --git a/change/@fluentui-react-conformance-griffel-0e193d59-759e-48f2-a8a2-d0b3fb5bfb55.json b/change/@fluentui-react-conformance-griffel-0e193d59-759e-48f2-a8a2-d0b3fb5bfb55.json new file mode 100644 index 00000000000000..39c093c1aff4b6 --- /dev/null +++ b/change/@fluentui-react-conformance-griffel-0e193d59-759e-48f2-a8a2-d0b3fb5bfb55.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Fix cleanup of overridesWin test", + "packageName": "@fluentui/react-conformance-griffel", + "email": "elcraig@microsoft.com", + "dependentChangeType": "none" +} diff --git a/packages/fluentui/react-northstar/test/specs/commonTests/isConformant.tsx b/packages/fluentui/react-northstar/test/specs/commonTests/isConformant.tsx index e1092c7dfbc7e4..c286351db36643 100644 --- a/packages/fluentui/react-northstar/test/specs/commonTests/isConformant.tsx +++ b/packages/fluentui/react-northstar/test/specs/commonTests/isConformant.tsx @@ -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 extends Pick, 'disabledTests' | 'testOptions'> { +export interface Conformant + extends Pick, 'disabledTests' | 'testOptions' | 'getTargetElement'> { /** Path to the test file. */ testPath: string; constructorName?: string; @@ -27,6 +34,8 @@ export interface Conformant extends Pick { @@ -8,5 +8,7 @@ describe('FormButton', () => { constructorName: 'FormButton', forwardsRefTo: `Button`, targetComponent: Button, + getTargetElement: (result, attr) => + attr === 'className' ? result.container.querySelector(`.${formButtonClassName}`) : result.getByRole('button'), }); }); diff --git a/packages/fluentui/react-northstar/test/specs/components/Form/FormCheckbox-test.tsx b/packages/fluentui/react-northstar/test/specs/components/Form/FormCheckbox-test.tsx index 17f6447f406e95..aa3d817ed4e83d 100644 --- a/packages/fluentui/react-northstar/test/specs/components/Form/FormCheckbox-test.tsx +++ b/packages/fluentui/react-northstar/test/specs/components/Form/FormCheckbox-test.tsx @@ -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', () => { @@ -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'), }); }); diff --git a/packages/fluentui/react-northstar/test/specs/components/Form/FormDatepicker-test.tsx b/packages/fluentui/react-northstar/test/specs/components/Form/FormDatepicker-test.tsx index 8e95819732feb9..3d2974082dbdb1 100644 --- a/packages/fluentui/react-northstar/test/specs/components/Form/FormDatepicker-test.tsx +++ b/packages/fluentui/react-northstar/test/specs/components/Form/FormDatepicker-test.tsx @@ -1,6 +1,6 @@ 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, { @@ -8,5 +8,9 @@ describe('FormDatepicker', () => { constructorName: 'FormDatepicker', forwardsRefTo: false, targetComponent: Datepicker, + getTargetElement: (result, attr) => + attr === 'className' + ? result.container.querySelector(`.${formDatepickerClassName}`) + : result.container.querySelector(`.${datepickerClassName}`), }); }); diff --git a/packages/fluentui/react-northstar/test/specs/components/Form/FormDropdown-test.tsx b/packages/fluentui/react-northstar/test/specs/components/Form/FormDropdown-test.tsx index 7e00f4b32f3cbd..07b15ebb1c2626 100644 --- a/packages/fluentui/react-northstar/test/specs/components/Form/FormDropdown-test.tsx +++ b/packages/fluentui/react-northstar/test/specs/components/Form/FormDropdown-test.tsx @@ -1,6 +1,6 @@ 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, { @@ -8,5 +8,9 @@ describe('FormDropdown', () => { constructorName: 'FormDropdown', forwardsRefTo: false, targetComponent: Dropdown, + getTargetElement: (result, attr) => + attr === 'className' + ? result.container.querySelector(`.${formDropdownClassName}`) + : result.container.querySelector(`.${dropdownClassName}`), }); }); diff --git a/packages/fluentui/react-northstar/test/specs/components/Form/FormInput-test.tsx b/packages/fluentui/react-northstar/test/specs/components/Form/FormInput-test.tsx index dece9026185454..d68dfdd66ff1e6 100644 --- a/packages/fluentui/react-northstar/test/specs/components/Form/FormInput-test.tsx +++ b/packages/fluentui/react-northstar/test/specs/components/Form/FormInput-test.tsx @@ -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', () => { @@ -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'), }); }); diff --git a/packages/fluentui/react-northstar/test/specs/components/Form/FormRadioGroup-test.tsx b/packages/fluentui/react-northstar/test/specs/components/Form/FormRadioGroup-test.tsx index 835aac1b7579ab..35329ff6ee5c8c 100644 --- a/packages/fluentui/react-northstar/test/specs/components/Form/FormRadioGroup-test.tsx +++ b/packages/fluentui/react-northstar/test/specs/components/Form/FormRadioGroup-test.tsx @@ -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', () => { @@ -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'), }); }); diff --git a/packages/fluentui/react-northstar/test/specs/components/Form/FormSlider-test.tsx b/packages/fluentui/react-northstar/test/specs/components/Form/FormSlider-test.tsx index ab8abf1272f3fd..cd083233447fa1 100644 --- a/packages/fluentui/react-northstar/test/specs/components/Form/FormSlider-test.tsx +++ b/packages/fluentui/react-northstar/test/specs/components/Form/FormSlider-test.tsx @@ -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, { @@ -15,5 +15,9 @@ describe('FormSlider', () => { onKeyPress: 'input', onKeyUp: 'input', }, + getTargetElement: (result, attr) => + attr === 'className' + ? result.container.querySelector(`.${formSliderClassName}`) + : result.container.querySelector(`.${sliderClassName}`), }); }); diff --git a/packages/fluentui/react-northstar/test/specs/components/Form/FormTextArea-test.tsx b/packages/fluentui/react-northstar/test/specs/components/Form/FormTextArea-test.tsx index ef2cde2b33c4fb..c360b4a7f9de75 100644 --- a/packages/fluentui/react-northstar/test/specs/components/Form/FormTextArea-test.tsx +++ b/packages/fluentui/react-northstar/test/specs/components/Form/FormTextArea-test.tsx @@ -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'; @@ -9,5 +9,7 @@ describe('FormTextArea', () => { constructorName: 'FormTextArea', forwardsRefTo: false, targetComponent: TextArea, + getTargetElement: (result, attr) => + attr === 'className' ? result.container.querySelector(`.${formTextAreaClassName}`) : result.getByRole('textbox'), }); }); diff --git a/packages/fluentui/react-northstar/test/specs/components/Menu/MenuItem-test.tsx b/packages/fluentui/react-northstar/test/specs/components/Menu/MenuItem-test.tsx index dc1551a20ae3d9..731146585e7a2f 100644 --- a/packages/fluentui/react-northstar/test/specs/components/Menu/MenuItem-test.tsx +++ b/packages/fluentui/react-northstar/test/specs/components/Menu/MenuItem-test.tsx @@ -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'] } }, }); diff --git a/packages/fluentui/react-northstar/test/specs/components/Toolbar/ToolbarMenuItem-test.ts b/packages/fluentui/react-northstar/test/specs/components/Toolbar/ToolbarMenuItem-test.ts index 0cfa92a4db57f5..f284fa1351d3c9 100644 --- a/packages/fluentui/react-northstar/test/specs/components/Toolbar/ToolbarMenuItem-test.ts +++ b/packages/fluentui/react-northstar/test/specs/components/Toolbar/ToolbarMenuItem-test.ts @@ -2,7 +2,7 @@ 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, { @@ -10,6 +10,7 @@ describe('ToolbarMenuItem', () => { wrapperComponent: Box, autoControlledProps: ['menuOpen'], constructorName: 'ToolbarMenuItem', + getTargetElement: result => result.container.querySelector(`.${toolbarMenuItemClassName}`), }); implementsShorthandProp(ToolbarMenuItem)('menu', ToolbarMenu, { implementsPopper: true, diff --git a/packages/fluentui/react-northstar/test/specs/components/Toolbar/ToolbarMenuRadioGroup-test.ts b/packages/fluentui/react-northstar/test/specs/components/Toolbar/ToolbarMenuRadioGroup-test.ts index 3db98819d15cd3..4e361f4bd50ff2 100644 --- a/packages/fluentui/react-northstar/test/specs/components/Toolbar/ToolbarMenuRadioGroup-test.ts +++ b/packages/fluentui/react-northstar/test/specs/components/Toolbar/ToolbarMenuRadioGroup-test.ts @@ -8,6 +8,7 @@ describe('ToolbarMenuRadioGroup', () => { testPath: __filename, wrapperComponent: ToolbarMenuRadioGroupWrapper, constructorName: 'ToolbarMenuRadioGroup', + getTargetElement: result => result.container.querySelector(`[data-aa-class="ToolbarMenuRadioGroup"]`), }); describe('accessibility', () => { diff --git a/packages/react-accordion/src/components/Accordion/Accordion.test.tsx b/packages/react-accordion/src/components/Accordion/Accordion.test.tsx index 77a5d8676e661b..d5216f292cdde2 100644 --- a/packages/react-accordion/src/components/Accordion/Accordion.test.tsx +++ b/packages/react-accordion/src/components/Accordion/Accordion.test.tsx @@ -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'], }); diff --git a/packages/react-accordion/src/components/AccordionHeader/AccordionHeader.test.tsx b/packages/react-accordion/src/components/AccordionHeader/AccordionHeader.test.tsx index 92d682c8d2eb82..625f2398a04341 100644 --- a/packages/react-accordion/src/components/AccordionHeader/AccordionHeader.test.tsx +++ b/packages/react-accordion/src/components/AccordionHeader/AccordionHeader.test.tsx @@ -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'; @@ -13,7 +12,6 @@ describe('AccordionHeader', () => { isConformant({ Component: AccordionHeader, displayName: 'AccordionHeader', - helperComponents: [AccordionHeaderContext.Provider], testOptions: { 'has-static-classnames': [ { diff --git a/packages/react-accordion/src/components/AccordionItem/AccordionItem.test.tsx b/packages/react-accordion/src/components/AccordionItem/AccordionItem.test.tsx index 5d5b57565b38db..6da9384d908b8b 100644 --- a/packages/react-accordion/src/components/AccordionItem/AccordionItem.test.tsx +++ b/packages/react-accordion/src/components/AccordionItem/AccordionItem.test.tsx @@ -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'], }); diff --git a/packages/react-accordion/src/components/AccordionPanel/AccordionPanel.test.tsx b/packages/react-accordion/src/components/AccordionPanel/AccordionPanel.test.tsx index ce86790665b2c1..004cf68dc4ac04 100644 --- a/packages/react-accordion/src/components/AccordionPanel/AccordionPanel.test.tsx +++ b/packages/react-accordion/src/components/AccordionPanel/AccordionPanel.test.tsx @@ -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 => ( + undefined }}> + {props.children} + + ); + isConformant({ Component: AccordionPanel, displayName: 'AccordionPanel', - customMount: (node: React.ReactElement, options?: MountRendererProps) => - mount(node, { - ...options, - wrappingComponent: AccordionItemContext.Provider, - wrappingComponentProps: { value: { open: true } }, - }), + renderOptions: { wrapper: Wrapper }, }); /** diff --git a/packages/react-conformance-griffel/src/overridesWin.ts b/packages/react-conformance-griffel/src/overridesWin.ts index 0cad8908afee5f..3adc5a6b1f2371 100644 --- a/packages/react-conformance-griffel/src/overridesWin.ts +++ b/packages/react-conformance-griffel/src/overridesWin.ts @@ -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); document.body.removeChild(container); } diff --git a/packages/react-conformance-griffel/tsconfig.lib.json b/packages/react-conformance-griffel/tsconfig.lib.json index 86d60d269c39ca..77c308eb5ad7bc 100644 --- a/packages/react-conformance-griffel/tsconfig.lib.json +++ b/packages/react-conformance-griffel/tsconfig.lib.json @@ -5,7 +5,7 @@ "lib": ["DOM", "ES2019"], "outDir": "dist", "declaration": true, - "types": ["static-assets", "environment", "jest"], + "types": ["static-assets", "environment", "jest", "node"], "module": "CommonJS" }, "exclude": ["**/*.spec.ts", "**/*.spec.tsx", "**/*.test.ts", "**/*.test.tsx"], diff --git a/packages/react-conformance/config/tests.js b/packages/react-conformance/config/tests.js deleted file mode 100644 index 3882d3702ddc99..00000000000000 --- a/packages/react-conformance/config/tests.js +++ /dev/null @@ -1,7 +0,0 @@ -/** Jest test setup file. */ - -const { configure } = require('enzyme'); -const Adapter = require('enzyme-adapter-react-16'); - -// Configure enzyme. -configure({ adapter: new Adapter() }); diff --git a/packages/react-conformance/jest.config.js b/packages/react-conformance/jest.config.js index 8e6760391cdb8e..ddecc5348fd564 100644 --- a/packages/react-conformance/jest.config.js +++ b/packages/react-conformance/jest.config.js @@ -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; diff --git a/packages/react-conformance/package.json b/packages/react-conformance/package.json index 85496f5fe52af3..61391f3f848e55 100644 --- a/packages/react-conformance/package.json +++ b/packages/react-conformance/package.json @@ -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", diff --git a/packages/react-conformance/src/defaultErrorMessages.tsx b/packages/react-conformance/src/defaultErrorMessages.tsx index d0016f97ce2219..753e1eaf6cf6ae 100644 --- a/packages/react-conformance/src/defaultErrorMessages.tsx +++ b/packages/react-conformance/src/defaultErrorMessages.tsx @@ -4,6 +4,7 @@ import { EOL } from 'os'; import * as path from 'path'; import { errorMessageColors, formatArray, getErrorMessage, formatErrors, getPackagePath } from './utils/index'; +import { prettyDOM } from '@testing-library/react'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -155,9 +156,7 @@ export const defaultErrorMessages = { // Possible solutions: // 1. Check if your component has a ref prop. // 2. For function components, make sure that you are using forwardRef. - // 3. Check if your component passes ref to an inner component and add targetComponent to isConformant in - // your test file. - // 4. If your component uses an "elementRef" prop instead of "ref", add elementRefName: 'elementRef' to + // 3. If your component uses an "elementRef" prop instead of "ref", add elementRefName: 'elementRef' to // isConformant in your test file. return getErrorMessage({ displayName, @@ -165,8 +164,6 @@ export const defaultErrorMessages = { suggestions: [ `Check if your component has a ${resolveInfo('ref')} prop.`, `For function components, make sure that you are using ${resolveInfo('forwardRef')}.`, - `Check if your component passes the ref to an inner component and add ${resolveInfo('targetComponent')} ` + - 'to isConformant in your test file.', `If your component uses an "elementRef" prop instead of "ref", add ${resolveInfo( `elementRefName: 'elementRef'`, )} to isConformant in your test file.`, @@ -186,7 +183,7 @@ export const defaultErrorMessages = { // 1. Make sure you're applying the ref to the root element in your component. // 2. Check if your component uses an element ref and add elementRefName: 'elementRef' to isConformant in // your test file. - // 3. Check if your component passes ref to an inner component and add targetComponent to isConformant in + // 3. Check if your component passes ref to an inner component and add getTargetElement to isConformant in // your test file. return getErrorMessage({ displayName, @@ -200,7 +197,7 @@ export const defaultErrorMessages = { `elementRefName: 'elementRef'`, )} to isConformant in your test file.`, `Check if your component passes ref to an inner component and add ${resolveInfo( - `targetComponent`, + `getTargetElement`, )} to isConformant in your test file.`, ], error, @@ -227,7 +224,7 @@ export const defaultErrorMessages = { overview: `has a custom 'size' prop but also applies 'size' as a native prop.`, details: [ `After passing 'size="${sizeValue}"' to ${testErrorName(displayName)} it was applied to this element:`, - appliedToElement.outerHTML, + prettyDOM(appliedToElement) as string, ], suggestions: [ `Ensure 'size' is omitted when calling native props helpers.`, @@ -242,15 +239,14 @@ export const defaultErrorMessages = { error: Error, testClassName: string, classNames: string[] | undefined, - componentHTML: string, + rootEl: HTMLElement, ) => { const { displayName } = testInfo; const { testErrorInfo, resolveInfo, failedError, testErrorName } = errorMessageColors; - // Show part of the HTMl in the debug message if possible - const debugHTML = componentHTML.includes(testClassName) - ? componentHTML.substr(0, componentHTML.indexOf(testClassName) + 50) + '...' - : ''; + // Show part of the HTML in the debug message if possible + const elementWithClass = rootEl.getElementsByClassName(testClassName)[0]; + const debugHTML = elementWithClass ? prettyDOM(elementWithClass, 50) + '...' : ''; // Message Description: Handles scenario where className prop doesn't exist or isn't applied on the component // @@ -305,6 +301,8 @@ export const defaultErrorMessages = { // // Possible solutions: // 1. Check the placement of your className and ensure that it is merged with defaults. + // 2. Check if your component passes className to an inner element and add getTargetElement to + // isConformant in your test file. return getErrorMessage({ displayName, overview: `overwrites default classNames with the user-supplied className.`, @@ -320,6 +318,8 @@ export const defaultErrorMessages = { ], suggestions: [ `Check the placement of your className and ensure that it is ${resolveInfo('merged')} with defaults.`, + `Check if your component passes className to an inner element and add ${resolveInfo('getTargetElement')} ` + + 'to isConformant in your test file.', ], error, }); @@ -476,7 +476,7 @@ export const defaultErrorMessages = { testInfo: IsConformantOptions, error: Error, componentClassName: string, - classNames: string, + classNames: string[], ) => { const { displayName } = testInfo; const { testErrorInfo, resolveInfo, failedError } = errorMessageColors; @@ -484,7 +484,10 @@ export const defaultErrorMessages = { return getErrorMessage({ displayName, overview: `does not have default className (${testErrorInfo(componentClassName)}).`, - details: [`After render it has the following classes:`, ` ${failedError(`className='${classNames}'`)}`], + details: [ + `After render it has the following classes:`, + ` ${failedError(`className='${classNames.join(' ')}'`)}`, + ], suggestions: [ `Ensure that your component has default a className and it is ${resolveInfo('merged')} with other classNames.`, ], @@ -521,7 +524,6 @@ export const defaultErrorMessages = { 'component-has-static-classnames-object-exported': ( testInfo: IsConformantOptions, error: Error, - componentClassName: string, exportName: string, ) => { const { componentPath, displayName } = testInfo; @@ -545,7 +547,6 @@ export const defaultErrorMessages = { 'component-has-static-classnames-in-correct-format': ( testInfo: IsConformantOptions, error: Error, - componentClassName: string, exportName: string, ) => { const { componentPath, displayName } = testInfo; @@ -571,15 +572,29 @@ export const defaultErrorMessages = { error: Error, componentName: string, missingClassNames: string, + rootEl: HTMLElement, ) => { const { displayName } = testInfo; - const { testErrorInfo, failedError } = errorMessageColors; + const { testErrorInfo, failedError, resolveInfo } = errorMessageColors; return getErrorMessage({ displayName, overview: `missing one or more static classNames on component (${testErrorInfo(componentName)}).`, - details: [`Missing the following classes after render:`, ` ${failedError(missingClassNames)}`], - suggestions: [`Ensure that each slot of the component has its corresponding static className.`], + details: [ + `Missing the following classes after render:`, + ` ${failedError(missingClassNames)}`, + 'Actual HTML:', + prettyDOM(rootEl) as string, + ], + suggestions: [ + `Ensure that each slot of the component has its corresponding static className.`, + `If the component is rendered in a portal, add ${resolveInfo( + `getTargetElement`, + )} to isConformant in your test file.`, + `If the component requires certain props to render all slots, add ${resolveInfo( + 'staticClassNames.props', + )} to isConformant in your test file. (You can add multiple prop combinations.)`, + ], error, }); }, diff --git a/packages/react-conformance/src/defaultTests.tsx b/packages/react-conformance/src/defaultTests.tsx index 9715fbe8e3912c..e260634e92d8aa 100644 --- a/packages/react-conformance/src/defaultTests.tsx +++ b/packages/react-conformance/src/defaultTests.tsx @@ -1,15 +1,29 @@ +import * as React from 'react'; +import * as _ from 'lodash'; +import * as path from 'path'; +import { render } from '@testing-library/react'; + import { TestObject, IsConformantOptions } from './types'; import { defaultErrorMessages } from './defaultErrorMessages'; import { ComponentDoc } from 'react-docgen-typescript'; -import { getComponent, getPackagePath, getCallbackArguments, validateCallbackArguments } from './utils/index'; -import { mount } from 'enzyme'; +import { getPackagePath, getCallbackArguments, validateCallbackArguments } from './utils/index'; import { act } from 'react-dom/test-utils'; -import * as React from 'react'; -import * as _ from 'lodash'; -import * as path from 'path'; - const CALLBACK_REGEX = /^on(?!Render[A-Z])[A-Z]/; +const DEFAULT_CLASSNAME_PREFIX = 'fui-'; + +/** + * Find the target element where the attribute is applied using either `getTargetElement`, + * or the first child of the container. + */ +function getTargetElement( + testInfo: IsConformantOptions, + ...[result, attr]: Parameters['getTargetElement']> +) { + return testInfo.getTargetElement + ? testInfo.getTargetElement(result, attr) + : (result.container.firstElementChild as HTMLElement); +} /* eslint-disable @typescript-eslint/naming-convention */ @@ -36,9 +50,8 @@ export const defaultTests: TestObject = { 'component-renders': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => { it(`renders (component-renders)`, () => { try { - const { requiredProps, Component, customMount = mount } = testInfo; - const mountedComponent = customMount(); - expect(mountedComponent.exists()).toBeTruthy(); + const { requiredProps, Component, renderOptions } = testInfo; + expect(() => render(, renderOptions)).not.toThrow(); } catch (e) { throw new Error(defaultErrorMessages['component-renders'](testInfo, e)); } @@ -70,23 +83,19 @@ export const defaultTests: TestObject = { /** Component handles ref */ 'component-handles-ref': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => { it(`handles ref (component-handles-ref)`, () => { - try { - const { Component, requiredProps, elementRefName = 'ref', targetComponent, customMount = mount } = testInfo; - const rootRef = React.createRef(); - const mergedProps: Partial<{}> = { - ...requiredProps, - [elementRefName]: rootRef, - }; + // This test simply verifies that the passed ref is applied to an element *anywhere* in the DOM + const { Component, requiredProps, elementRefName = 'ref', renderOptions } = testInfo; + const rootRef = React.createRef(); + const mergedProps: Partial<{}> = { + ...requiredProps, + [elementRefName]: rootRef, + }; - act(() => { - targetComponent - ? customMount().find(targetComponent) - : customMount(); + const { baseElement } = render(, renderOptions); - expect(rootRef.current).toBeTruthy(); - // Ref should resolve to an HTML element. - expect(rootRef.current?.getAttribute).toBeTruthy(); - }); + try { + expect(rootRef.current).toBeInstanceOf(HTMLElement); + expect(baseElement.contains(rootRef.current)).toBe(true); } catch (e) { throw new Error(defaultErrorMessages['component-handles-ref'](testInfo, e)); } @@ -96,45 +105,31 @@ export const defaultTests: TestObject = { /** Component has ref applied to the root component DOM node */ 'component-has-root-ref': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => { it(`applies ref to root element (component-has-root-ref)`, () => { - try { - const { - customMount = mount, - Component, - requiredProps, - helperComponents = [], - wrapperComponent, - elementRefName = 'ref', - targetComponent, - primarySlot = 'root', - } = testInfo; - - const rootRef = React.createRef(); - const mergedProps: Partial<{}> = { - ...requiredProps, - ...(primarySlot !== 'root' - ? { - // If primarySlot is something other than 'root', add the ref to - // the root slot rather than to the component's props. - root: { ref: rootRef }, - } - : { - [elementRefName]: rootRef, - }), - }; + const { renderOptions, Component, requiredProps, elementRefName = 'ref', primarySlot = 'root' } = testInfo; - const el = targetComponent - ? customMount().find(targetComponent) - : customMount(); - - act(() => { - const component = getComponent(el, helperComponents, wrapperComponent); - - // Do an instanceof check first because if `ref` returns a class instance, the toBe check - // will print out the very long stringified version in the error (which isn't helpful) - expect(rootRef.current).toBeInstanceOf(HTMLElement); + const rootRef = React.createRef(); + const mergedProps: Partial<{}> = { + ...requiredProps, + ...(primarySlot !== 'root' + ? { + // If primarySlot is something other than 'root', add the ref to + // the root slot rather than to the component's props. + root: { ref: rootRef }, + } + : { + [elementRefName]: rootRef, + }), + }; + + const result = render(, renderOptions); + const refEl = getTargetElement(testInfo, result, 'ref'); + expect(refEl).toBeTruthy(); - expect(rootRef.current).toBe(component.getDOMNode()); - }); + try { + // Do an instanceof check first because if `ref` returns a class instance, the toBe check + // will print out the very long stringified version in the error (which isn't helpful) + expect(rootRef.current).toBeInstanceOf(HTMLElement); + expect(rootRef.current).toBe(refEl); } catch (e) { throw new Error(defaultErrorMessages['component-has-root-ref'](testInfo, e)); } @@ -167,7 +162,7 @@ export const defaultTests: TestObject = { } it(`does not apply native size prop if custom one is defined (omits-size-prop)`, () => { - const { customMount = mount, Component, requiredProps, helperComponents = [], wrapperComponent } = testInfo; + const { renderOptions, Component, requiredProps } = testInfo; const size = sizeLiteralMatch?.[1] || 'foo'; const mergedProps = { @@ -176,9 +171,8 @@ export const defaultTests: TestObject = { // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; // we know the size prop is supported but there's not a good way to derive the actual type - const el = customMount(); - const component = getComponent(el, helperComponents, wrapperComponent); - const elementWithSize = component.getDOMNode().querySelector('[size]'); + const { baseElement } = render(, renderOptions); + const elementWithSize = baseElement.querySelector('[size]'); try { expect(elementWithSize).toBeFalsy(); @@ -190,9 +184,10 @@ export const defaultTests: TestObject = { /** Component file handles classname prop */ 'component-handles-classname': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => { - const { Component, wrapperComponent, helperComponents = [], requiredProps, customMount = mount } = testInfo; + const { Component, requiredProps, renderOptions } = testInfo; const testClassName = 'testComponentClassName'; let handledClassName = false; + let defaultClassNames: string[]; // eslint-disable-next-line @typescript-eslint/no-explicit-any const mergedProps: any = { @@ -200,46 +195,43 @@ export const defaultTests: TestObject = { className: testClassName, }; + it('has default classNames', () => { + // this is not a real test, it's just to get the default class names without causing + // possible side effects within another test by rendering the component twice + const defaultResult = render(, renderOptions); + const defaultEl = getTargetElement(testInfo, defaultResult, 'className'); + defaultClassNames = classListToStrings(defaultEl.classList); + }); + it(`handles className prop (component-handles-classname)`, () => { - const el = customMount(); - const component = getComponent(el, helperComponents, wrapperComponent); - const domNode = component.getDOMNode(); - const classNames = (domNode.getAttribute('class') || '').split(' '); + const result = render(, renderOptions); + const domNode = getTargetElement(testInfo, result, 'className'); + expect(domNode).toBeTruthy(); + const classNames = classListToStrings(domNode.classList); try { expect(classNames).toContain(testClassName); handledClassName = true; } catch (e) { throw new Error( - defaultErrorMessages['component-handles-classname']( - testInfo, - e, - testClassName, - classNames, - domNode.outerHTML, - ), + defaultErrorMessages['component-handles-classname'](testInfo, e, testClassName, classNames, domNode), ); } }); it(`preserves component's default classNames (component-preserves-default-classname)`, () => { - if (!handledClassName) { - return; // don't run this test if the main className test failed + if (!handledClassName || !defaultClassNames?.length) { + return; // don't run this test if the main className test failed or there are no defaults } - const defaultEl = customMount(); - const defaultComponent = getComponent(defaultEl, helperComponents, wrapperComponent); - const defaultClassNames = defaultComponent.getDOMNode().getAttribute('class')?.split(' ') || []; - const el = customMount(); - const component = getComponent(el, helperComponents, wrapperComponent); - const classNames = (component.getDOMNode().getAttribute('class') || '').split(' '); + const result = render(, renderOptions); + const el = getTargetElement(testInfo, result, 'className'); + const classNames = classListToStrings(el.classList); let defaultClassName: string = ''; try { - if (defaultClassNames.length && defaultClassNames[0]) { - for (defaultClassName of defaultClassNames) { - expect(classNames).toContain(defaultClassName); - } + for (defaultClassName of defaultClassNames) { + expect(classNames).toContain(defaultClassName); } } catch (e) { throw new Error( @@ -258,20 +250,18 @@ export const defaultTests: TestObject = { /** Component file has assigned and exported static class */ 'component-has-static-classname': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => { const { - componentPath, Component, - wrapperComponent, - helperComponents = [], requiredProps, - customMount = mount, + renderOptions, + testOptions: { 'component-has-static-classname': { prefix = DEFAULT_CLASSNAME_PREFIX } = {} } = {}, } = testInfo; - const componentClassName = `fui-${componentInfo.displayName}`; + const componentClassName = `${prefix}${componentInfo.displayName}`; it(`has static classname (component-has-static-classname)`, () => { - const defaultEl = customMount(); - - const defaultComponent = getComponent(defaultEl, helperComponents, wrapperComponent); - const classNames = defaultComponent.prop('className'); + const result = render(, renderOptions); + const rootEl = getTargetElement(testInfo, result, 'className'); + expect(rootEl).toBeTruthy(); + const classNames = classListToStrings(rootEl.classList); try { expect(classNames).toContain(componentClassName); @@ -281,12 +271,20 @@ export const defaultTests: TestObject = { ); } }); + }, - it(`static classname is exported at top-level (component-has-static-classname-exported)`, () => { - if (testInfo.isInternal) { - return; - } + 'component-has-static-classname-exported': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => { + if (testInfo.isInternal) { + return; + } + + const { + componentPath, + testOptions: { 'component-has-static-classname': { prefix = DEFAULT_CLASSNAME_PREFIX } = {} } = {}, + } = testInfo; + const componentClassName = `${prefix}${componentInfo.displayName}`; + it(`static classname is exported at top-level (component-has-static-classname-exported)`, () => { const exportName = componentInfo.displayName.slice(0, 1).toLowerCase() + componentInfo.displayName.slice(1) + 'ClassName'; @@ -303,14 +301,7 @@ export const defaultTests: TestObject = { }, 'component-has-static-classnames-object': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => { - const { - componentPath, - Component, - wrapperComponent, - helperComponents = [], - requiredProps, - customMount = mount, - } = testInfo; + const { componentPath, Component, requiredProps, renderOptions } = testInfo; const componentName = componentInfo.displayName; const componentClassName = `fui-${componentName}`; @@ -330,12 +321,7 @@ export const defaultTests: TestObject = { handledClassNamesObjectExport = true; } catch (e) { throw new Error( - defaultErrorMessages['component-has-static-classnames-object-exported']( - testInfo, - e, - componentClassName, - exportName, - ), + defaultErrorMessages['component-has-static-classnames-object-exported'](testInfo, e, exportName), ); } }); @@ -360,12 +346,7 @@ export const defaultTests: TestObject = { expect(classNamesFromFile).toEqual(expectedClassNames); } catch (e) { throw new Error( - defaultErrorMessages['component-has-static-classnames-in-correct-format']( - testInfo, - e, - componentClassName, - exportName, - ), + defaultErrorMessages['component-has-static-classnames-in-correct-format'](testInfo, e, exportName), ); } }); @@ -383,27 +364,27 @@ export const defaultTests: TestObject = { ...requiredProps, ...staticClassNames.props, }; - const defaultEl = customMount(); - const defaultComponent = getComponent(defaultEl, helperComponents, wrapperComponent); + const result = render(, renderOptions); + const rootEl = getTargetElement(testInfo, result, 'className'); const indexFile = require(indexPath); const classNamesFromFile = indexFile[exportName]; const expectedClassNames: { [key: string]: string } = staticClassNames.expectedClassNames ?? classNamesFromFile; - const missingClassNames = Object.values(expectedClassNames).reduce((acc, className) => { - if (defaultComponent.find(`.${className}`).length < 1) { - (acc as string[]).push(className); - } - return acc; - }, []); + const missingClassNames = Object.values(expectedClassNames).filter( + className => !rootEl.classList.contains(className) && !rootEl.querySelector(`.${className}`), + ); - if (missingClassNames.length > 0) { + try { + expect(missingClassNames).toHaveLength(0); + } catch (e) { throw new Error( defaultErrorMessages['component-has-static-classnames']( testInfo, - new Error(), + e, componentName, missingClassNames.join(', '), + rootEl, ), ); } @@ -563,17 +544,10 @@ export const defaultTests: TestObject = { 'primary-slot-gets-native-props': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => { it(`applies correct native props to the primary and root slots (primary-slot-gets-native-props)`, () => { try { - const { - customMount = mount, - Component, - requiredProps, - helperComponents = [], - wrapperComponent, - targetComponent, - primarySlot = 'root', - } = testInfo; + const { Component, requiredProps, primarySlot = 'root', renderOptions } = testInfo; // This test only applies if this component has a primary slot other than 'root' + // (this also prevents the test from running for northstar and v8) if (primarySlot === 'root') { return; } @@ -599,19 +573,11 @@ export const defaultTests: TestObject = { [testDataAttribute]: testDataAttribute, }; - const el = targetComponent - ? customMount().find(targetComponent) - : customMount(); + const { container } = render(, renderOptions); + const rootNode = container.firstElementChild as HTMLElement; + expect(rootNode).toBeTruthy(); act(() => { - const component = getComponent(el, helperComponents, wrapperComponent); - - const rootNode = component.getDOMNode(); - expect(rootNode).toBeInstanceOf(HTMLElement); - if (!(rootNode instanceof HTMLElement)) { - return; - } - // Find the node that represents the primary slot, searching for its data attribute const primaryNode = rootNode.querySelector(`[${primarySlotDataTag}]`); @@ -622,10 +588,10 @@ export const defaultTests: TestObject = { } // className and style should go the *root* slot - expect(rootNode.className).toContain(testClass); + expect(classListToStrings(rootNode.classList)).toContain(testClass); expect(rootNode.style.fontFamily).toEqual(testStyleFontFamily); // ... and not the primary slot - expect(primaryNode.className).not.toContain(testClass); + expect(classListToStrings(primaryNode.classList)).not.toContain(testClass); expect(primaryNode.style.fontFamily).not.toEqual(testStyleFontFamily); // Ref and all other native props should go to the *primary* slot @@ -641,3 +607,15 @@ export const defaultTests: TestObject = { }); }, }; + +function classListToStrings(classList: DOMTokenList): string[] { + // We should be able to just do [...classList] but that requires lib: dom.iterable in tsconfig.json. + // Due to path aliases, react-conformance gets type-checked with each converged package, + // so we'd need to add dom.iterable in all converged packages too. + // This function is a workaround. + const result: string[] = []; + for (let i = 0; i < classList.length; i++) { + result.push(classList[i]); + } + return result; +} diff --git a/packages/react-conformance/src/index.ts b/packages/react-conformance/src/index.ts index 7e1ab65f4ed4fc..a901708e93cc36 100644 --- a/packages/react-conformance/src/index.ts +++ b/packages/react-conformance/src/index.ts @@ -1,3 +1,2 @@ export { isConformant } from './isConformant'; -export * from './types'; -export { getComponent } from './utils/index'; +export type { ConformanceTest, IsConformantOptions, TestObject, TestOptions } from './types'; diff --git a/packages/react-conformance/src/isConformant.ts b/packages/react-conformance/src/isConformant.ts index 41abe227afbbd6..734e1cb0090828 100644 --- a/packages/react-conformance/src/isConformant.ts +++ b/packages/react-conformance/src/isConformant.ts @@ -11,11 +11,6 @@ export function isConformant(...testInfo: Partial { - afterEach(() => { - // TODO: remove this once cleanup is properly implemented or after moving to testing-library - document.body.innerHTML = ''; - }); - if (!fs.existsSync(componentPath)) { throw new Error(`Path ${componentPath} does not exist`); } diff --git a/packages/react-conformance/src/types.ts b/packages/react-conformance/src/types.ts index eb65c180fae0c7..f505ea3350f17c 100644 --- a/packages/react-conformance/src/types.ts +++ b/packages/react-conformance/src/types.ts @@ -2,7 +2,7 @@ import * as React from 'react'; import { ComponentDoc } from 'react-docgen-typescript'; import * as ts from 'typescript'; -import { mount, ComponentType } from 'enzyme'; +import { render } from '@testing-library/react'; /** * Individual test options @@ -22,6 +22,10 @@ export interface TestOptions { [key: string]: string; }; }[]; + 'component-has-static-classname'?: { + /** Prefix for the classname, if not `fui-` */ + prefix?: string; + }; } export interface IsConformantOptions { @@ -38,9 +42,9 @@ export interface IsConformantOptions { */ displayName: string; /** - * In case that the mount from enzyme does not work for the component, a custom mount function can be provided. + * Custom options passed to `@testing-library/react`'s `render` method. */ - customMount?: typeof mount; + renderOptions?: Parameters[1]; /** * If there are tests that aren't supposed to run on a component, this allows to opt out of any test. */ @@ -67,14 +71,6 @@ export interface IsConformantOptions { * Allows specific test options. */ testOptions?: TestOptions; - /** - * 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[]; /** * An alternative name for the ref prop which resolves to * the root element (e.g. `elementRef`). @@ -82,9 +78,12 @@ export interface IsConformantOptions { */ elementRefName?: string; /** - * Child component that will receive unhandledProps. + * Get the element that will receive native props (or the specified native prop, if it matters) */ - targetComponent?: ComponentType; + getTargetElement?: ( + renderResult: ReturnType, + attr: keyof React.AllHTMLAttributes | 'ref' | `data-${string}`, + ) => HTMLElement; /** * The name of the slot designated as "primary", which receives native props passed to the component. * This is 'root' by default, and only needs to be specified if it's a slot other than 'root'. diff --git a/packages/react-conformance/src/utils/getComponent.ts b/packages/react-conformance/src/utils/getComponent.ts deleted file mode 100644 index aaafce1b1a8675..00000000000000 --- a/packages/react-conformance/src/utils/getComponent.ts +++ /dev/null @@ -1,32 +0,0 @@ -import * as React from 'react'; -import { ReactWrapper } from 'enzyme'; - -const getDisplayName = (Component: React.ElementType) => { - return ( - (Component as React.ComponentType).displayName || - (Component as React.ComponentType).name || - (typeof Component === 'string' && Component.length > 0 ? Component : 'Unknown') - ); -}; - -const toNextNonTrivialChild = (from: ReactWrapper, helperComponentNames: string[]): ReactWrapper => { - const current = from.childAt(0); - - return current && helperComponentNames.includes(current.name()) - ? toNextNonTrivialChild(current, helperComponentNames) - : current; -}; - -export const getComponent = ( - wrapper: ReactWrapper, - helperComponents: React.ElementType[], - wrapperComponent?: React.ElementType, -) => { - const helperComponentNames = [...helperComponents, ...(wrapperComponent ? [wrapperComponent] : [])].map( - getDisplayName, - ); - const componentElement = toNextNonTrivialChild(wrapper, helperComponentNames); - // in that case 'topLevelChildElement' we've found so far is a wrapper's topmost child - // thus, we should continue search - return wrapperComponent ? toNextNonTrivialChild(componentElement, helperComponentNames) : componentElement; -}; diff --git a/packages/react-conformance/src/utils/index.ts b/packages/react-conformance/src/utils/index.ts index 8cefc1688874bd..417145e92af8c0 100644 --- a/packages/react-conformance/src/utils/index.ts +++ b/packages/react-conformance/src/utils/index.ts @@ -1,5 +1,4 @@ export * from './errorMessages'; export * from './getCallbackArguments'; -export * from './getComponent'; export { getPackagePath } from './getPackagePath'; export * from './validateCallbackArguments'; diff --git a/packages/react-focus/src/common/isConformant.ts b/packages/react-focus/src/common/isConformant.ts index c8704335599c23..3acdaa57d2eeb2 100644 --- a/packages/react-focus/src/common/isConformant.ts +++ b/packages/react-focus/src/common/isConformant.ts @@ -5,11 +5,12 @@ export function isConformant( testInfo: Omit, 'componentPath'> & { componentPath?: string }, ) { const defaultOptions: Partial> = { - disabledTests: [ - 'kebab-aria-attributes', - // Focus* components don't have static classes - 'component-has-static-classname', - ], + disabledTests: ['kebab-aria-attributes', 'component-has-static-classname-exported'], + testOptions: { + 'component-has-static-classname': { + prefix: 'ms-', + }, + }, componentPath: module!.parent!.filename.replace('.test', ''), }; diff --git a/packages/react-menu/src/components/Menu/Menu.test.tsx b/packages/react-menu/src/components/Menu/Menu.test.tsx index 84e830aacafc00..3f4e4a050c5838 100644 --- a/packages/react-menu/src/components/Menu/Menu.test.tsx +++ b/packages/react-menu/src/components/Menu/Menu.test.tsx @@ -20,6 +20,7 @@ describe('Menu', () => { 'component-handles-classname', 'component-has-static-classname', 'component-has-static-classnames-object', + 'component-has-static-classname-exported', // Menu does not have own styles 'make-styles-overrides-win', ], diff --git a/packages/react-menu/src/components/MenuList/MenuList.test.tsx b/packages/react-menu/src/components/MenuList/MenuList.test.tsx index 029e4b0f290087..ae15108fc7eefb 100644 --- a/packages/react-menu/src/components/MenuList/MenuList.test.tsx +++ b/packages/react-menu/src/components/MenuList/MenuList.test.tsx @@ -4,13 +4,12 @@ import * as renderer from 'react-test-renderer'; import { render } from '@testing-library/react'; import { useHasParentContext } from '@fluentui/react-context-selector'; import { isConformant } from '../../common/isConformant'; -import { MenuListContext, MenuListProvider } from '../../contexts/menuListContext'; +import { MenuListContext } from '../../contexts/menuListContext'; describe('MenuList', () => { isConformant({ Component: MenuList, displayName: 'MenuList', - helperComponents: [MenuListProvider], disabledTests: [ // MenuTrigger does not have own styles 'make-styles-overrides-win', diff --git a/packages/react-menu/src/components/MenuPopover/MenuPopover.test.tsx b/packages/react-menu/src/components/MenuPopover/MenuPopover.test.tsx index 369366111ff0a0..e0b6725bd8d1a9 100644 --- a/packages/react-menu/src/components/MenuPopover/MenuPopover.test.tsx +++ b/packages/react-menu/src/components/MenuPopover/MenuPopover.test.tsx @@ -1,14 +1,17 @@ import * as React from 'react'; import { fireEvent, render } from '@testing-library/react'; -import { Portal } from '@fluentui/react-portal'; import { MenuPopover } from './MenuPopover'; import { isConformant } from '../../common/isConformant'; +import { MenuPopoverProps } from './MenuPopover.types'; describe('MenuPopover', () => { + const testid = 'test'; + isConformant({ Component: MenuPopover, displayName: 'MenuPopover', - helperComponents: [Portal, 'span'], + requiredProps: { 'data-testid': testid } as MenuPopoverProps, + getTargetElement: result => result.getByTestId(testid), }); it('renders a default state', () => { @@ -24,7 +27,6 @@ describe('MenuPopover', () => { ['onKeyDown', fireEvent.keyDown], ])('should pass original %s handler to menu popup', (handler, trigger) => { // Arrange - const testid = 'test'; const spy = jest.fn(); const props = { [handler]: spy }; const { getByTestId } = render( diff --git a/packages/react-menu/src/components/MenuTrigger/MenuTrigger.test.tsx b/packages/react-menu/src/components/MenuTrigger/MenuTrigger.test.tsx index 0deb6f7a764af9..bbf1b7cc6632df 100644 --- a/packages/react-menu/src/components/MenuTrigger/MenuTrigger.test.tsx +++ b/packages/react-menu/src/components/MenuTrigger/MenuTrigger.test.tsx @@ -20,6 +20,7 @@ describe('MenuTrigger', () => { 'component-handles-classname', 'component-has-static-classname', 'component-has-static-classnames-object', + 'component-has-static-classname-exported', // MenuTrigger does not have own styles 'make-styles-overrides-win', ], diff --git a/packages/react-popover/src/components/Popover/Popover.test.tsx b/packages/react-popover/src/components/Popover/Popover.test.tsx index 7eccbcc4079da6..ef2231346dc5bf 100644 --- a/packages/react-popover/src/components/Popover/Popover.test.tsx +++ b/packages/react-popover/src/components/Popover/Popover.test.tsx @@ -8,6 +8,7 @@ describe('Popover', () => { isConformant({ Component: Popover, displayName: 'Popover', + requiredProps: { children:
hello
}, disabledTests: [ // Popover does not render DOM elements 'component-handles-ref', @@ -15,6 +16,7 @@ describe('Popover', () => { 'component-handles-classname', 'component-has-static-classname', 'component-has-static-classnames-object', + 'component-has-static-classname-exported', // Popover does not have own styles 'make-styles-overrides-win', ], diff --git a/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx b/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx index 875d38e10fe25d..28e20767799a20 100644 --- a/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx +++ b/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx @@ -3,16 +3,22 @@ import * as React from 'react'; import { PopoverSurface } from './PopoverSurface'; import { render, fireEvent } from '@testing-library/react'; import { isConformant } from '../../common/isConformant'; -import { Portal } from '@fluentui/react-portal'; import { mockPopoverContext } from '../../common/mockUsePopoverContext'; +import { PopoverSurfaceProps } from './PopoverSurface.types'; jest.mock('../../popoverContext'); describe('PopoverSurface', () => { + // PopoverSurface is rendered by a Portal so won't be available in the rendered container + const testid = 'component'; + // also include an aria-label to prevent warnings in debug mode + const props = { 'data-testid': testid, 'aria-label': 'test' }; + isConformant({ Component: PopoverSurface, displayName: 'PopoverSurface', - helperComponents: [Portal, 'span'], + requiredProps: props as PopoverSurfaceProps, + getTargetElement: result => result.getByTestId(testid), }); beforeEach(() => { @@ -20,14 +26,11 @@ describe('PopoverSurface', () => { mockPopoverContext({}); }); - // PopoverSurface is rendered by a Portal so won't be available in the rendered container - const testid = 'component'; - /** * Note: see more visual regression tests for PopoverSurface in /apps/vr-tests. */ it('renders a default state', () => { - const { getByTestId } = render(Default PopoverSurface); + const { getByTestId } = render(Default PopoverSurface); expect(getByTestId(testid)).toMatchSnapshot(); }); @@ -39,7 +42,7 @@ describe('PopoverSurface', () => { // Arrange const spy = jest.fn(); const { getByTestId } = render( - + Content , ); @@ -54,7 +57,7 @@ describe('PopoverSurface', () => { it('should set aria-modal true if focus trap is active', () => { // Arrange mockPopoverContext({ trapFocus: true }); - const { getByTestId } = render(Content); + const { getByTestId } = render(Content); // Assert expect(getByTestId(testid).getAttribute('aria-modal')).toEqual('true'); @@ -63,7 +66,7 @@ describe('PopoverSurface', () => { it('should set role dialog if focus trap is active', () => { // Arrange mockPopoverContext({ trapFocus: true }); - const { queryByRole } = render(Content); + const { queryByRole } = render(Content); // Assert expect(queryByRole('dialog')).not.toBeNull(); @@ -72,7 +75,7 @@ describe('PopoverSurface', () => { it('should set role complementary if focus trap is not active', () => { // Arrange mockPopoverContext({ trapFocus: false }); - const { getByTestId } = render(Content); + const { getByTestId } = render(Content); // Assert expect(getByTestId(testid)).not.toBeNull(); diff --git a/packages/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap b/packages/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap index c1bb15e2e6c8eb..89ee2483bdd5c7 100644 --- a/packages/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap +++ b/packages/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap @@ -2,6 +2,7 @@ exports[`PopoverSurface renders a default state 1`] = `
{ 'component-handles-classname', 'component-has-static-classname', 'component-has-static-classnames-object', + 'component-has-static-classname-exported', // PopoverTrigger does not have own styles 'make-styles-overrides-win', ], diff --git a/packages/react-provider/src/components/FluentProvider/FluentProvider.test.tsx b/packages/react-provider/src/components/FluentProvider/FluentProvider.test.tsx index 3791ec1e295395..be52bebf70eab8 100644 --- a/packages/react-provider/src/components/FluentProvider/FluentProvider.test.tsx +++ b/packages/react-provider/src/components/FluentProvider/FluentProvider.test.tsx @@ -1,10 +1,8 @@ import { resetIdsForTests } from '@fluentui/react-utilities'; -import { TextDirectionProvider } from '@griffel/react'; import * as React from 'react'; import { FluentProvider } from './FluentProvider'; import * as renderer from 'react-test-renderer'; import { isConformant } from '../../common/isConformant'; -import { ProviderContext } from '@fluentui/react-shared-contexts'; describe('FluentProvider', () => { // eslint-disable-next-line @typescript-eslint/no-empty-function @@ -18,7 +16,6 @@ describe('FluentProvider', () => { disabledTests: ['component-handles-classname'], Component: FluentProvider, displayName: 'FluentProvider', - helperComponents: [ProviderContext.Provider, TextDirectionProvider], }); afterEach(() => { diff --git a/packages/react-radio/src/components/Radio/Radio.test.tsx b/packages/react-radio/src/components/Radio/Radio.test.tsx index 37747ee2f1ba3a..9200123a60076c 100644 --- a/packages/react-radio/src/components/Radio/Radio.test.tsx +++ b/packages/react-radio/src/components/Radio/Radio.test.tsx @@ -5,6 +5,8 @@ import { isConformant } from '../../common/isConformant'; import { Radio } from './Radio'; describe('Radio', () => { + const noOp = () => undefined; + isConformant({ Component: Radio, displayName: 'Radio', @@ -63,13 +65,13 @@ describe('Radio', () => { }); it('respects checked', () => { - const { getByRole } = render(); + const { getByRole } = render(); expect((getByRole('radio') as HTMLInputElement).checked).toBe(true); }); it('respects checked updates', () => { - const { rerender, getByRole } = render(); - rerender(); + const { rerender, getByRole } = render(); + rerender(); expect((getByRole('radio') as HTMLInputElement).checked).toBe(false); }); diff --git a/packages/react-radio/src/components/RadioGroup/RadioGroup.test.tsx b/packages/react-radio/src/components/RadioGroup/RadioGroup.test.tsx index a3b5834b4f1a3b..fe7052774f1086 100644 --- a/packages/react-radio/src/components/RadioGroup/RadioGroup.test.tsx +++ b/packages/react-radio/src/components/RadioGroup/RadioGroup.test.tsx @@ -168,7 +168,7 @@ describe('RadioGroup', () => { const { getByDisplayValue } = render( - + undefined} /> , ); diff --git a/packages/react-slider/src/components/Slider/Slider.test.tsx b/packages/react-slider/src/components/Slider/Slider.test.tsx index 047d247a74cbff..91d7ec01d6dc8d 100644 --- a/packages/react-slider/src/components/Slider/Slider.test.tsx +++ b/packages/react-slider/src/components/Slider/Slider.test.tsx @@ -9,7 +9,7 @@ describe('Slider', () => { Component: Slider, displayName: 'Slider', primarySlot: 'input', - disabledTests: ['kebab-aria-attributes', 'component-has-static-classname'], + disabledTests: ['component-has-static-classname', 'component-has-static-classname-exported'], }); afterEach(() => { diff --git a/packages/react-toolbar/src/components/Toolbar/Toolbar.test.tsx b/packages/react-toolbar/src/components/Toolbar/Toolbar.test.tsx index 49f94005155d19..257d57a7b235a1 100644 --- a/packages/react-toolbar/src/components/Toolbar/Toolbar.test.tsx +++ b/packages/react-toolbar/src/components/Toolbar/Toolbar.test.tsx @@ -7,7 +7,7 @@ describe('Toolbar', () => { isConformant({ Component: Toolbar, displayName: 'Toolbar', - disabledTests: ['component-has-static-classname'], + disabledTests: ['component-has-static-classname', 'component-has-static-classname-exported'], }); // TODO add more tests here, and create visual regression tests in /apps/vr-tests diff --git a/packages/react-toolbar/src/components/ToolbarButton/ToolbarButton.test.tsx b/packages/react-toolbar/src/components/ToolbarButton/ToolbarButton.test.tsx index b0453398bd876f..df61fa61e2b18d 100644 --- a/packages/react-toolbar/src/components/ToolbarButton/ToolbarButton.test.tsx +++ b/packages/react-toolbar/src/components/ToolbarButton/ToolbarButton.test.tsx @@ -7,7 +7,7 @@ describe('ToolbarButton', () => { isConformant({ Component: ToolbarButton, displayName: 'ToolbarButton', - disabledTests: ['component-has-static-classname'], + disabledTests: ['component-has-static-classname', 'component-has-static-classname-exported'], }); // TODO add more tests here, and create visual regression tests in /apps/vr-tests diff --git a/packages/react-toolbar/src/components/ToolbarDivider/ToolbarDivider.test.tsx b/packages/react-toolbar/src/components/ToolbarDivider/ToolbarDivider.test.tsx index 939e3ba07191c8..7104cf4c6479eb 100644 --- a/packages/react-toolbar/src/components/ToolbarDivider/ToolbarDivider.test.tsx +++ b/packages/react-toolbar/src/components/ToolbarDivider/ToolbarDivider.test.tsx @@ -7,7 +7,7 @@ describe('ToolbarDivider', () => { isConformant({ Component: ToolbarDivider, displayName: 'ToolbarDivider', - disabledTests: ['component-has-static-classname'], + disabledTests: ['component-has-static-classname', 'component-has-static-classname-exported'], }); // TODO add more tests here, and create visual regression tests in /apps/vr-tests diff --git a/packages/react-tooltip/src/components/Tooltip/Tooltip.test.tsx b/packages/react-tooltip/src/components/Tooltip/Tooltip.test.tsx index a2ae9db500f464..0f6f3fc6189ee0 100644 --- a/packages/react-tooltip/src/components/Tooltip/Tooltip.test.tsx +++ b/packages/react-tooltip/src/components/Tooltip/Tooltip.test.tsx @@ -34,9 +34,7 @@ describe('Tooltip', () => { 'component-handles-classname', 'component-has-static-classname', 'component-has-static-classnames-object', - 'as-renders-fc', - 'as-passes-as-value', - 'as-renders-html', + 'component-has-static-classname-exported', ], }); diff --git a/packages/react/src/common/isConformant.ts b/packages/react/src/common/isConformant.ts index 54d81ceacc108b..756b618ea0065a 100644 --- a/packages/react/src/common/isConformant.ts +++ b/packages/react/src/common/isConformant.ts @@ -7,8 +7,11 @@ export function isConformant( const defaultOptions: Partial> = { disabledTests: [ 'kebab-aria-attributes', - // Disabled as v8 has different prefix + // v8 has a different prefix, and there's a setting for that now, + // but a lot of components don't set a consistent/expected classname 'component-has-static-classname', + // v8 doesn't export classnames + 'component-has-static-classname-exported', // Will enable with appropriate overrides separately 'consistent-callback-names', 'consistent-callback-args', diff --git a/packages/react/src/components/Callout/Callout.test.tsx b/packages/react/src/components/Callout/Callout.test.tsx index d7f31077f2eb90..581f18c64d775c 100644 --- a/packages/react/src/components/Callout/Callout.test.tsx +++ b/packages/react/src/components/Callout/Callout.test.tsx @@ -25,7 +25,6 @@ describe('Callout', () => { isConformant({ Component: Callout, displayName: 'Callout', - targetComponent: CalloutContent, requiredProps: { doNotLayer: true }, disabledTests: ['component-handles-classname'], }); diff --git a/packages/react/src/components/ContextualMenu/ContextualMenu.test.tsx b/packages/react/src/components/ContextualMenu/ContextualMenu.test.tsx index dad3b627ffc613..c0b1a45a8b5772 100644 --- a/packages/react/src/components/ContextualMenu/ContextualMenu.test.tsx +++ b/packages/react/src/components/ContextualMenu/ContextualMenu.test.tsx @@ -4,7 +4,6 @@ import * as ReactTestUtils from 'react-dom/test-utils'; import { KeyCodes } from '../../Utilities'; import { FocusZoneDirection } from '../../FocusZone'; import * as renderer from 'react-test-renderer'; -import { CalloutContent } from '../Callout/CalloutContent'; import { ContextualMenu } from './ContextualMenu'; import { canAnyMenuItemsCheck } from './ContextualMenu.base'; import { ContextualMenuItemType } from './ContextualMenu.types'; @@ -39,17 +38,16 @@ describe('ContextualMenu', () => { isConformant({ Component: ContextualMenu, displayName: 'ContextualMenu', - targetComponent: CalloutContent, + getTargetElement: (result, attr) => + attr === 'className' + ? // className goes on a FocusZone inside the callout + result.baseElement.querySelector('[data-focuszone-id]')! + : // generally the CalloutContent's root element gets native props and ref + (result.baseElement.getElementsByClassName('ms-Callout-container')[0] as HTMLElement), requiredProps: { hidden: false, items: [{ text: 'test', key: 'Today', secondaryText: '7:00 PM', ariaLabel: 'foo' }], }, - // TODO: ContextualMenu handles ref but doesn't pass: - // expect(rootRef.current?.getAttribute).toBeDefined(); - // - // This test failure likely stems from Callout as it experiences the same error. The failing test should be - // further investigated and re-enabled in a future pull request. - disabledTests: ['component-handles-ref', 'component-handles-classname'], }); it('allows setting aria-label per ContextualMenuItem', () => { diff --git a/packages/react/src/components/HoverCard/HoverCard.test.tsx b/packages/react/src/components/HoverCard/HoverCard.test.tsx index b243dd596cbaac..f20b886ce6c843 100644 --- a/packages/react/src/components/HoverCard/HoverCard.test.tsx +++ b/packages/react/src/components/HoverCard/HoverCard.test.tsx @@ -80,8 +80,6 @@ describe('HoverCard', () => { Component: HoverCard, displayName: 'HoverCard', componentPath: path.join(__dirname, 'HoverCard.ts'), - // cast due to slight mismatch in style props - targetComponent: ExpandingCardBase as React.ComponentType, // Problem: Ref doesn't match DOM node and returns outermost wrapper div. // Solution: Ensure ref is passed correctly to the root element. disabledTests: ['component-has-root-ref', 'component-handles-ref'], diff --git a/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap b/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap index cee41ef36f036a..bbf74a577a39e4 100644 --- a/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap +++ b/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap @@ -2,11 +2,7 @@ exports[`Modal renders Draggable Modal correctly 1`] = `