Callout, Dialog, Modal: convert tests to use testing-library #21664
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
12 tasks
|
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:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: e07cc826b280603418e2e8bc79b161e459de9741 (build) |
📊 Bundle size reportUnchanged fixtures
|
Perf Analysis (
|
| 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)
spmonahan
approved these changes
Feb 10, 2022
| }); | ||
|
|
||
| it('target MouseEvents does not throw exception', () => { | ||
| it('does not throw with target MouseEvent', () => { | ||
| const mouseEvent = document.createEvent('MouseEvent'); |
There was a problem hiding this comment.
Nit: I know you're just migrating from one testing library to another but should this be changed from document.createEvent.... to new MouseEvent()?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Is this comment still relevant?
ecraig12345
force-pushed
the
testing-lib-surfaces
branch
from
b855377
to
6187fc3
Compare
45 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
afterEachwhich doesdocument.body.innerHTML = ''.Related Issue(s)
Part of #21663