feat: improve react-18 tests #25758
Merged
feat: improve react-18 tests #25758
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
…rsion instead hardcoded folder name
Hotell
changed the title
Hotell
commented
Nov 22, 2022
| coverageDirectory: './coverage', | ||
| setupFilesAfterEnv: ['./config/tests.js'], | ||
| moduleNameMapper: { |
There was a problem hiding this comment.
previous setup was using path mappings for v9 which is has no effect
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "name": "@fluentui/react-18-tests-v8", | |||
| "description": "React 18 test application", | |||
| "version": "1.0.0", | |||
| "version": "8.0.0", | |||
There was a problem hiding this comment.
this is needed as tooling checks version and tweaks its behaviours based on that ( cypress )
| "@fluentui/scripts": "^1.0.0", | ||
| "@types/react": "18.0.14", | ||
| "@types/react-dom": "18.0.6", | ||
| "swc-loader": "^0.2.3" |
There was a problem hiding this comment.
swc-loader is in root package version already - single version policy conformance
| it('renders ContextualMenu when trigger button is clicked', () => { | ||
| mount( | ||
| <React.StrictMode> | ||
| <ContextualMenuExample /> |
There was a problem hiding this comment.
this uses implementation instead of having the same code on 3 places
|
|
||
| const providerSelector = '.fui-FluentProvider'; | ||
|
|
||
| describe('App with React 18', () => { |
There was a problem hiding this comment.
propagated these test to Root as they are rendering the App instead of manually re-creating Provider
| </React.StrictMode>, | ||
| ); | ||
|
|
||
| cy.get(providerSelector) |
There was a problem hiding this comment.
curious what's the purpose of testing this on both e2e(cypress) and unit(jest) level @TristanWatanabe @micahgodbolt as it creates duplication.
Also I'm not sure about robustness of this test. what has that className to do with react 18 compatibility ?
Note - same questions for v8 react-18-tests
There was a problem hiding this comment.
I would guess that it's currently a simple to to check that the app does not crash (i.e. provider exists). Honestly I would try to leverage our existing cypress tests instead instead of having separate tests just for react 18 🤔
| @@ -10,6 +10,19 @@ const applyV8WebpackConfig: (config: Configuration) => Configuration = require(' | |||
|
|
|||
| const isLocalRun = !process.env.DEPLOYURL; | |||
|
|
|||
| function isV8() { | |||
There was a problem hiding this comment.
context aware check instead of hard-coding path.
refactor later: ideally this shouln't be here but somewhere else
There was a problem hiding this comment.
probably should be some kind of utility like isV8ExecutionContext
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: c093fbbbcdabefd2603cbd840b039d583fb0ff0e (build) |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 1287 | 1298 | 5000 | |
| Button | mount | 915 | 931 | 5000 | |
| FluentProvider | mount | 1482 | 1491 | 5000 | |
| FluentProviderWithTheme | mount | 576 | 572 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 542 | 538 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 584 | 568 | 10 | |
| MakeStyles | mount | 1974 | 1968 | 50000 | |
| SpinButton | mount | 2320 | 2283 | 5000 |
|
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 84e0eab:
|
🕵 fluentuiv8 Open the Visual Regressions report to inspect the 2 screenshots✅ There was 0 screenshots added, 2 screenshots removed, 1037 screenshots unchanged, 0 screenshots with different dimensions and 0 screenshots with visible difference. unknown 2 screenshots
|
📊 Bundle size reportUnchanged fixtures
|
Perf Analysis (
|
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| TreeWith60ListItems.default | 143 | 122 | 1.17:1 |
| RefMinimalPerf.default | 199 | 186 | 1.07:1 |
| GridMinimalPerf.default | 305 | 288 | 1.06:1 |
| LayoutMinimalPerf.default | 329 | 309 | 1.06:1 |
| TextAreaMinimalPerf.default | 429 | 405 | 1.06:1 |
| FlexMinimalPerf.default | 257 | 248 | 1.04:1 |
| TextMinimalPerf.default | 313 | 301 | 1.04:1 |
| CardMinimalPerf.default | 472 | 460 | 1.03:1 |
| ListNestedPerf.default | 487 | 472 | 1.03:1 |
| LoaderMinimalPerf.default | 268 | 259 | 1.03:1 |
| ReactionMinimalPerf.default | 337 | 328 | 1.03:1 |
| StatusMinimalPerf.default | 616 | 599 | 1.03:1 |
| VideoMinimalPerf.default | 642 | 625 | 1.03:1 |
| AvatarMinimalPerf.default | 165 | 162 | 1.02:1 |
| ChatDuplicateMessagesPerf.default | 215 | 210 | 1.02:1 |
| ChatMinimalPerf.default | 639 | 627 | 1.02:1 |
| DividerMinimalPerf.default | 316 | 309 | 1.02:1 |
| FormMinimalPerf.default | 329 | 323 | 1.02:1 |
| HeaderSlotsPerf.default | 699 | 685 | 1.02:1 |
| ListMinimalPerf.default | 463 | 455 | 1.02:1 |
| MenuButtonMinimalPerf.default | 1369 | 1347 | 1.02:1 |
| PortalMinimalPerf.default | 141 | 138 | 1.02:1 |
| ProviderMergeThemesPerf.default | 1009 | 990 | 1.02:1 |
| ProviderMinimalPerf.default | 319 | 314 | 1.02:1 |
| SegmentMinimalPerf.default | 306 | 299 | 1.02:1 |
| SkeletonMinimalPerf.default | 307 | 300 | 1.02:1 |
| IconMinimalPerf.default | 582 | 569 | 1.02:1 |
| AnimationMinimalPerf.default | 479 | 472 | 1.01:1 |
| DialogMinimalPerf.default | 685 | 680 | 1.01:1 |
| ItemLayoutMinimalPerf.default | 981 | 970 | 1.01:1 |
| LabelMinimalPerf.default | 344 | 341 | 1.01:1 |
| ListWith60ListItems.default | 503 | 499 | 1.01:1 |
| PopupMinimalPerf.default | 567 | 559 | 1.01:1 |
| SliderMinimalPerf.default | 1239 | 1231 | 1.01:1 |
| SplitButtonMinimalPerf.default | 3311 | 3263 | 1.01:1 |
| TableMinimalPerf.default | 363 | 358 | 1.01:1 |
| CustomToolbarPrototype.default | 2162 | 2148 | 1.01:1 |
| TooltipMinimalPerf.default | 1911 | 1883 | 1.01:1 |
| TreeMinimalPerf.default | 706 | 700 | 1.01:1 |
| AttachmentMinimalPerf.default | 122 | 122 | 1:1 |
| AttachmentSlotsPerf.default | 884 | 881 | 1:1 |
| ButtonOverridesMissPerf.default | 1029 | 1024 | 1:1 |
| ChatWithPopoverPerf.default | 294 | 293 | 1:1 |
| CheckboxMinimalPerf.default | 1520 | 1527 | 1:1 |
| DropdownMinimalPerf.default | 2170 | 2170 | 1:1 |
| EmbedMinimalPerf.default | 2627 | 2631 | 1:1 |
| ListCommonPerf.default | 511 | 511 | 1:1 |
| MenuMinimalPerf.default | 739 | 741 | 1:1 |
| RosterPerf.default | 1710 | 1702 | 1:1 |
| RadioGroupMinimalPerf.default | 391 | 390 | 1:1 |
| TableManyItemsPerf.default | 1574 | 1577 | 1:1 |
| ToolbarMinimalPerf.default | 804 | 800 | 1:1 |
| CarouselMinimalPerf.default | 357 | 360 | 0.99:1 |
| ImageMinimalPerf.default | 333 | 338 | 0.99:1 |
| DatepickerMinimalPerf.default | 4671 | 4743 | 0.98:1 |
| DropdownManyItemsPerf.default | 540 | 551 | 0.98:1 |
| HeaderMinimalPerf.default | 315 | 321 | 0.98:1 |
| InputMinimalPerf.default | 842 | 856 | 0.98:1 |
| AlertMinimalPerf.default | 221 | 229 | 0.97:1 |
| BoxMinimalPerf.default | 295 | 305 | 0.97:1 |
| ButtonMinimalPerf.default | 134 | 138 | 0.97:1 |
| ButtonSlotsPerf.default | 420 | 432 | 0.97:1 |
| AccordionMinimalPerf.default | 122 | 129 | 0.95:1 |
🕵 fluentuiv9 Open the Visual Regressions report to inspect the 2 screenshots✅ There was 0 screenshots added, 0 screenshots removed, 1749 screenshots unchanged, 0 screenshots with different dimensions and 2 screenshots with visible difference. unknown 2 screenshots
|
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 1207 | 1179 | 5000 | |
| Breadcrumb | mount | 2781 | 2818 | 1000 | |
| Checkbox | mount | 2646 | 2695 | 5000 | |
| CheckboxBase | mount | 2383 | 2390 | 5000 | |
| ChoiceGroup | mount | 4309 | 4298 | 5000 | |
| ComboBox | mount | 1181 | 1198 | 1000 | |
| CommandBar | mount | 9376 | 9313 | 1000 | |
| ContextualMenu | mount | 10473 | 10614 | 1000 | |
| DefaultButton | mount | 1366 | 1377 | 5000 | |
| DetailsRow | mount | 3375 | 3383 | 5000 | |
| DetailsRowFast | mount | 3412 | 3391 | 5000 | |
| DetailsRowNoStyles | mount | 3241 | 3249 | 5000 | |
| Dialog | mount | 2980 | 2978 | 1000 | |
| DocumentCardTitle | mount | 582 | 585 | 1000 | |
| Dropdown | mount | 3191 | 3177 | 5000 | |
| FocusTrapZone | mount | 1976 | 1972 | 5000 | |
| FocusZone | mount | 1929 | 1909 | 5000 | |
| GroupedList | mount | 1854 | 2073 | 2 | |
| GroupedList | virtual-rerender | 1107 | 1132 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 1639 | 1610 | 2 | |
| GroupedListV2 | mount | 556 | 572 | 2 | |
| GroupedListV2 | virtual-rerender | 541 | 553 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 570 | 569 | 2 | |
| IconButton | mount | 1801 | 1807 | 5000 | |
| Label | mount | 760 | 753 | 5000 | |
| Layer | mount | 4179 | 4228 | 5000 | |
| Link | mount | 862 | 872 | 5000 | |
| MenuButton | mount | 1615 | 1604 | 5000 | |
| MessageBar | mount | 2322 | 2307 | 5000 | |
| Nav | mount | 3094 | 3085 | 1000 | |
| OverflowSet | mount | 1416 | 1416 | 5000 | |
| Panel | mount | 2541 | 2540 | 1000 | |
| Persona | mount | 1285 | 1298 | 1000 | |
| Pivot | mount | 1517 | 1518 | 1000 | |
| PrimaryButton | mount | 1487 | 1493 | 5000 | |
| Rating | mount | 7011 | 7032 | 5000 | |
| SearchBox | mount | 1511 | 1512 | 5000 | |
| Shimmer | mount | 2934 | 2958 | 5000 | |
| Slider | mount | 2126 | 2117 | 5000 | |
| SpinButton | mount | 4331 | 4274 | 5000 | |
| Spinner | mount | 821 | 841 | 5000 | |
| SplitButton | mount | 2947 | 2833 | 5000 | |
| Stack | mount | 867 | 859 | 5000 | |
| StackWithIntrinsicChildren | mount | 2359 | 2354 | 5000 | |
| StackWithTextChildren | mount | 5063 | 5042 | 5000 | |
| SwatchColorPicker | mount | 9559 | 9546 | 5000 | |
| TagPicker | mount | 2345 | 2332 | 5000 | |
| TeachingBubble | mount | 76837 | 75904 | 5000 | |
| Text | mount | 808 | 831 | 5000 | |
| TextField | mount | 1543 | 1547 | 5000 | |
| ThemeProvider | mount | 1447 | 1447 | 5000 | |
| ThemeProvider | virtual-rerender | 1141 | 1152 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1998 | 2010 | 5000 | |
| Toggle | mount | 1139 | 1140 | 5000 | |
| buttonNative | mount | 534 | 538 | 5000 |
…normalize all test scenarios
Hotell
force-pushed
the
hotell/build/tweak-react-18-tests
branch
from
df82d58 to
cba9aba
Compare
…2e and normalize all test scenarios
…,test,e2e and normalize all test scenarios
Hotell
commented
Nov 24, 2022
| @@ -9,12 +12,18 @@ module.exports = { | |||
| globals: { | |||
| 'ts-jest': { | |||
| tsConfig: '<rootDir>/tsconfig.spec.json', | |||
| diagnostics: false, | |||
| diagnostics: { warnOnly: true, exclude: ['packages/**'] }, | |||
There was a problem hiding this comment.
this exposed lot of warnings from v8 packages - caused by not following isolateModules.
this will resolve it #25774
Hotell
added
Fluent UI react-components (v9)
Fluent UI react (v8)
Hotell
commented
Nov 24, 2022
| }, | ||
| }, | ||
| module: { | ||
| rules: [ | ||
| { | ||
| test: /\.tsx?$/, | ||
| exclude: /node_modules/, | ||
| use: 'swc-loader', | ||
| use: { |
There was a problem hiding this comment.
v8 cannot be compiled with swc, switching to esbuild for now swc-project/swc#2037
ling1726
reviewed
Nov 24, 2022
| }, | ||
| }, | ||
| { | ||
| test: /\.scss$/, |
There was a problem hiding this comment.
why are sass and css loaders only added now?
There was a problem hiding this comment.
previously v8 needed to build everything prior to run. also it was misconfigured to use v9 path aliases etc. more info is in PR description
ling1726
reviewed
Nov 24, 2022
| @@ -10,6 +10,19 @@ const applyV8WebpackConfig: (config: Configuration) => Configuration = require(' | |||
|
|
|||
| const isLocalRun = !process.env.DEPLOYURL; | |||
|
|
|||
| function isV8() { | |||
| const packageJsonPath = path.join(process.cwd(), 'package.json'); | |||
There was a problem hiding this comment.
this assumes that the cwd is in the root of a package, perhaps use findUp ?
There was a problem hiding this comment.
curios why ? cypress is always invoked from root of a package
There was a problem hiding this comment.
is that always the case? I've worked on component packages before and ran yarn somewhere from src
There was a problem hiding this comment.
I see, well that's definitely not supported behaviour as it adds magic to our DX. you should execute task always from root.
ATM the dev flow we support is like this:
yarn workspace <package-name> <npm-script>recommendedcd packages/<package> && yarn <npm-script>not recommended
in future, with nx in place, this will be kinda enforced as well (running task from root).
ling1726
approved these changes
Nov 25, 2022
marcosmoura
added a commit
to marcosmoura/fluentui
that referenced
this pull request
Nov 29, 2022
* master: (34 commits) chore(react-tooltip): migrate to new package structure (microsoft#25818) chore(react-field): migrate to new package structure (microsoft#25817) Update vr screenshotdiff lib to accept vr host url as param (microsoft#25772) feat(scripts): enable strict checking for additional sub-folders(packages) v4 (microsoft#25710) fix(tools): bump norhtstar packages v9 deps on dep mismatch resolution (microsoft#25806) feat: remove react-storybook and replace its functionality via standard react-storybook-addon package (microsoft#25786) applying package updates chore(react-spinbutton): migrate to new package structure (microsoft#25813) chore(react-spinner): migrate to new package structure (microsoft#25814) chore(react-provider): migrate to new package structure (microsoft#25809) chore(react-radio, shared-contexts): migrate to new package structure (microsoft#25810) chore(react-theme): migrate to new package structure (microsoft#25812) docs: add Fluent UI Insights EP04 to README (microsoft#25775) chore(react-migration-v8-v9): use same build process/setup as v9/ts-solution packages (microsoft#25679) docs: Improves `Table` documentation (microsoft#25787) feat: improve react-18 tests (microsoft#25758) docs: Add examples for DataGrid (microsoft#25783) chore(react-tree): scaffold TreeItemLayout (microsoft#25781) perf: make ts-minbar test compilation faster and asset preparation simpler (microsoft#25754) chore: creates TreeItem and basic Tree (microsoft#25742) ...
Hotell
added a commit
to Hotell/fluentui
that referenced
this pull request
Feb 9, 2023
* fix(react-18-tests-v9): enable e2e run on ci and normalize all test scenarios * feat(scripts): resolve cypress webpack config based on packageJson.version instead hardcoded folder name * feat(react-18-tests-v8): enable build-less DX for start,test,e2e and normalize all test scenarios * feat(react-18-tests-v9): use react-components suite with type checking enabled
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.
Previous Behavior
tests and app code is more or less copy paste that can get out of sync quickly. Also this approach goes against good testing practices.
v8:
lage build --to- (undocumented, not good DX)v9:
e2edefined thus tests are not run on ciNew Behavior
tests follow best practices and use application code within
v8:
v9:
e2eruns on ciRelated Issue(s)