I think we are using too many mocks for subprocess in our unittests. If we just mock every call to subprocess, the tests become less clear, and they don’t really cover everything that might be happening.
This is challenging, because things are order dependent. So I’ve been experimenting with mocks and fakes.
The idea is that we will mock only the specific command that we are testing, letting everything else continue calling the real thing. We replace the command with a fake implementation that will record some values in memory when required, and return the same things that the real would return. In addition, we keep a mock where we can assert the order of the calls.
Before going any further, I would like to put this for discussion. This approach also has drawbacks, but I would like to implement it with all our plugins and with LXD/LXC.
I saw the PR before this post, so I’m not sure where you were going to discuss it, but I’ll copy my reply here anyway:
You seem to be doing two things here in terms of the test API:
call(['pip', 'install', 'foo'] -> fake_pip.install['installed']
How about a proper API here? Like fake_pip.installed_packages
----disable-pip-version-check -> fake_pip.install['disable_pip_version_check']
How about fake_pip.args? Since these are literally the CLI args used that you want to check here and it’ll be clearer from the test what’s a verbatim argument and what’s a result value.
For comparison, with FakeLXD values like .name, .status and mocked JSON reflect the mocked commands that were run in the test, like fake_pip.install does. But tests still assert mocked calls as a “safety guard”, for example if an apt-get call is accidentally missing, you will see it - I’m not sure what the equivalent would be in FakeLXD terms.
Would you mind focussing your proposal on designing the test API for FakePip and FakeLXD with a couple of examples, rather than the implementation?
