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

Introduce TestShell for making it easier to introduce new tests #30

Merged
merged 1 commit into from
Jun 22, 2020

Conversation

david-binda
Copy link
Collaborator

While working on the other PRs, I found it unnecessarily verbose to write an integration test.

Feels like it could be made easier and less error-prone (eg: having the class to throw an exception by default in case unknown command is run) in case the project would provide some helpers. Eg.: a TestShell class, which would do the heavy lifting, while letting the developer to only focus on registering the commands.

In the PR I'm also adding a test debug function implementation, which can be shared across the tests, again minimising the amount of code developer has to write (including the PHPCS exclusion comment).

Please, take this only as a proposal, and feel free to say "No" if you don't see the fit here :)

@sirbrillig
Copy link
Owner

This looks great! Yeah, having to mock everything for every integration test was getting pretty verbose. It was fine when there were just a couple of them but now we have a lot more things going on. This is a big improvement.

I merged your other PRs, causing conflicts here, but after a rebase I think this will be a great addition. Thank you.

Copy link
Owner

@sirbrillig sirbrillig left a comment

Choose a reason for hiding this comment

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

Perfect! Thank you!

@sirbrillig sirbrillig merged commit 365b1d9 into sirbrillig:master Jun 22, 2020
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.

2 participants