wolever / parameterized

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

Parameterized doens't play nicely with `unittest.mock.patch` decorator #66

Open thejcannon opened 5 years ago

thejcannon commented 5 years ago

Howdy!

I've found that mixing Python's unittest.mock.patch decorator with parameterized.expand leads to unfortunate side-effects.

@parameterized.expand([("x", "y"),])
@patch("foo")
def test_bar(foo, x, y):
   pass

Will lead to UnboundLocalError: local variable 'patching' referenced before assignment (at unittest\mock.py:1181). I'm sure this is due to the nature of how expand generates test cases.

There's a workaround, which is use patch as a context manager in the test body. However, I'd still love to use both as decorators for all that delicious syntactic sugar and saved whitespace 😄.

thejcannon commented 5 years ago

Well this parametrized test which tests patch certainly makes my situation a puzzler.

thejcannon commented 5 years ago

I think the issue I'm running into only occurs if the test raises an exception. So really the bug just makes tracking down the cause of failing tests a bit more challenging.

Edit: And looks like the root cause of the exception raised (in my instance) was the parameter ordering (foo should've been last).

wolever commented 5 years ago

Good to know, thanks! That seems like a common stumbling block, so I've updated the documentation to include an example.

ltsampros commented 5 years ago

I think this is still an issue as I can reproduce it even with the correct ordering of the variable.

@parameterized.expand([("x", "y"),])
@patch("foo")
def test_bar(x, y, foo):
   assertEqual(1,2)

As expected it's only reproducible with py2 using the mocks backport. py3 works fine.

korcek-juraj commented 5 years ago

I have the same issue with v0.7.0.

The setup is following:

@parameterized.expand([("x", "y"),])
@patch("internal.lib.timezone_at", new=mock_timezone_at('Europe/Zurich'))
def test_bar(x, y):
   assertEqual(1,2)

If context manager is used instead, everything works:

@parameterized.expand([("x", "y"),])
def test_bar(x, y):
   with patch("internal.lib.timezone_at", new=mock_timezone_at('Europe/Zurich')):
       assertEqual(1,2)

In other words, UnboundLocalError is thrown instead of the actual exception when patch is used as decorator together with parameterized.

wolever commented 5 years ago

Hrm, I've added a test case for this, but I can't seem to recreate the issue: 85222cfd5f93ba026a74a4726ae98d6f89fff502

I've also tried to reproduce it by raising an explicit AssertionError in the test, and I still see the correct stack trace:

FAIL: parameterized.test.test_mock_patch_standalone_function(42, {})
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/wolever/code/parameterized/.tox/py27-nose/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/Users/wolever/code/parameterized/parameterized/parameterized.py", line 387, in <lambda>
    nose_func = wraps(func)(lambda *args: func(*args[:-1], **args[-1]))
  File "/Users/wolever/code/parameterized/.tox/py27-nose/lib/python2.7/site-packages/mock/mock.py", line 1305, in patched
    return func(*args, **keywargs)
  File "/Users/wolever/code/parameterized/parameterized/test.py", line 207, in test_mock_patch_standalone_function
    raise AssertionError("foo")
AssertionError: foo

Could you try running this test with pip install tox then tox -e py27-nose (or replace nose with whatever testrunner you use) and let me know if you can reproduce it?

I'm wondering if it's an issue with a particular version of mock, or something similar.

korcek-juraj commented 5 years ago

The new test passed with pytest3, however, I managed to recreate the same issue by adding

    @parameterized.expand([(42,)])
    @mock.patch('os.umask', new=lambda x: x)
    def test_mock_patch_standalone_function2(foo):
        raise ValueError()

into TestParameterizedExpandWithNoMockPatchForClass TestCase.

wolever commented 5 years ago

@korcek-juraj hrm… I'm still not able to reproduce the issue. When I use the example you've included, I get the correct stack trace:

============================================================================== FAILURES ==============================================================================
_______________________________________________________________ test_mock_patch_standalone_function[0] _______________________________________________________________

foo = 42

    @parameterized([(42, )])
    @mock.patch("os.umask", new=some_func)
    def test_mock_patch_standalone_function(foo):
>       raise ValueError()
E       ValueError

parameterized/test.py:210: ValueError
============================================================ 1 failed, 86 passed, 1 error in 0.37 seconds ============================================================
ERROR: InvocationError for command '/Users/wolever/code/parameterized/.tox/py27-pytest2/bin/py.test parameterized/test.py' (exited with code 1)
@parameterized([(42, )])
@mock.patch("os.umask", new=lambda x: x)
def test_mock_patch_standalone_function(foo, mock_umask):
    raise ValueError()

To help me debug this further, could you:

  1. Add the test case which is failing
  2. Run it with tox -e py27-nose (or whatever the test runner you're using is)
  3. Run .tox/py27-nose/bin/pip freeze and post the output?

Mine is:

$ .tox/py27-nose/bin/pip freeze
funcsigs==1.0.2
mock==2.0.0
nose==1.3.7
parameterized==0.7.0
pbr==5.1.1
six==1.12.0
korcek-juraj commented 5 years ago

@wolever When I run the test case given by you using tox -e py27-pytest3 I get:

Exception: Warning: '@parameterized' tests won't work inside subclasses of 'TestCase' - use '@parameterized.expand' instead.

When I change @parameterized to @parameterized.expand I get the original error:

UnboundLocalError: local variable 'patching' referenced before assignment

Output of .tox/py27-pytest3/bin/pip freeze is:

atomicwrites==1.3.0
attrs==19.1.0
funcsigs==1.0.2
mock==2.0.0
more-itertools==5.0.0
nose==1.3.7
parameterized==0.7.0
pathlib2==2.3.3
pbr==5.1.3
pluggy==0.9.0
py==1.8.0
pytest==3.10.1
scandir==1.10.0
six==1.12.0

Do you want me to create PR adding the test that is failing?

wolever commented 5 years ago

Yes, a PR would be helpful, thanks!

korcek-juraj commented 5 years ago

Okay, PR #72 created. In Travis you can see that most of the builds fail with UnboundLocalError.

floer32 commented 5 years ago

Slightly tangential but could be relevant if there are doc updates here

Personally, I've had so many problems with brittleness in mock-patching, vs. rock-solid results from pytest's monkeypatch. They're not the same and sometimes mock is really what's needed (for checking called-with and so on). Many times the job is just to get some I/O out of the way and monkeypatch works.

So it may be worth noting that for pytest users, monkeypatch is a choice to consider to avoid the brittleness. https://docs.pytest.org/en/latest/monkeypatch.html

wolever commented 5 years ago

Oof, okay, I see the issue now. This is a good bug - nice find! And thanks for taking the time to submit the PR.

Basically, the bug is happening because the patches are being copied to each expanded function by @functools.wraps(…). This means the patchings are also being copied, which is undesirable (ie, because they will be called twice).

Would you be able to take a look the potential fix I've push'd and see if it works for you?

Running the test suite shows the expected failure.

korcek-juraj commented 5 years ago

I have tested it by running the test suite as well as the tests in my code and I confirm that it indeed works now. Thanks a lot for fixing it!

korcek-juraj commented 5 years ago

@hangtwenty thanks for the insight

dvirtz commented 4 years ago

I still see this error in 0.7.1. Hasn't it been fixed already?

dvirtz commented 4 years ago

Can you please merge #72 and cut out a new release?

justinbricker-eb commented 4 years ago

Is this still unresolved?

atleta commented 4 years ago

It seems so. I've just ran into it with 0.7.1 (Params are in the correct order: mock last)

wolever commented 4 years ago

Unfortunately this is still unresolved. The PR in question - #72 - is just a demonstration of the bug, not an actual fix.

StefanoChiodino commented 4 years ago

My workaround was to use patch as a context manager:

with patch('my_calendar.requests') as mock_requests:
thejcannon commented 4 years ago

I should mention I work around it by using pytest-mock which has other nicities.

adamchainz commented 4 years ago

This was a bug in unittest.mock, addressed in https://bugs.python.org/issue40126 . Although the linked issue says it was only fixed for Python 3.7+, I have found the test from #72 passes on Python 3.6.10 as well. It still fails on Python 2.7.17 , so I guess the mock backport doesn't have the fix yet.