Skip to content

Inheriting from instantiated generic class causes mocking to fail #250

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

Closed
jhance opened this issue Jul 20, 2016 · 6 comments · Fixed by #289 or #295
Closed

Inheriting from instantiated generic class causes mocking to fail #250

jhance opened this issue Jul 20, 2016 · 6 comments · Fixed by #289 or #295
Milestone

Comments

@jhance
Copy link

jhance commented Jul 20, 2016

My reproduction (python 2)

import mock

from typing import Generic, TypeVar

T = TypeVar('T')

class Bar(Generic[T]):
    def __init__(self):
        pass

    def do_bar(self):
        print("bar")

class Foo(Bar[int]):
    def do_foo(self):
        self.do_bar()

@mock.patch.object(Bar, 'do_bar')
def mocked(mock_do_bar):
    mock_do_bar.return_value = None
    foo = Foo()
    foo.do_foo()

mocked()

will unexpectedly print bar.

The inheritance here is crucial.

@gvanrossum
Copy link
Member

(PS try to trim trailing whitespace when copy/pasting text into github. It causes spurious horizontal scroll bars to appear and weird line breaks. I cleaned it up for you this time.)

@jhance
Copy link
Author

jhance commented Jul 20, 2016

Oh, I did not realize I somehow accrued trailing whitespace. Must have been because I pastebin'd it to Jukka then copied from there. Sorry about that :(

@gvanrossum gvanrossum added the bug label Jul 20, 2016
@gvanrossum
Copy link
Member

Anyway, I do think this is a bug, but I don't know how to fix it. Looking at the MRO for Foo, we have:

[__main__.Foo,
 __main__.Bar<~T>[int],
 __main__.Bar<~T>,
 typing.Generic<~T>,
 typing.Generic,
 <type 'object'>]

I think the double occurrence of Bar may be the problem -- you're patching the original, but the MRO search finds the unpatched copy in Bar[int].

I think here's a workaround:

...
BarInt = Bar[int]

class Foo(BarInt):
    def do_foo(self):
        self.do_bar()

@mock.patch.object(BarInt, 'do_bar')
def mocked(mock_do_bar):
...

@jhance
Copy link
Author

jhance commented Jul 20, 2016

I see.

Well, a workaround is good enough for now. Thanks! (It has the somewhat unfortunate property that you have to mock all the instantiations being used - but in my case that is only 2 of them, which isn't so bad).

Edit: Confirmation that the workaround indeed works around.

@gvanrossum
Copy link
Member

Sorry, this wasn't fixed by #289.

@gvanrossum gvanrossum reopened this Oct 11, 2016
gvanrossum pushed a commit that referenced this issue Oct 21, 2016
Fixes #250

The main idea here is optimizing generics for cases where a type information is added to existing code.
Now:
* ``Node[int]`` and ``Node`` have identical ``__bases__`` and identical ``__mro__[1:]`` (except for the first item, since it is the class itself).
* After addition of typing information (i.e. making some classes generic), ``__mro__`` is changed very little, at most one bare ``Generic`` appears in ``__mro__``.
* Consequently, only non-parameterized generics appear in ``__bases__`` and ``__mro__[1:]``.

Interestingly, this could be achieved in few lines of code and no existing test break.

On the positive side of this approach, there is very little chance that existing code (even with sophisticated "magic") will break after addition of typing information.

On the negative side, it will be more difficult for _runtime_ type-checkers to perform decorator-based type checks (e.g. enforce method overriding only by consistent methods). Essentially, now type erasure happens partially at the class creation time (all bases are reduced to origin).

(We have __orig_class__ and __orig_bases__ to help runtime checkers.)
@jhance
Copy link
Author

jhance commented Oct 28, 2016

Thanks :D

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