Pytest 3.7.1 isn't keeping class variables set by autouse function #3778
Comments
nicoddemus
added
type: bug
|
We don't have this issue with |
|
@ashahba I'll test things locally really quick |
|
Can confirm |
|
Thanks @s1113950! |
|
I've bisected a problem myself to c6b11b9 - which is about missing methods in a decorated fixture: def decorated_drf_client(f):
@wraps(f)
def wrapper(*args, **kwargs):
drf_client = f(*args, **kwargs)
def assert_status(url_or_response, expected_status,
expected_data=None, data=None, method=None,
format=None):
__tracebackhide__ = True
…
return drf_client
return wrapper
@decorated_drf_client
@pytest.fixture
def drf_client(db):
return CustomAPIClient(HTTP_ACCEPT='application/json; version=1.0')The I've put it here a comment since it sounds very similar. @s1113950 Mine appears to be the same as #3774 at least apparently. |
|
@blueyed #3780 should fix your issue, could you try it out just to make sure we are not missing anything? @s1113950 #3780 should also fix your issue if you change the order of the decorators: class CommandTester(object):
@pytest.fixture(scope='class', autouse=True)
@classmethod
def setup(self):
self.registry = os.getenv('MLT_REGISTRY', 'localhost:5000')
self.tfjob_templates = ('tf-dist-mnist', 'tf-distributed')As discussed in #3780 we plan to eventually raise an error if fixtures are called directly, and applying |
|
I tried using version 😞 We're also running |
|
Ahh good to know. We need another fix, fortunately it is simple. Thanks for reporting! |
|
Our project is open source if you want to repro for yourself 😄 : https://github.com/IntelAI/mlt (see It should repro itself without those env vars set, but just in case. Otherwise I can try a different fix that you guys make, or try and submit one myself when I have time (probably after next Wednesday when sprint restarts) |
|
@s1113950 thanks, but fortunately this is trivial to reproduce in isolation. 👍 Unfortunately after giving this a go I think there's no easy fix for this, please read #3781. The bottom line is that fixtures decorated with Fortunately there's an easy workaround: --- foo1.py 2018-08-09 18:12:46.211165800 -0300
+++ foo2.py 2018-08-09 18:12:58.771387100 -0300
@@ -1,6 +1,6 @@
class CommandTester(object):
- @classmethod
@pytest.fixture(scope='class', autouse=True)
- def setup(cls):
+ def setup(self):
+ cls = type(self)
cls.registry = os.getenv('MLT_REGISTRY', 'localhost:5000')
cls.tfjob_templates = ('tf-dist-mnist', 'tf-distributed') |
|
👍 thanks for the workaround! |
|
@nicoddemus |
|
@blueyed no, it is all related to that now we wrap the fixture function itself in order to generate a warning in case it is called directly (#3661). Really a shame that such a seemingly simple change has caused so much problems. Before I mentioned:
But it seems this might not be actually possible in the end, see #3781 (comment). Can you see if the workaround in #3778 (comment) works for you? It should work for any pytest version. |
|
@nicoddemus |
|
Oh sorry @blueyed I missed your comment! 😅 Does changing the order of the decorators still gives an error? @pytest.fixture
@decorated_drf_client
def drf_client(db):
return CustomAPIClient(HTTP_ACCEPT='application/json; version=1.0') |
|
@nicoddemus the workaround didn't work, getting original error again: where it isn't called as expected. my patch attempt |
|
@s1113950 sorry, I wasn't clear enough. You need to set the attributes to the class explicitly instead of relying on the function receiving a class as first argument because of the Try this: class CommandTester(object):
@pytest.fixture(scope='class', autouse=True)
def setup(self):
type(self).registry = os.getenv('MLT_REGISTRY', 'localhost:5000')
type(self).tfjob_templates = ('tf-dist-mnist', 'tf-distributed') |
|
That worked! Thanks so much |
|
Great, thanks for checking! Let's leave this open for now until we see if @blueyed has the same problem. 👍 |
|
@nicoddemus |
|
What exactly will be the intended behavior at the end of all this? Will the original behavior be supported eventually? Or will there be a new way? Just trying to think of the migration path I should take. |
|
IMHO we should just go ahead and say this is not supported because:
@blueyed @asottile @RonnyPfannschmidt thoughts? |
I'm inclined to agree, |
|
Well let's close this as "won't fix because it worked by accident, with an acceptable workaround" and also because we just couldn't make it work in #3781. If someone can come up with a reasonable patch in the future we can revisit this. 👍 |
How does this relate to the documentation? At face value it seems contradictory. Should the docs be updated or am I missing something? |
|
OK so yeah this is happening to me too on python So now for example in my test It was working fine until all of a sudden this behavior as you documented it above started happening. My older docker containers do not have this issue. It seems like maybe it's an operating system update that does it, or something??? Or maybe when I upgraded Pycharm? So then I changed my base class like this: Now it works. But what the heck? Why did this suddenly start being needed? |
|
@nddipiazza if you read the whole thread the answer is there -- I'll reiterate though -- it used to work by accident, it was broken in pytest 3.7 (way back in 2018), it's hard (impossible?) to get it working in all cases and there's a very easy workaround so it was decided that it will not be supported |
Link to sample failure (it's a public project so hopefully you can see this?)
https://circleci.com/gh/IntelAI/mlt/1857
pytest env:
The vars get set as they used to, but then in between the variables being set by this class here and when the first test is called, the variables disappear:
I stepped through every line that was called, and it was all stuff in
pytestor an associated package. In the beginning you'll notice thatself.registryis indeed set.full
pip listis here:This used to work with the prior version of
pytest(we hadn't peggedpytestas we should have and tests all broke after the 8/2 release).EDIT
Pegging to these versions made things work again:
https://github.com/IntelAI/mlt/commit/2d5ec3251da295051b0862fa0db3c6e71dbb22ec
The text was updated successfully, but these errors were encountered: