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

Facepile (v8): convert tests to use testing-library #22282

Conversation

TristanWatanabe
Copy link
Member

Current Behavior

  • v8 Facepile tests don't use testing-library

New Behavior

  • Convert Facepile to fully use testing-library

Related Issue(s)

part of #21663

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 1, 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 52d9fd3:

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

@size-auditor
Copy link

size-auditor bot commented Apr 1, 2022

Asset size changes

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

Baseline commit: 69bcdf2f52fe6a8bf7d5f7ca71be74d753202356 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 1, 2022

📊 Bundle size report

🤖 This report was generated against 69bcdf2f52fe6a8bf7d5f7ca71be74d753202356

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 1, 2022

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 973 964 5000
Breadcrumb mount 2919 2884 1000
Checkbox mount 1614 1572 5000
CheckboxBase mount 1333 1384 5000
ChoiceGroup mount 5084 4989 5000
ComboBox mount 1035 1046 1000
CommandBar mount 11196 11237 1000
ContextualMenu mount 12280 12233 1000
DefaultButton mount 1224 1233 5000
DetailsRow mount 4123 4182 5000
DetailsRowFast mount 4119 4131 5000
DetailsRowNoStyles mount 3883 3834 5000
Dialog mount 2438 2389 1000
DocumentCardTitle mount 166 177 1000
Dropdown mount 3579 3530 5000
FocusTrapZone mount 1917 1915 5000
FocusZone mount 1891 1897 5000
IconButton mount 1877 1887 5000
Label mount 384 384 5000
Layer mount 3134 3108 5000
Link mount 516 492 5000
MenuButton mount 1592 1575 5000
MessageBar mount 2295 2253 5000
Nav mount 3554 3531 1000
OverflowSet mount 1161 1185 5000
Panel mount 2315 2247 1000
Persona mount 1100 1087 1000
Pivot mount 1616 1534 1000
PrimaryButton mount 1380 1370 5000
Rating mount 8297 8274 5000
SearchBox mount 1381 1415 5000
Shimmer mount 2713 2649 5000
Slider mount 2100 2052 5000
SpinButton mount 5385 5355 5000
Spinner mount 478 459 5000
SplitButton mount 3445 3388 5000
Stack mount 541 546 5000
StackWithIntrinsicChildren mount 2422 2449 5000
StackWithTextChildren mount 5597 5657 5000
SwatchColorPicker mount 12331 12403 5000
TagPicker mount 2862 2894 5000
TeachingBubble mount 99006 99129 5000
Text mount 451 489 5000
TextField mount 1515 1492 5000
ThemeProvider mount 1227 1275 5000
ThemeProvider virtual-rerender 676 682 5000
ThemeProvider virtual-rerender-with-unmount 1976 1998 5000
Toggle mount 865 849 5000
buttonNative mount 134 141 5000

@TristanWatanabe TristanWatanabe marked this pull request as ready for review April 1, 2022 15:18
@TristanWatanabe TristanWatanabe requested review from a team, Markionium and mtennoe as code owners April 1, 2022 15:18

const faces = findNodes(wrapper, '.ms-Facepile-person .ms-Persona-coin');
expect(addButton.firstElementChild!.firstElementChild!.className).toContain('ms-Persona--size32');
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way to get this that's less reliant on the current DOM structure, like by text or aria-label or even classname?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately no text or aria-label, went with querying for a classname instead

@TristanWatanabe
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

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.

5 participants