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

Create LocalProcess helper #714

Open
danepowell opened this issue Nov 3, 2021 · 0 comments
Open

Create LocalProcess helper #714

danepowell opened this issue Nov 3, 2021 · 0 comments

Comments

@danepowell
Copy link
Contributor

danepowell commented Nov 3, 2021

#713 resulted in a drop in code coverage because we added new methods to LocalMachineHelper that had to be mocked and therefore aren't tested. The problem is that LocalMachineHelper as a whole was already being mocked and there's no way to call just some of the original methods with Prophecy (phpspec/prophecy#332 (comment)).

Per the Prophecy maintainer, this is a sign that LocalMachineHelper violates SRP and I'm inclined to agree.

I propose refactoring LocalMachineHelper to move execute() and executeCommand() into a dedicated LocalProcess class. This could be mocked on its own and neatly encapsulates and isolates interaction with the local shell, which is 90% of what's being mocked today.

We could even extend this class later on to create helpers for specific oft-used processes like Git.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants