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

Provide a helper to copy args #37

Open
blueyed opened this issue Mar 21, 2016 · 6 comments
Open

Provide a helper to copy args #37

blueyed opened this issue Mar 21, 2016 · 6 comments

Comments

@blueyed
Copy link
Contributor

blueyed commented Mar 21, 2016

mock will store references in call_args and call_args_list (see https://docs.python.org/3/library/unittest.mock-examples.html#coping-with-mutable-arguments).

I think that pytest-mock could provide a helper based on the example from the doc:

from copy import deepcopy
>>> class CopyingMock(MagicMock):
...     def __call__(self, *args, **kwargs):
...         args = deepcopy(args)
...         kwargs = deepcopy(kwargs)
...         return super(CopyingMock, self).__call__(*args, **kwargs)

The following works (by extending the pytest-mock mocker fixture).

@pytest.fixture
def mocker(mocker):
    from copy import deepcopy
    from mock import MagicMock

    class CopyingMock(MagicMock):
        def __call__(self, *args, **kwargs):
            args = deepcopy(args)
            kwargs = deepcopy(kwargs)
            return super(CopyingMock, self).__call__(*args, **kwargs)

    mocker.CopyingMock = CopyingMock
    return mocker
    patched = mocker.patch('foo.bar', new_callable=mocker.CopyingMock)

Not sure if that's helpful enough and/or if there could be a mocker_copy fixture instead, which would handle new_callable not only for patch().

@blueyed
Copy link
Contributor Author

blueyed commented Mar 21, 2016

This is how a mocker_copy fixture could look like:

@pytest.fixture
def mocker_copy(mocker):
    from copy import deepcopy
    from functools import partial
    from mock import MagicMock

    class CopyingMock(MagicMock):
        def __call__(self, *args, **kwargs):
            args = deepcopy(args)
            kwargs = deepcopy(kwargs)
            return super(CopyingMock, self).__call__(*args, **kwargs)

    mocker.patch._start_patch = partial(mocker.patch._start_patch,
                                        new_callable=CopyingMock)
    return mocker

@nicoddemus
Copy link
Member

Hi @blueyed, thanks for the suggestion. I think adding this functionality to pytest-mock is a good idea.

I've never needed this functionality myself so I can't judge which approach would feel best from the user point of view, but if I have to choose I would rather have the first approach as it doesn't introduce a new fixture to a feature which I believe is not used that much. What do you think?

@blueyed
Copy link
Contributor Author

blueyed commented Mar 22, 2016

@nicoddemus
Thanks for considering it.
I've went with the 2nd approach, since it's much easier to use and provides good separation.

You can easily run into this issue when checking calls that happen in a loop and use e.g. a dict as arg, which gets (re-)assigned in the loop.

Please note that the new_callable=CopyingMock can still be overridden, and new_callable=MagicMock would cause the default behavior again.

We could also consider to have this for the current mocker fixture by default, but that would break is checks etc and is probably therefore not a good idea.

@nicoddemus
Copy link
Member

We could also consider to have this for the current mocker fixture by default, but that would break is checks etc and is probably therefore not a good idea.

I agree.

Are you working on a PR?

@blueyed
Copy link
Contributor Author

blueyed commented Mar 22, 2016

@nicoddemus
Nope.

Feel free to take my patch from above.

@nicoddemus
Copy link
Member

OK, thanks for the suggestion and patch. 😄

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