RFC: Internal stories for E2E testing by ling1726 · Pull Request #19333 · microsoft/fluentui

ling1726 · 2021-08-10T10:12:32Z

Description of changes

Adds relevant proposal from #18895 for hiding certain stories only
required for internal e2e testing to our storybook.

Focus areas to test

(optional)

Adds relevant proposal from microsoft#18895 for hiding certain stories only required for internal e2e testing to our storybook.

ling1726 · 2021-08-10T10:15:20Z

@Hotell @PeterDraex Would appreciate a review to finally open up the door to higher quality testing 🙏

codesandbox-ci · 2021-08-10T10:21:47Z

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 3225373:

Sandbox	Source
Fluent UI React Starter	Configuration

size-auditor · 2021-08-10T10:24:12Z

Asset size changes

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

Baseline commit: 1706b9cd5b28d63b015f0a296059456465a1edd6 (build)

fabricteam · 2021-08-10T10:27:08Z

📊 Bundle size report

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-accordion` Accordion (including children components)	`75.666 kB` `22.328 kB`
`react-avatar` Avatar	`56.558 kB` `15.66 kB`
`react-badge` Badge	`24.343 kB` `7.165 kB`
`react-badge` CounterBadge	`27.156 kB` `7.851 kB`
`react-badge` PresenseBadge	`237 B` `177 B`
`react-button` Button	`24.934 kB` `8.001 kB`
`react-button` CompoundButton	`30.226 kB` `8.878 kB`
`react-button` MenuButton	`26.521 kB` `8.509 kB`
`react-button` ToggleButton	`34.531 kB` `8.637 kB`
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`177.039 kB` `50.136 kB`
`react-components` react-components: FluentProvider & webLightTheme	`36.288 kB` `11.615 kB`
`react-divider` Divider	`15.889 kB` `5.747 kB`
`react-image` Image	`10.642 kB` `4.264 kB`
`react-label` Label	`9.397 kB` `3.839 kB`
`react-link` Link	`14.715 kB` `6.012 kB`
`react-make-styles` makeStaticStyles (runtime)	`7.59 kB` `3.321 kB`
`react-make-styles` makeStyles + mergeClasses (runtime)	`22.135 kB` `8.356 kB`
`react-make-styles` makeStyles + mergeClasses (build time)	`2.557 kB` `1.202 kB`
`react-menu` Menu (including children components)	`114.545 kB` `34.515 kB`
`react-menu` Menu (including selectable components)	`117.227 kB` `35.059 kB`
`react-popover` Popover	`124.172 kB` `36.121 kB`
`react-portal` Portal	`7.78 kB` `2.672 kB`
`react-positioning` usePopper	`23.154 kB` `7.942 kB`
`react-provider` FluentProvider	`16.286 kB` `5.991 kB`
`react-text` Text - Default	`11.798 kB` `4.452 kB`
`react-text` Text - Wrappers	`15.414 kB` `4.734 kB`
`react-theme` Teams: all themes	`32.941 kB` `6.674 kB`
`react-theme` Teams: Light theme	`20.247 kB` `5.662 kB`
`react-tooltip` Tooltip	`45.278 kB` `15.451 kB`
`react-utilities` SSRProvider	`213 B` `170 B`

_{🤖 This report was generated against 1706b9cd5b28d63b015f0a296059456465a1edd6}

fabricteam · 2021-08-10T10:38:40Z

Perf Analysis (`@fluentui/react`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	907	853	5000
BaseButton	mount	880	894	5000
Breadcrumb	mount	2558	2598	1000
ButtonNext	mount	430	419	5000
Checkbox	mount	1535	1495	5000
CheckboxBase	mount	1262	1284	5000
ChoiceGroup	mount	4723	4708	5000
ComboBox	mount	969	967	1000
CommandBar	mount	10069	10065	1000
ContextualMenu	mount	6093	6059	1000
DefaultButton	mount	1148	1113	5000
DetailsRow	mount	3703	3727	5000
DetailsRowFast	mount	3682	3672	5000
DetailsRowNoStyles	mount	3434	3500	5000
Dialog	mount	2141	2112	1000
DocumentCardTitle	mount	144	142	1000
Dropdown	mount	3194	3204	5000
FluentProviderNext	mount	7546	7365	5000
FocusTrapZone	mount	1840	1796	5000
FocusZone	mount	1818	1807	5000
IconButton	mount	1708	1718	5000
Label	mount	343	344	5000
Layer	mount	1788	1734	5000
Link	mount	454	481	5000
MakeStyles	mount	1789	1814	50000
MenuButton	mount	1464	1482	5000
MessageBar	mount	1999	2036	5000
Nav	mount	3202	3197	1000
OverflowSet	mount	1093	1047	5000
Panel	mount	2072	2059	1000
Persona	mount	837	839	1000
Pivot	mount	1382	1390	1000
PrimaryButton	mount	1258	1276	5000
Rating	mount	7610	7562	5000
SearchBox	mount	1284	1278	5000
Shimmer	mount	2482	2477	5000
Slider	mount	1950	1944	5000
SpinButton	mount	4893	4899	5000
Spinner	mount	417	409	5000
SplitButton	mount	3103	3104	5000
Stack	mount	490	497	5000
StackWithIntrinsicChildren	mount	1532	1549	5000
StackWithTextChildren	mount	4507	4470	5000
SwatchColorPicker	mount	10123	10331	5000
Tabs	mount	1400	1377	1000
TagPicker	mount	2547	2597	5000
TeachingBubble	mount	11765	11719	5000
Text	mount	409	417	5000
TextField	mount	1350	1338	5000
ThemeProvider	mount	1176	1163	5000
ThemeProvider	virtual-rerender	599	612	5000
Toggle	mount	798	806	5000
buttonNative	mount	116	112	5000

Perf Analysis (`@fluentui/react-northstar`)

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
AccordionMinimalPerf.default	151	134	1.13:1
ButtonMinimalPerf.default	172	157	1.1:1
ChatDuplicateMessagesPerf.default	304	276	1.1:1
GridMinimalPerf.default	339	318	1.07:1
DropdownManyItemsPerf.default	688	648	1.06:1
SkeletonMinimalPerf.default	361	339	1.06:1
ReactionMinimalPerf.default	390	371	1.05:1
AnimationMinimalPerf.default	411	396	1.04:1
AttachmentMinimalPerf.default	159	153	1.04:1
LabelMinimalPerf.default	392	378	1.04:1
IconMinimalPerf.default	604	580	1.04:1
AlertMinimalPerf.default	262	254	1.03:1
BoxMinimalPerf.default	352	342	1.03:1
CarouselMinimalPerf.default	460	446	1.03:1
FlexMinimalPerf.default	285	278	1.03:1
MenuButtonMinimalPerf.default	1679	1635	1.03:1
DropdownMinimalPerf.default	3102	3030	1.02:1
HeaderMinimalPerf.default	352	344	1.02:1
HeaderSlotsPerf.default	758	745	1.02:1
ItemLayoutMinimalPerf.default	1214	1190	1.02:1
LoaderMinimalPerf.default	680	664	1.02:1
RadioGroupMinimalPerf.default	454	446	1.02:1
RefMinimalPerf.default	235	231	1.02:1
SplitButtonMinimalPerf.default	3757	3701	1.02:1
CardMinimalPerf.default	547	540	1.01:1
ChatWithPopoverPerf.default	354	351	1.01:1
DatepickerMinimalPerf.default	5326	5252	1.01:1
DividerMinimalPerf.default	356	353	1.01:1
ImageMinimalPerf.default	371	368	1.01:1
ListMinimalPerf.default	511	508	1.01:1
MenuMinimalPerf.default	826	815	1.01:1
TextAreaMinimalPerf.default	484	479	1.01:1
TreeMinimalPerf.default	800	791	1.01:1
ButtonSlotsPerf.default	557	556	1:1
CheckboxMinimalPerf.default	2692	2686	1:1
FormMinimalPerf.default	391	392	1:1
InputMinimalPerf.default	1268	1266	1:1
ListCommonPerf.default	607	609	1:1
PopupMinimalPerf.default	597	597	1:1
SegmentMinimalPerf.default	339	340	1:1
TextMinimalPerf.default	331	332	1:1
VideoMinimalPerf.default	617	615	1:1
AttachmentSlotsPerf.default	1059	1066	0.99:1
ButtonOverridesMissPerf.default	1694	1714	0.99:1
ChatMinimalPerf.default	634	640	0.99:1
DialogMinimalPerf.default	740	745	0.99:1
EmbedMinimalPerf.default	4054	4093	0.99:1
ListNestedPerf.default	535	542	0.99:1
ListWith60ListItems.default	621	626	0.99:1
RosterPerf.default	1113	1123	0.99:1
PortalMinimalPerf.default	180	181	0.99:1
ProviderMergeThemesPerf.default	1679	1694	0.99:1
ProviderMinimalPerf.default	994	999	0.99:1
SliderMinimalPerf.default	1548	1561	0.99:1
StatusMinimalPerf.default	668	678	0.99:1
TableManyItemsPerf.default	1852	1866	0.99:1
CustomToolbarPrototype.default	3826	3867	0.99:1
ToolbarMinimalPerf.default	940	945	0.99:1
TableMinimalPerf.default	393	400	0.98:1
TreeWith60ListItems.default	169	173	0.98:1
AvatarMinimalPerf.default	186	192	0.97:1
LayoutMinimalPerf.default	353	364	0.97:1
TooltipMinimalPerf.default	988	1020	0.97:1

Hotell · 2021-08-11T14:15:22Z

+We propose to use an extra filename extension and naming convention for internal stories:
+
+```ts
+// MenuTabstopsInternal.stories.tsx


typo? last time we talked I remember we wanted to go for MenuTabstops.internal.stories.tsx ?

-> just checked resolved comments from your convo with @PeterDraex .

In general I agree with the reasoning but I'd still keep the additional internal suffix - pascal cased file names are not very easy to read and one might get into typos/git casing issues rather quickly

The rule would make it also easily extendable for future, lets say if we decided to build on public facing stories, or only internal/e2e facing stories separately for performance reasons etc (with this we will be always shipping lot of JS for internal stories to consumers )

Rule could look like

ComponentFoo.stories.tsx -> export const ComponentFoo = (args) => {}

ComponentFoo.internal.stories.tsx -> export const ComponentFooInternal = () => {}

with this we can very easily filter out based on environment which stories should be build

// @filename ./.storybook/main.js const {env} = require 'environments'; module.exports = { stories: [ '../src/**/*.stories.mdx', '../src/**/*.stories.@(ts|tsx)', env.dev ? '../src/**/*.internal.stories.@(ts|tsx)' : '' ].filter(Boolean), }

the file extension adds one 'extra' thing to enforce, since we need to enforce story naming too, makes more sense if we want to break out stories into their own files that the naming of the file reflects the story name

thoughts ^ ? :D

the file extension adds one 'extra' thing to enforce

IMHO its same restriction as adding Internal suffix for the file name. but go for it -> my reasoning still applies and proposed approach might "block" us in the future.

but lets iterate on it. thx

I also prefer .internal but not blocking on that.

sure I can add the .internal suffix by popular demand :)

Hotell · 2021-08-11T14:35:03Z

+
+Storybook has proposed a feature for this in [storybookjs/storybook#9209](https://github.com/storybookjs/storybook/issues/9209)
+which will configure stories to exist in deeplink URL format, but do not appear in the nav tree or the docs page. As stated in the issue,
+we can workaround before the release of this feature by modifying `manager-head.html` and set `display:none`for all


hmm just a note, AFAIR storybook doesn't support inheritance when manager-head.html is used -> thus if this is being set in root ./storybook and some package would like to use some specific manager overrides this would break the flow

This measure is only to ensure that we don't make it easy to see these stories, for users in the public docsite. therefore this config in react-components should be the only one needed, although I could be missing some long term storybook composition details you might be planning

#19040 finally introduces a concept of production by setting NODE_ENV, so we can achieve the same after that PR by doing custom story loading, which can also be messy.... I don't mind which one, but this proposal has already been blocked multiple times in the last few months and blocks us from testing meaningful edge cases in the browser

sounds good, I mentioned this just a heads up ;) no need to be afraid of getting blocked :D

Hotell · 2021-08-11T16:02:57Z

more than 2 approvals -> ready to merge. thx!

ecraig12345 · 2021-08-11T16:34:05Z

more than 2 approvals -> ready to merge. thx!

It might also be good as a general practice to give people at least 2 business days to review (particularly if nobody from a regional team has approved)--I'd like to review this too but didn't get to look at it yesterday.

ecraig12345 · 2021-08-11T23:01:40Z

+We can simply use a css wildcard query selector:
+
+```css
+[id*='internal'] {


Maybe this is just the engineer instinct to try to break everything 😆 but can this be made any more specific, like [id$='-internal']? (reference)

yes of course, the idea was to just make the entire word internal a keyword, we can also constrain the position

Hotell · 2021-08-12T09:37:13Z

more than 2 approvals -> ready to merge. thx!

It might also be good as a general practice to give people at least 2 business days to review (particularly if nobody from a regional team has approved)--I'd like to review this too but didn't get to look at it yesterday.

that's good point. I wrote that to encourage @ling1726 to progress fast/er based on this concern "but this proposal has already been blocked multiple times in the last few months"

RFC: Internal stories for E2E testing

b714355

Adds relevant proposal from microsoft#18895 for hiding certain stories only required for internal e2e testing to our storybook.

msft-fluent-ui-bot assigned smhigley Aug 10, 2021

update

8aff383

ling1726 marked this pull request as ready for review August 10, 2021 10:14

ling1726 requested review from a team August 10, 2021 10:15

ling1726 commented Aug 10, 2021

View reviewed changes

Comment thread rfcs/convergence/authoring-stories.md Outdated

Update authoring-stories.md

f047b56

PeterDraex reviewed Aug 10, 2021

View reviewed changes

Comment thread rfcs/convergence/authoring-stories.md Outdated

Comment thread rfcs/convergence/authoring-stories.md Outdated

Update authoring-stories.md

f8510d8

PeterDraex approved these changes Aug 10, 2021

View reviewed changes

theerebuss approved these changes Aug 10, 2021

View reviewed changes

Hotell reviewed Aug 11, 2021

View reviewed changes

Hotell added Storybook Type: RFC Request for Feedback labels Aug 11, 2021

Hotell added this to the August Project Cycle Q3 2021 milestone Aug 11, 2021

Hotell approved these changes Aug 11, 2021

View reviewed changes

ecraig12345 approved these changes Aug 11, 2021

View reviewed changes

Update authoring-stories.md

3225373

ling1726 merged commit 46ea002 into microsoft:master Aug 13, 2021

ling1726 mentioned this pull request Aug 13, 2021

feat(storybook): Hide internal stories, add internal stories and E2E tests for Menu #18627

Closed

1 task

Uh oh!

Conversation

ling1726 commented Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Focus areas to test

Uh oh!

ling1726 commented Aug 10, 2021

Uh oh!

Uh oh!

codesandbox-ci Bot commented Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

size-auditor Bot commented Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

fabricteam commented Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

fabricteam commented Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react-northstar)

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hotell Aug 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ling1726 Aug 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hotell commented Aug 11, 2021

Uh oh!

ecraig12345 commented Aug 11, 2021

Uh oh!

ecraig12345 Aug 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hotell commented Aug 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ling1726 commented Aug 10, 2021 •

edited

Loading

codesandbox-ci Bot commented Aug 10, 2021 •

edited

Loading

size-auditor Bot commented Aug 10, 2021 •

edited

Loading

fabricteam commented Aug 10, 2021 •

edited

Loading

fabricteam commented Aug 10, 2021 •

edited

Loading

Perf Analysis (`@fluentui/react-northstar`)

Hotell Aug 11, 2021 •

edited

Loading

ling1726 Aug 11, 2021 •

edited

Loading

ecraig12345 Aug 11, 2021 •

edited

Loading

Hotell commented Aug 12, 2021 •

edited

Loading