wolever / parameterized

Parameterized testing with any Python test framework
Other
833 stars 105 forks source link

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

Closed ArthurGW closed 1 year ago

ArthurGW commented 2 years ago

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 commented 1 year ago

Hey! I'm not able to reproduce this issue; tests with @mock.patch.expand seem to work: https://github.com/wolever/parameterized/pull/156

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

ArthurGW commented 1 year ago

Hey, @wolever thanks for looking into it!

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

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 commented 1 year ago

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

ArthurGW commented 1 year ago

No worries!

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

wolever commented 1 year ago

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 https://github.com/wolever/parameterized/pull/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!

ArthurGW commented 1 year ago

Hah! Thanks for the fix :)