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

Can't mock a @property without calling it? #339

Open
shish opened this issue Aug 15, 2022 · 5 comments
Open

Can't mock a @property without calling it? #339

shish opened this issue Aug 15, 2022 · 5 comments

Comments

@shish
Copy link
Contributor

shish commented Aug 15, 2022

I'm using TestSlide version: 2.7.0

Given:

import testslide

class SampleClass:
    @property
    def prop(self) -> str:
        raise RuntimeError("must not be called")

    def meth(self) -> str:
        raise RuntimeError("must not be called")


class SampleTest(testslide.TestCase):
    def test_prop_attr(self) -> None:
        host = SampleClass()
        self.patch_attribute(host, "prop", "patched")
        self.assertEqual(host.prop, "patched")

    def test_prop_callable(self) -> None:
        host = SampleClass()
        self.mock_callable(host, "prop").to_return_value("patched")
        self.assertEqual(host.prop, "patched")

    def test_meth(self) -> None:
        host = SampleClass()
        self.mock_callable(host, "meth").to_return_value("patched")
        self.assertEqual(host.meth(), "patched")

When I run:
testslide patch_test.py

I expected this to happen:
prop() is not called

But, instead this happened:
prop() is called, both when using patch_attribute and mock_callable.

patch_test.SampleTest
  test_meth
  test_prop_attr: RuntimeError: must not be called
  test_prop_callable: RuntimeError: must not be called

Failures:

  1) patch_test.SampleTest: test_prop_attr
    1) RuntimeError: must not be called
      File "patch_test.py", line 15, in test_prop_attr
        self.patch_attribute(host, "prop", "patched")
      File "patch_test.py", line 6, in prop
        raise RuntimeError("must not be called")

  2) patch_test.SampleTest: test_prop_callable
    1) RuntimeError: must not be called
      File "patch_test.py", line 20, in test_prop_callable
        self.mock_callable(host, "prop").to_return_value("patched")
      File "patch_test.py", line 6, in prop
        raise RuntimeError("must not be called")
@shish
Copy link
Contributor Author

shish commented Aug 15, 2022

For context - this was causing issues with a @property that throws an exception in the unit-test environment, so somebody wants to mock it out. The current theory is that testslide is calling the property in order to find the original value so that it can un-patch it later, however everything explodes if the property raises an exception and there's no obvious workaround.

@fornellas
Copy link
Contributor

fornellas commented Aug 15, 2022

The current theory is that testslide is calling the property in order to find the original value so that it can un-patch it later, however everything explodes if the property raises an exception and there's no obvious workaround.

You'd be correct: this is how patching works. If the property is broken, then... things should break anyway :-/ There's no workaround: the property should be fixed.

self.mock_callable(host, "prop").to_return_value("patched")

This on the other hand... I'd expect to fail: a property is not... callable. It should have failed just as if one was trying to do mock_callable against a string attribute of an object. This is a bug (though not the one reported here):

import testslide

class SampleClass:
    def __init__(self):
        self.attr = "str"

class SampleTest(testslide.TestCase):
    def test_prop_callable(self) -> None:
        host = SampleClass()
        self.mock_callable(host, "attr").to_return_value("patched")
        self.assertEqual(host.attr, "patched")

yields:

blah_test.SampleTest
  test_prop_callable: ValueError: mock_callable() can only be used with callable attributes and 'str' is not.

@printercu
Copy link

If the property is broken, then... things should break anyway :-/ There's no workaround: the property should be fixed.

Property may call external service that is unavailable during tests and it will raise an error. This is what mocking should prevent.

@fornellas
Copy link
Contributor

fornellas commented Aug 18, 2022

Hum... I see your point, that's fair.

I think the way to address this is to add (yet) more complexity on top:

  • When patching, check whether the attribute is a property.
  • If it is, then save the property itself (to unpatch), and not the property value.

There are then 2 bugs to fix:

  • Make the test below pass.
  • Have mock_callable against a property / attribute fail.
import testslide

class SampleClass:
    @property
    def prop(self) -> str:
        raise RuntimeError("must not be called")

class SampleTest(testslide.TestCase):
    def test_prop_attr(self) -> None:
        host = SampleClass()
        self.patch_attribute(host, "prop", "patched")
        self.assertEqual(host.prop, "patched")

@macisamuele
Copy link
Contributor

@fornellas I can try working on this within the next 2 weeks.
Internally I've already prepared a prototype of the possible fix and I need to reserve enough bandwidth to clean-it-up and fix tests as needed

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

4 participants