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(create-app): add integration tests #3204

Merged
merged 1 commit into from
May 10, 2021

Conversation

jamesgeorge007
Copy link
Contributor

Description

This PR aims at adding an integration test suite for the create-app CLI utility.

Additional context

No


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other (Test update)

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@Shinigami92 Shinigami92 added p1-chore Doesn't change code behavior (priority) test labels Apr 29, 2021
@Shinigami92
Copy link
Member

Nice 🙌

@antfu
Copy link
Member

antfu commented May 10, 2021

Thanks for your contributions! Can you describe a bit about the context why you want to add these tests? Having tests is good thing for sure, but I am a bit afraid of this could foot-gun contributors for future changes, which also require them to change the corresponding tests. As the logic here is fair simply and more UI oriented, I am a bit now sure if the value for testing this is worth-wise.

@Shinigami92
Copy link
Member

I think it's a good thing also to have tests for the CLI, cause if someone ones to change it somehow, they get notified via tests if the CLI is still working as expected or if there is e.g. a breaking change.

@jamesgeorge007
Copy link
Contributor Author

jamesgeorge007 commented May 10, 2021

@antfu, thanks for raising your concerns. I do agree with @Shinigami92. For a CLI utility aimed at project scaffolding, there are a couple of things that are expected not to change. For instance, generating a project in the current directory and the associated validations. Also, there are project-specific implementations like the --template option. This PR aims at adding a test suite to ensure the minimum expectations. Adding a new template, or tweaking the CLI UI, doesn't require getting one's hands on the test suite.

@patak-dev patak-dev merged commit b93bf9c into vitejs:main May 10, 2021
@jamesgeorge007 jamesgeorge007 deleted the integration-tests branch May 10, 2021 09:00
fi3ework pushed a commit to fi3ework/vite that referenced this pull request May 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1-chore Doesn't change code behavior (priority) test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants