Incorrect teardown after mocking return value of classmethod implemented in base class

[adam-sikora]

Following tests should pass

from flexmock import flexmock


class A:

    @classmethod
    def foo(cls, value: str):
        return value


class B(A):
    pass


def test_foo_return():
    flexmock(B).should_receive('foo').and_return('a')
    B.foo('')

    flexmock(B).should_receive('foo').and_return('b')
    B.foo('')


def test_foo_args():
    flexmock(B).should_receive('foo').with_args('a')
    B.foo('a')

but the second test fails with:

E       flexmock.MethodSignatureError: foo()
E       Did not match expectation foo(value="a")

For this issue to happen following conditions must be met:

• classmethod is implemented in base class and derived class is mocked
• before failing test there is a test that mocks return value of the function twice. If the return value is mocked only once then everything works
• second test must call the method on class not on instance
  If everything is done in single test then it works as expected. The failure also does not happen if the mocking of return value is split into separate tests.

This issue started happening with 0.10.5 and is present since then.

[adam-sikora]

Single test to reproduce the issue:

import flexmock


class A:

    @classmethod
    def foo(cls, value: str):
        return value


class B(A):
    pass


def test_foo():
    flexmock.flexmock(B).should_receive('foo').and_return('a')
    B.foo('')

    flexmock.flexmock(B).should_receive('foo').and_return('b')
    B.foo('')

    flexmock._api.flexmock_teardown()

    flexmock.flexmock(B).should_receive('foo').with_args('a')
    B.foo('a')

[adam-sikora]

I have also found out that the issue can be fixed by reversing the order in in which _reset() is called on expectations in flexmock_teardown().

        for expectation in reversed(expectations):
            expectation._reset()

[ollipa]

Thank you for the detailed bug report! I will take a look at this when I have time.

[christophe-riolo]

@ollipa I investigated what's happening and I found the following, here is the minimal code to reproduce in the tests, with what is happening

flexmock(some_module.DerivedClass)
some_module.DerivedClass.should_receive("class_method_with_args")
# Here we created an expectation I will call E1 that has `_local_override` to `True`
# Indeed we created a bound method on `DerivedClass` that copies `SomeClass.class_method_with_args`

some_module.DerivedClass.should_receive("class_method_with_args")
# Here we created an expectation I will call E2 that has `_local_override` to `False`
# Indeed because of E1 `DerivedClass.class_method_with_args` now exists, it's the local override

flexmock_teardown()  # This is creating the issues

So when E1 is reset in the teardown, we successfully delete the local override DerivedClass.class_method_with_args, but when E2 is reset, because we don't know it was actually a local override, then we actually restore DerivedClass.class_method_with_args, so the DerivedClass becomes in a weird status.

Worse, after the teardown, because of the way we restore the method, flexmock now identifies it as a method and not a classmethod, I wonder if this can happen in normal use 🤔

Anyway, we need to be able to convey in E2 that is it still a local override, I was thinking that we could store it in the class in an attribute similar to what we do for the method type, eg. DerivedClass.class_method_with_args__flexmock__local_override instead of storing it in the expectation, because it becomes a property of the method itself more than of the expectation, what do you think ? (actually it could be DerivedClass.class_method_with_args.__flexmock_local_override__ rather, same for the method_type)

[ollipa]

@christophe-riolo, that sounds good to me. I didn't really have time to work on this myself but I can take a look at your PR if you implement the fix.