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

Python3: problem with new_instances isolation #13

Closed
obestwalter opened this Issue Jan 8, 2016 · 13 comments

Comments

Projects
None yet
4 participants
@obestwalter

obestwalter commented Jan 8, 2016

I am porting our code from python2.7 to python3.4 and tests started failing due to missing isolation in tests where new_instances is used. Testframework is pytest 2.8.4

I thought if it's an obvious bug it's easy to reproduce so I created a minimal testcase I would expect to reproduce it, but sadly it doesn't. I add it here anyway, because it illustrates the problem.

file one:

class OriginalClass(object):
    def __init__(self):
        pass

file two:

from flexmock import flexmock

from one import OriginalClass

class TestA(object):
    def test_reproduce_flexmock_bug(self):
        flexmock(OriginalClass).new_instances(flexmock(bla=lambda: True))
        print(OriginalClass().__class__)
        assert OriginalClass().__class__.__name__ == 'MockClass'


class TestB(object):
    def test_check_isolation(self):
        print(OriginalClass().__class__)
        assert OriginalClass().__class__.__name__ == 'OriginalClass'

I would expect test_check_isolation to fail because the new_instances mock is wrongly still active from the other test, but this works with no problem on python2 and python3.

A workaround in the production code is two separate the two classes that affect each others test behaviour into two different modules.

I would like to understand why this is happening and what else could cause this behaviour. Any ideas?

@bkabrda

This comment has been minimized.

Owner

bkabrda commented Jan 11, 2016

This is weird. I'll try looking into that as soon as I get some free time. Thanks for the report!

@obestwalter

This comment has been minimized.

obestwalter commented Jan 13, 2016

Thanks but as long as nobody else reports this problem I wouldnt' even bother. I will try to have another look when I get around to it to find if I missed something in the real world tests, that makes something like this possible and maybe I find something out.

@obestwalter

This comment has been minimized.

obestwalter commented Jan 13, 2016

I would appreciate some hints how I could hunt that problem down, if you have an idea though :)

@bkabrda

This comment has been minimized.

Owner

bkabrda commented Jan 13, 2016

I don't have any immediate advice, but there are some things to consider that might help while trying to reproduce this:

  • Is the mocked class a subclass of another class?
  • Does it use multiple inheritance?
  • Does it use a custom metaclass?
  • Does it subclass object in your code on Python 2?
  • If you have access to a different Python 3.X, could you also try testing on that and confirm if the problem occurs there?
  • Is this a class from your application or perhaps from an open source dependency that I could have a look at?
  • Can you reproduce this if you use "file two" from your report, but the actual class from your application that causes the issue?

Maybe answers to these questions can help us get a reproducer or at least an idea what might be causing the problem.

@obestwalter

This comment has been minimized.

obestwalter commented Jan 13, 2016

I worked through your list and the last one lead me to a minimal test case (only on file necessary and supersimple), which I could swear I tried already before. Maybe I am halluzinating as well, because the day was very long ... anyway here it is. It passes on 2.7.6 and fails on 3.4.3 and 3.5.0.

from flexmock import flexmock

class OriginalClass(object):
    def __init__(self, arg):
        self.arg = arg


class TestA(object):
    def test_reproduce_flexmock_bug(self):
        flexmock(OriginalClass).new_instances(flexmock(bla=lambda: True))
        assert OriginalClass(1).__class__.__name__ == 'MockClass'


class TestB(object):
    def test_check_isolation(self):
        assert OriginalClass(1).__class__.__name__ == 'OriginalClass'

This is the Error:

FAILED
self = <test_fm_bug.TestB object at 0x7f19d1cb99e8>

    def test_check_isolation(self):
>       assert OriginalClass(1).__class__.__name__ == 'OriginalClass'
E       TypeError: object() takes no parameters

self = <test_fm_bug.TestB object at 0x7f19d1cb99e8>
@bkabrda

This comment has been minimized.

Owner

bkabrda commented Jan 14, 2016

Cool, thanks a lot, I can reproduce the error on Python 3 now. I don't have much time ATM, but hopefully I'll get some during weekend or next week to debug this.

@bkabrda

This comment has been minimized.

Owner

bkabrda commented Jan 18, 2016

I believe we're hitting a bug in Python 3 itself [1]. The minimal reproducer actually doesn't need flexmock at all:

class A(object):
    def __init__(self, x):
        pass

def f(self, x):
    pass

x = A.__new__
A.__new__ = f
A.__new__ = x

A(1)

It seems that there's nothing I can do about this until this is fixed in Python. I'll leave this issue opened for now and I'll keep an eye on that Python issue to see where this will go.

[1] http://bugs.python.org/issue25731

@obestwalter

This comment has been minimized.

obestwalter commented Jan 18, 2016

Wow. Nasty. Ok, might be fixed then in 3.5.X ... until then all we can do is separate tests using new_instances into their own modules. Not too painful if one knows about it.

@bkabrda

This comment has been minimized.

Owner

bkabrda commented Jan 19, 2016

Yup. I'll try to release new version with the other fix today; I'll add a note about this bug to documentation.

bkabrda added a commit that referenced this issue Jan 20, 2016

@bkabrda

This comment has been minimized.

Owner

bkabrda commented Jan 20, 2016

I added a note in the documentation about this issue. I was experimenting with the Python issue 25731 fix and it does fix this bug without any changes required in flexmock. This means there is nothing else I can do about this, so I'm closing this issue.

@carldunham

This comment has been minimized.

carldunham commented Nov 18, 2016

So until this makes it into Python, is there a work-around?

@bkabrda

This comment has been minimized.

Owner

bkabrda commented Nov 21, 2016

As @obestwalter wrote, you can "separate tests using new_instances into their own modules". Other than that, I don't think there's another workaround.

@Incanus3

This comment has been minimized.

Incanus3 commented Dec 7, 2016

Hi,
I'm getting a different issue, but I think it still matches the 'Python3: problem with new_instances isolation' description. I'm stubbing new instances of my HTTPClient wrapper to verify correct behavior of the program on network failures. The stubbing goes like this:

http_error = HTTPError('broken for testing')
broken_http_client = flexmock(HTTPClient('example.com'))
broken_http_client.should_receive('request').and_raise(http_error)
flexmock(HTTPClient).new_instances(broken_http_client)

The test that does this stubbing works correctly, but once I added this tests, other tests that result in the HTTPClient constructor being called broke down with the following error:

flexmock.MethodSignatureError: __new__(<class 'lib.http.HTTPSClient'>, "pg.eet.cz", proxy=None, logger=<eet.loggers.EETLogger object at 0x7f973674d9e8>, verify=True)

The method signature is correct as verified by both the tests passing before adding this one and the program communicating correctly over the network. But even if the method signature was wrong, this test shouldn't affect the other ones. I also tried to move the test into it's own class in a separate file (package), but nothing changed.

P.S. I'm using python version 3.5.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment