Skip to content

feat(cli): improve tests for create command#959

Merged
dOrgJelli merged 2 commits intoprealpha-devfrom
nk/create-cmd-test
Jun 27, 2022
Merged

feat(cli): improve tests for create command#959
dOrgJelli merged 2 commits intoprealpha-devfrom
nk/create-cmd-test

Conversation

@Niraj-Kamdar
Copy link
Copy Markdown
Contributor

@Niraj-Kamdar Niraj-Kamdar commented Jun 24, 2022

Copy link
Copy Markdown
Contributor

@cbrzn cbrzn left a comment

Choose a reason for hiding this comment

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

Hey Niraj thanks for start a tackle on this 🙏

I was thinking that this is not the approach we want to take if we want to close #907 - I think instead we should create a new file in __tests__/unit/ folder named create.spec.ts and basically try to run the functions from there and check if the behavior is the expected.

For example, I thought that we can follow these approaches:
tj/commander.js#438 (comment)
tj/commander.js#1565 (comment)
and execute every method of instance?

Another approach we can take is to (talking from the context of create tests) execute the run function and see if it indeed creates a folder for example

Note that the objective of these approaches is to be able to take up our code coverage, as you mentioned before :-D

@Niraj-Kamdar
Copy link
Copy Markdown
Contributor Author

@cbrzn I'd still keep e2e tests, we just need to add unit tests.

@cbrzn
Copy link
Copy Markdown
Contributor

cbrzn commented Jun 27, 2022

@cbrzn I'd still keep e2e tests, we just need to add unit tests.

That's correct! I would remove the closes ... from the description and just make this an improvement of the current integration tests. As we talked before, let's tackle the unit tests post alpha

Copy link
Copy Markdown
Contributor

@dOrgJelli dOrgJelli left a comment

Choose a reason for hiding this comment

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

This is really nice @Niraj-Kamdar 👏

@dOrgJelli
Copy link
Copy Markdown
Contributor

Also IMO @cbrzn @Niraj-Kamdar I think this fully tests the create functionality, so I'm not sure we need unit tests if the e2e tests can exercise all functionality.

@dOrgJelli dOrgJelli merged commit af44279 into prealpha-dev Jun 27, 2022
@dOrgJelli dOrgJelli deleted the nk/create-cmd-test branch April 10, 2023 16:53
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