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

Callout, Dialog, Modal: convert tests to use testing-library #21664

Merged
merged 4 commits into from
Feb 10, 2022

Conversation

ecraig12345
Copy link
Member

Current Behavior

Callout, Dialog, and Modal's tests use a mix of enzyme, react-test-renderer, and react-dom, along with assorted odd patterns such as unnecessary snapshot testing.

New Behavior

Convert the tests to all use testing-library. In some cases I also tweaked what the tests were doing to be simpler or to test things more directly/effectively: for example, replacing some of the snapshot tests with direct tests of the relevant DOM.

Please review snapshot updates with whitespace changes hidden!

I noticed that some of the tests were having issues due to conformance tests not fully cleaning up. As a temporary workaround in react-conformance (until it's also converted to use testing-library), I added an afterEach which does document.body.innerHTML = ''.

Related Issue(s)

Part of #21663

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 9, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit eba0ae2:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@size-auditor
Copy link

size-auditor bot commented Feb 9, 2022

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: e07cc826b280603418e2e8bc79b161e459de9741 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Feb 9, 2022

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-accordion
Accordion (including children components)
54.905 kB
17.567 kB
react-avatar
Avatar
44.89 kB
13.02 kB
react-badge
Badge
20.831 kB
6.533 kB
react-badge
CounterBadge
21.699 kB
6.827 kB
react-badge
PresenceBadge
21.785 kB
6.507 kB
react-button
Button
27.932 kB
8.025 kB
react-button
CompoundButton
33.196 kB
8.987 kB
react-button
MenuButton
29.584 kB
8.598 kB
react-button
SplitButton
35.729 kB
9.676 kB
react-button
ToggleButton
37.209 kB
8.627 kB
react-card
Card - All
47.867 kB
14.195 kB
react-card
Card
43.56 kB
13.004 kB
react-card
CardFooter
7.615 kB
3.23 kB
react-card
CardHeader
8.893 kB
3.675 kB
react-card
CardPreview
7.588 kB
3.255 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
165.744 kB
46.848 kB
react-components
react-components: FluentProvider & webLightTheme
32.479 kB
10.625 kB
react-divider
Divider
15.256 kB
5.511 kB
react-image
Image
10.067 kB
3.934 kB
react-input
Input
21.5 kB
7.116 kB
react-label
Label
8.303 kB
3.472 kB
react-link
Link
11.064 kB
4.487 kB
react-menu
Menu (including children components)
102.914 kB
31.69 kB
react-menu
Menu (including selectable components)
105.269 kB
32.056 kB
react-popover
Popover
95.841 kB
29.229 kB
react-portal
Portal
6.249 kB
2.163 kB
react-provider
FluentProvider
13.962 kB
5.231 kB
react-select
Select
7.716 kB
3.24 kB
react-slider
Slider
22.928 kB
7.747 kB
react-spinner
Spinner
6.773 kB
2.879 kB
react-switch
Switch
25.387 kB
8.204 kB
react-text
Text - Default
10.755 kB
4.212 kB
react-text
Text - Wrappers
14.067 kB
4.558 kB
react-tooltip
Tooltip
42.479 kB
14.593 kB
🤖 This report was generated against e07cc826b280603418e2e8bc79b161e459de9741

@fabricteam
Copy link
Collaborator

fabricteam commented Feb 9, 2022

Perf Analysis (@fluentui/react)

Scenario Render type Master Ticks PR Ticks Iterations Status
ContextualMenu mount 7297 36414 1000 Possible regression
FluentProviderWithTheme mount 133 148 10 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 749 750 5000
BaseButton mount 806 766 5000
Breadcrumb mount 2344 2287 1000
ButtonNext mount 423 418 5000
Checkbox mount 1318 1342 5000
CheckboxBase mount 1151 1107 5000
ChoiceGroup mount 4149 4139 5000
ComboBox mount 855 859 1000
CommandBar mount 8874 8898 1000
ContextualMenu mount 7297 36414 1000 Possible regression
DefaultButton mount 1497 989 5000
DetailsRow mount 3190 3198 5000
DetailsRowFast mount 4194 3203 5000
DetailsRowNoStyles mount 3110 3036 5000
Dialog mount 1902 1925 1000
DocumentCardTitle mount 151 165 1000
Dropdown mount 2773 3542 5000
FluentProviderNext mount 1649 1605 5000
FluentProviderWithTheme mount 133 148 10 Possible regression
FluentProviderWithTheme virtual-rerender 102 104 10
FluentProviderWithTheme virtual-rerender-with-unmount 169 168 10
FocusTrapZone mount 1616 1509 5000
FocusZone mount 1537 1560 5000
IconButton mount 1501 1499 5000
Label mount 318 317 5000
Layer mount 2590 2580 5000
Link mount 438 428 5000
MakeStyles mount 1492 1492 50000
MenuButton mount 1266 1272 5000
MessageBar mount 1769 1750 5000
Nav mount 2848 2829 1000
OverflowSet mount 968 977 5000
Panel mount 1847 1887 1000
Persona mount 761 767 1000
Pivot mount 1278 1258 1000
PrimaryButton mount 1090 1124 5000
Rating mount 6625 6668 5000
SearchBox mount 1184 1159 5000
Shimmer mount 2201 2191 5000
Slider mount 1694 1712 5000
SpinButton mount 4282 4328 5000
Spinner mount 379 397 5000
SplitButton mount 2744 2715 5000
Stack mount 475 469 5000
StackWithIntrinsicChildren mount 1990 2003 5000
StackWithTextChildren mount 4505 4510 5000
SwatchColorPicker mount 11901 9974 5000
TagPicker mount 2200 2235 5000
TeachingBubble mount 10874 11291 5000
Text mount 386 337 5000
TextField mount 1214 1245 5000
ThemeProvider mount 1020 1047 5000
ThemeProvider virtual-rerender 555 549 5000
ThemeProvider virtual-rerender-with-unmount 1607 1622 5000
Toggle mount 719 706 5000
buttonNative mount 141 125 5000

Perf Analysis (@fluentui/react-northstar)

⚠️ No perf measurements available

});

it('target MouseEvents does not throw exception', () => {
it('does not throw with target MouseEvent', () => {
const mouseEvent = document.createEvent('MouseEvent');
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I know you're just migrating from one testing library to another but should this be changed from document.createEvent.... to new MouseEvent()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, maybe it was due to the API not being available in the past? But I'm changing various other things like that.

isFocused = options.containsFocus;
restoreCalled = true;
};
const onRestoreFocus = jest.fn();
// In order to have eventlisteners that have been added to the window to be called the JSX needs
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment still relevant?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, will remove

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants