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

test: several fixes for the design-system test framework #16502

Merged
merged 7 commits into from
May 18, 2021

Conversation

elevatebart
Copy link
Contributor

@elevatebart elevatebart commented May 13, 2021

  • Update the @storyboook/testing-react from canary to 0.12.0
  • Load the real FileTree/index.ts instead of the one that is in the dist folder in the test
  • Add 2 stories based tests

@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 13, 2021

Thanks for taking the time to open a PR!

@elevatebart elevatebart requested a review from agg23 May 13, 2021 15:30
agg23
agg23 previously approved these changes May 13, 2021
@@ -66,7 +67,7 @@ const tree: TreeParent = {
],
}

export const VirtualizedTree = createStory(() => {
export const VirtualizedTree: Story = createStory(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This cast should be unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually is, because createStory returns Story<unknown> I am wondering what would type the Story here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated utils.ts a bit so that they become unnecessary.

@cypress
Copy link

cypress bot commented May 14, 2021



Test summary

4074 0 50 2Flakiness 0


Run details

Project cypress
Status Passed
Commit 21428b5
Started May 18, 2021 7:47 PM
Ended May 18, 2021 7:57 PM
Duration 10:08 💡
OS Linux Debian - 10.8
Browser Chrome 90

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

👀

@@ -8,7 +8,7 @@ export const createStorybookConfig = (config: Meta): Meta => config
/**
* Compact way of declaring a new story
*/
export const createStory = <T>(template: Story<T>, args?: Partial<T>) => {
export const createStory = <T = any>(template: Story<T>, args?: Partial<T>): Story<T> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the default of any necessary? Can't you just omit that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not find out how to.

If I do omit that, TypeScript makes it an unknown which will in turn give me unknown again when inferring the type for the Cypress mount() function... and will error

Copy link
Contributor

Choose a reason for hiding this comment

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

No matter what I try, I can't seem to get this working :(

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I figured out why it's being weird. I disabled TypeScript on the specs in the tsconfig, which means TS wasn't trying very hard to properly show the types. Removing the exclude clause has everything work correctly without these type changes (although you might want to change the default value of T to {}:

createStory = <T = {}>(...) => {...}

@lmiller1990 lmiller1990 self-requested a review May 18, 2021 00:28
lmiller1990
lmiller1990 previously approved these changes May 18, 2021
@elevatebart elevatebart merged commit deab1ea into develop May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants