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

remove the cli tests #13318

Closed
wants to merge 1 commit into from
Closed

remove the cli tests #13318

wants to merge 1 commit into from

Conversation

ndelangen
Copy link
Member

remove the cli tests

why?
These test were setup before we had the examples-v2 tests

The removed test tested whether the storybook CLI & resulting bootstrapped storybook works.
However these fixtures do not get updated, and so over time have come to reflect what user experience less and less.

Now that we have tests that run against the latest (and other versions) of things like CRA & angular-cli I think it's safe to remove these tests.

@ndelangen ndelangen added the maintenance User-facing maintenance tasks label Nov 27, 2020
@ndelangen ndelangen requested a review from igor-dv November 27, 2020 15:37
@ndelangen ndelangen self-assigned this Nov 27, 2020
@ndelangen ndelangen changed the title these test serve no purpose anymore remove the cli tests Nov 27, 2020
@jamesgeorge007
Copy link
Member

Isn't it a better idea to not remove the entire test suite, but only the test fixtures? #12936 aims at leveraging Jest for the purpose; we can keep the test case for default behavior.

@ndelangen
Copy link
Member Author

The fixtures ARE the only tests AFAIK?

@jamesgeorge007
Copy link
Member

The fixtures ARE the only tests AFAIK?

#12936 introduces a test case that makes sure that the closest match shows up on supplying an unknown command.

@ndelangen
Copy link
Member Author

@jamesgeorge007 ah that seems really valuable, I like that. If this is merged, can you adjust your PR to add any requirements back for the above?

@jamesgeorge007
Copy link
Member

@jamesgeorge007 ah that seems really valuable, I like that. If this is merged, can you adjust your PR to add any requirements back for the above?

I think a better idea would be to address this as part of #12936 if you're okay with it.

@jamesgeorge007
Copy link
Member

ca7649b

@ndelangen ndelangen closed this Nov 30, 2020
@ndelangen ndelangen deleted the remove-cli-tests branch November 30, 2020 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants