#107 Encapsulate subprocess calls in run() function

Open
opened 3 months ago by floli · 2 comments
floli commented 3 months ago

This is a follow up to the discussion in #97.

I basically did the change above in https://projects.torsion.org/floli/borgmatic/src/branch/run_function but have a hard time mocking the function for the tests.

My main problems at mocking are:

  • To accept both variants of calling a function with an argument given the default value and the argument not given at all, which are essentially the same.

  • Previously, some tests mocked subprocess.check_call and subprocess.check_output. These are now both calls to run(capture_output = False) or run(capture_output = True). So I need to mock two calls to the same function with different argument sets.

Therefore my commit does not include any test changes and thus the tests will fail.

I am happy to make this a WIP pull-request, so at least @witten can contribute.

Best!

This is a follow up to the discussion in #97. I basically did the change above in https://projects.torsion.org/floli/borgmatic/src/branch/run_function but have a hard time mocking the function for the tests. My main problems at mocking are: * To accept both variants of calling a function with an argument given the default value and the argument not given at all, which are essentially the same. * Previously, some tests mocked `subprocess.check_call` and `subprocess.check_output`. These are now both calls to `run(capture_output = False)` or `run(capture_output = True)`. So I need to mock two calls to the same function with different argument sets. Therefore my commit does not include any test changes and thus the tests will fail. I am happy to make this a WIP pull-request, so at least @witten can contribute. Best!
witten commented 3 months ago
Owner

Here are some ideas that may help you out with automated testing these changes:

.with_args() will help you set up separate mocks for each of run(capture_output=False) and run(capture_output=True) branches. For instance:

flexmock(module).should_receive('run').with_args(capture_output=False)
flexmock(module).should_receive('run').with_args(capture_output=True).and_return(...)

I believe that first mock may handle this case:

To accept both variants of calling a function with an argument given the default value and the argument not given at all, which are essentially the same.

If not, you can always add an additional mock as well that expects no arguments, e.g. .with_args(). (To test the default behavior.)

And you’re welcome to make a WIP pull request if you like. I’m happy to contribute.

Here are some ideas that may help you out with automated testing these changes: [`.with_args()`](https://flexmock.readthedocs.io/en/latest/start/?highlight=with_args#mocks) will help you set up separate mocks for each of `run(capture_output=False)` and `run(capture_output=True)` branches. For instance: ```python flexmock(module).should_receive('run').with_args(capture_output=False) flexmock(module).should_receive('run').with_args(capture_output=True).and_return(...) ``` I believe that first mock may handle this case: > To accept both variants of calling a function with an argument given the default value and the argument not given at all, which are essentially the same. If not, you can always add an additional mock as well that expects no arguments, e.g. `.with_args()`. (To test the default behavior.) And you're welcome to make a WIP pull request if you like. I'm happy to contribute.
witten commented 1 month ago
Owner

For anyone else looking at this ticket, here’s the WIP PR: #109

For anyone else looking at this ticket, here's the WIP PR: https://projects.torsion.org/witten/borgmatic/pulls/109
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
Cancel
Save
There is no content yet.