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

expand doesn't work with mock.patch.multiple #138

Closed
ArthurGW opened this issue May 11, 2022 · 6 comments
Closed

expand doesn't work with mock.patch.multiple #138

ArthurGW opened this issue May 11, 2022 · 6 comments

Comments

@ArthurGW
Copy link

ArthurGW commented May 11, 2022

When using mock.patch.multiple as a decorator (class or test method), the mocks it creates are passed in as keyword arguments to the test function.

Unfortunately, this is not compatible with expand, as the standalone functions that it generates (parameterized.parameterized.param_as_standalone_func.standalone_func) only take unnamed positional arguments.

The simplest fix seems to be to just add unnamed keyword arguments as well:

@wraps(func)
def standalone_func(*a, **k):
    return func(*(a + p.args), **p.kwargs, **k)
@wolever
Copy link
Owner

wolever commented Mar 2, 2023

Hey! I'm not able to reproduce this issue; tests with @mock.patch.expand seem to work: #156

But please feel free to re-open this issue or create a PR if there's something I've missed!

@wolever wolever closed this as completed Mar 2, 2023
@ArthurGW
Copy link
Author

ArthurGW commented Mar 2, 2023

Hey, @wolever thanks for looking into it!

There's a couple of ways this can still break actually:

  • as expected, if you place expand below mock.patch.multiple in the decorator stack (I think this one can be ignored)
  • more relevantly, if you use mock.patch.multiple as a class decorator, which shows the same behaviour but I think is supposed to be supported?

For example, if I tweak the test class as below, then I get the error: TypeError: TestParameterizedExpandWithMockPatchMultiple.test_mock_patch_multiple_expand() got an unexpected keyword argument 'umask'.

@mock.patch.multiple("os", umask=mock.DEFAULT, getpid=mock.DEFAULT)
class TestParameterizedExpandWithMockPatchMultiple(TestCase):
    expect([
        "test_mock_patch_multiple_expand(42, 'umask', 'getpid')",
    ])

    @parameterized.expand([(42, )])
    def test_mock_patch_multiple_expand(self, umask, getpid, param):
        missing_tests.remove(
            "test_mock_patch_multiple_expand(%r, %r, %r)" %(
                param, umask._mock_name, getpid._mock_name
            )
        )

Whereas the equivalent non-multiple patches as below work correctly.

@mock.patch("os.getpid")
@mock.patch("os.umask")
class TestParameterizedExpandWithMockPatchMultiple(TestCase):
    expect([
        "test_mock_patch_multiple_expand(42, 'umask', 'getpid')",
    ])

    @parameterized.expand([(42, )])
    def test_mock_patch_multiple_expand(self, umask, getpid, param):
        missing_tests.remove(
            "test_mock_patch_multiple_expand(%r, %r, %r)" %(
                param, umask._mock_name, getpid._mock_name
            )
        )

@wolever wolever reopened this Mar 2, 2023
@wolever
Copy link
Owner

wolever commented Mar 2, 2023

Ah thank you! I'll drop in a fix this weekend, or feel free to open a PR with test!

@ArthurGW
Copy link
Author

ArthurGW commented Mar 3, 2023

No worries!

I could probably drop a PR this weekend if you want. Sadly unable to do it during work time

@wolever
Copy link
Owner

wolever commented Mar 27, 2023

Wow, that's a weird one! It seems like the class-level @mock.patch.multiple works if the method also has a @mock.patch.multiple applied! But as of #161 both should work now.

This fix will be part of the 0.9 release, which should be out by the time you read this.

Thanks for taking the time to submit a report!

@wolever wolever closed this as completed Mar 27, 2023
wolever added a commit that referenced this issue Mar 27, 2023
Fix #138: support for class-level mock.patch.multiple
@ArthurGW
Copy link
Author

Hah! Thanks for the fix :)

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

No branches or pull requests

2 participants