You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#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.
The text was updated successfully, but these errors were encountered:
#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()
andexecuteCommand()
into a dedicatedLocalProcess
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.
The text was updated successfully, but these errors were encountered: