typeguard icon indicating copy to clipboard operation
typeguard copied to clipboard

Support for type checking any mock with spec

Open fornellas opened this issue 4 years ago • 10 comments

While working on TestSlide (a Python test framework from Facebook), @david-caro, @fabriziocucci and myself found an issue with typeguard integration.

In summary, typeguard checks for types recursively (eg: for Union, Tuple etc), which is a good thing. However, we got failures when one of these recursive checks hits a mock object.

TestSlide type validation functionality plays nicely with mocks, and can detect and extract their templates for use in type validation. But, if a mock has no template, no type validation happens (just like typeguard).

TestSlide's interface for this is public and extensible, so we can add supports for any mock object, including TestSlide's own StrictMock.

I cut this clowny PR for TestSlide, to add the extra logic to deal with mocks, by patching typeguard.check_type and wrapping it, so we can move on temporarily.

@agronholm , I'm willing to cut a PR for typeguard to implement this public interface for dealing with mocks, what do you think? Once available at typeguard, we can drop it from TestSlide.

fornellas avatar Apr 20 '20 15:04 fornellas

I've recently merged a PR for bypassing type checks for unittest.[Magic]Mock objects. I just haven't cut a release since then. Would it suffice for you if I were to make a new release now?

agronholm avatar Apr 21 '20 06:04 agronholm

That's in a good direction, but still does not cover these 2 cases:

  • Using the mock spec for type checking when available.
  • Have a public API to extend support for any third party mocks.

What do you think?

fornellas avatar Apr 22 '20 09:04 fornellas

For the first item, I would be inclined to accept a PR for that. For the second item, I would consider a proposal for such an API.

agronholm avatar Apr 22 '20 09:04 agronholm

Cool! I'll do a PR for the first item as soon as I have time.

For the second, here's the proposal:

https://github.com/facebookincubator/TestSlide/blob/0b36ffdb40d471d9e945864647265f0033f2d521/testslide/lib.py#L18-L42

MOCK_TEMPLATE_EXTRACTORS would be public, and people can extend it, to add their custom mock template extractor functions like this:

https://github.com/facebookincubator/TestSlide/blob/0b36ffdb40d471d9e945864647265f0033f2d521/testslide/strict_mock.py#L734-L741

fornellas avatar Apr 22 '20 16:04 fornellas

This feature is trickier to implement than I thought at first because it requires a class compatibility check instead of the instance/value checks that typeguard currently uses.

agronholm avatar Jun 06 '20 11:06 agronholm

+1 Would like to see this happen. Mocks of third party package classes are being flagged by Typeguard as being incorrect types because of the way I'm setting the Mocks' return_value. In order to get around this, I'd have to create new instances of those third party packages, which defeats the purpose of Mocks and unit testing.

edit-- Typeguard docs say they ignore Mocks, but that doesn't seem to be the case: https://typeguard.readthedocs.io/en/latest/userguide.html?highlight=mock#support-for-mock-objects

edit^2 -- Actually it looks like I've unfortunately hit some kind of strange edge case scenario (explained below) where a mock is not used by Typeguard.

wanderrful avatar Aug 06 '20 14:08 wanderrful

Can you create a minimal script to demonstrate the problem?

agronholm avatar Aug 06 '20 15:08 agronholm

Unfortunately for me it seems to be a really weird edge case involving type expectations, because my attempts to reproduce it in a minimal way aren't working. I imagine the underlying reason could be because of the type expectations being more permissive when it's imported in a certain way or when the underlying classes are less complex.

class ThirdPartyPackageClass:
    """I should be able to mock this for unit tests without Typeguard complaining."""
    _internal_hash_map = dict()

    def __getitem__(self, item):
        return self._internal_hash_map[item]


class ClassIWantToTest:
    """This is the thing I wrote."""
    _foo: List[str] = list()

    def __init__(self, one: ThirdPartyPackageClass, key: str):
        if key not in one:
            raise Exception("Oops")
        self._foo = one[key]


def test_blah() -> None:
    """Typeguard is okay with this minimal example."""
    mock_data = {"hello": ["foo", "bar", "baz"]}
    my_mock = Mock(spec=ThirdPartyPackageClass, return_value=mock_data)

    a = ClassIWantToTest(my_mock(), "hello")  # noqa: F841
    assert a._foo == mock_data["hello"]

The idea is that we want to mock the minimal behavior for a unit test without needing to instantiate the entire class itself, which could potentially take up a lot of memory for integration/acceptance tests.

The underlying third party package in question here is Workbook from the openpyxl project: https://openpyxl.readthedocs.io/en/stable/api/openpyxl.workbook.workbook.html?highlight=workbook#openpyxl.workbook.workbook.Workbook

I imagine there must be something strange about its implementation that doesn't allow me to mock it naively for Typeguard (though Pytest and Mypy have no issue with it), and it somehow falls through the cracks with Typeguard.

In reality, what we get from Typeguard is something like this:

TypeError: type of argument "one" must be openpyxl.workbook.workbook.Workbook; got dict instead

What I'd expect from Typeguard is to recognize that I'm passing in a Mock object to the constructor of ClassIWantToTest and then just ignore it. I'm obviously just misunderstanding the documentation and intended behavior.

wanderrful avatar Aug 06 '20 15:08 wanderrful

The error you pasted here indicates that mocks are not used at all in the function call. Please paste the relevant target function header and the exact type of the object being passed.

agronholm avatar Aug 07 '20 08:08 agronholm

I only have an xfailing test for this in the code base, but I don't think it's going to start working any time soon.

agronholm avatar Dec 05 '21 21:12 agronholm