Skip to content
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

pytest 8.2.0 calls @propertys during unittest collection #12446

Closed
grzegorzmiazga opened this issue Jun 10, 2024 · 11 comments · Fixed by #12781
Closed

pytest 8.2.0 calls @propertys during unittest collection #12446

grzegorzmiazga opened this issue Jun 10, 2024 · 11 comments · Fixed by #12781
Labels
plugin: unittest related to the unittest integration builtin plugin type: performance performance or memory problem/improvement type: regression indicates a problem that was introduced in a release which was working previously

Comments

@grzegorzmiazga
Copy link

grzegorzmiazga commented Jun 10, 2024

Since 8.2.0 and changes to unittest collecting pytests behaves now slightly differently. If there is @Property defined in unittest class it is called during tests collection. This leads to some absurd behaviour with each property being collected twice. Attached example takes 20s to collect instead of 0.01s

import unittest
from time import sleep

class TestClass(unittest.TestCase):

    @property
    def variable(self):
        print('My collection sleep')
        sleep(10)


class MyTest(TestClass):
    def test_simple(self):
        assert True

pip list output:

pip list
Package                   Version
------------------------- ---------
attrs                     23.2.0
cffi                      1.16.0
cryptography              42.0.8
exceptiongroup            1.2.1
iniconfig                 2.0.0
jsonschema                4.22.0
jsonschema-specifications 2023.12.1
packaging                 24.1
pip                       22.0.4
pluggy                    1.5.0
pycparser                 2.22
pyOpenSSL                 24.1.0
pytest                    8.2.2
referencing               0.35.1
rpds-py                   0.18.1
setuptools                58.1.0
tomli                     2.0.1
@grzegorzmiazga
Copy link
Author

@bluetech I think this might be similar to #12425 but wasn't entirely sure if it is same case so reported it separately.

@The-Compiler
Copy link
Member

@bluetech
Copy link
Member

I'm not near a computer, the property issue is familiar (my first patch to pytest was fixing exactly that..), but the _ part sounds very strange, does it really not happen if you remove the _variable assignment?

@grzegorzmiazga grzegorzmiazga changed the title pytest 8.2.0 calls properites it collects matching variables in __init__ pytest 8.2.0 calls properites during tests collecting Jun 10, 2024
@grzegorzmiazga
Copy link
Author

grzegorzmiazga commented Jun 10, 2024

Right, sorry. In our code when _variable is not assigned we get exception thrown right away in code so I got wrong impression of this. Updated bug title and description. Seems just @property is needed to reproduce it.

@bluetech
Copy link
Member

Right. So the situation is this:

  • During collection, pytest needs to find fixtures (methods decorated with @pytest.fixture)
  • To do this, it scans all of the attributes of the class
  • Scanning an attribute of a class, when the attribute is a property, causes it to run

This has always been an issue with non-unittest test classes. In 8.2 it also started becoming an issue for unittest classes, because we've made the unittest code consistent with the non-unittest code. Unfortunately it also exposes unittest classes to this deficiency.

The reason unittest wasn't affected previously is that it used to work like this: do the collection on the class itself, not an instance, then bind it to an instance only later. When you access the property attribute on the class itself, the property doesn't trigger.

You might ask, if the old unittest was safer, why not switch non-unittest to that instead of the other way? The reason is that it's a much more impactful change, and brings some issues of its own. For example, to which instance should a class-scoped fixture method be bound?

In general I do think collecting on the class instead of an instance is probably better, but it will need some work and breaking changes to get there.

Another fix to the issue is check if the attribute is a property before accessing it and skip it if so. @RonnyPfannschmidt has a draft PR to do it (it's the oldest open PR). Downside of that is performance overhead, breaking change, and some technical issues.

@grzegorzmiazga
Copy link
Author

Is there any way to work around this? Like some way to make property be ignored while scanning?

@bluetech
Copy link
Member

I'm afraid not.

@grzegorzmiazga
Copy link
Author

So currently any project which has more complex properties in test classes should either stick to 8.1.2 or accept that test will just take longer due to collecting running those properties?

@bluetech
Copy link
Member

Yes

@RonnyPfannschmidt
Copy link
Member

i'll evaluate if we can put it behind a config option

@StefanBRas
Copy link

As a workaround, you can set an env var and then wrap property decorators like so:

import unittest
from time import sleep
import pytest
import os

@pytest.fixture(autouse=True, scope='session')
def set_is_running_tests():
    os.environ['IS_RUNNING_TESTS'] = 'True'
    yield
    del os.environ['IS_RUNNING_TESTS']

def t_property(func):
    @property
    def wrapper(*args, **kwargs):
        if os.environ.get('IS_RUNNING_TESTS'):
            return func(*args, **kwargs)
    return wrapper



class TestClass(unittest.TestCase):

    @t_property
    def variable(self):
        print('Inside variable property')
        sleep(2)
        return 1 


class MyTest(TestClass):
    def test_simple(self):
        assert self.variable == 1

using t_property it takes 2.01s to run, with property it takes 6.01s to run.

@Zac-HD Zac-HD added type: performance performance or memory problem/improvement type: regression indicates a problem that was introduced in a release which was working previously plugin: unittest related to the unittest integration builtin plugin labels Jun 24, 2024
@Zac-HD Zac-HD changed the title pytest 8.2.0 calls properites during tests collecting pytest 8.2.0 calls @propertys during test collection Jun 24, 2024
@Zac-HD Zac-HD changed the title pytest 8.2.0 calls @propertys during test collection pytest 8.2.0 calls @propertys during unittest collection Jun 24, 2024
patchback bot pushed a commit that referenced this issue Sep 8, 2024
nicoddemus pushed a commit that referenced this issue Sep 8, 2024
Resolves #12446

(cherry picked from commit c6a5290)

Co-authored-by: Anthony Sottile <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: unittest related to the unittest integration builtin plugin type: performance performance or memory problem/improvement type: regression indicates a problem that was introduced in a release which was working previously
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants