Skip to content

No warning when class is skipped due to __init__ #410

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
pytestbot opened this issue Dec 20, 2013 · 18 comments
Closed

No warning when class is skipped due to __init__ #410

pytestbot opened this issue Dec 20, 2013 · 18 comments
Labels
type: enhancement new feature or API change, should be merged into features branch

Comments

@pytestbot
Copy link
Contributor

Originally reported by: Sam Thursfield (BitBucket: samthursfield, GitHub: samthursfield)


I just spent a whole afternoon trying to work out why a testsuite class was being skipped. It turns out it was because there was an init() function in the class.

Py.test doesn't give any kind of warning as to why the function was skipped; it took going through the code and attaching 'pdb' to find the exception that says 'class test_htol_results.TestHtolResultRecords with init won't get collected'. OK, if I'd read the documentation from start to finish I'm sure I'd have known :-) But if Py.test had outputted a warning as to why the testsuite was skipped it would have saved me a lot of time.


@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


Yikes. Sorry you stumbled over this. Indeed i think we need to have a little warning subsystem so that the result at the end would say "1 passed, ..., 1 WARNING" and then have a switch to present those warnings. So i agree with your suggestion to present warnings.

@pytestbot
Copy link
Contributor Author

Original comment by Ronny Pfannschmidt (BitBucket: RonnyPfannschmidt, GitHub: RonnyPfannschmidt):


what version of pytest? recently we do cause a skip for those

@pytestbot
Copy link
Contributor Author

Original comment by Sam Thursfield (BitBucket: samthursfield, GitHub: samthursfield):


Ronny: it's version 2.5.2 of pytest

@pytestbot
Copy link
Contributor Author

Original comment by Torsten Landschoff (BitBucket: bluehorn, GitHub: bluehorn):


Yesterday the same thing happened to us: Somebody added a init to a test class and somehow all the tests in the class were skipped.

We finally found the reason here from http://pytest.org/latest/goodpractises.html?highlight=discovery#conventions-for-python-test-discovery:

pytest implements the following standard test discovery:
[...]

  • Test prefixed test classes (without an init method)
    [...]

Is there any reason why a test class can not have an init method without arguments? Interestingly enough, it is possible to override new in a test class.

Observe:

torsten@horatio:.../testclass_init$ cat test_init.py 
class TestInit(object):

    def __init__(self):
        pass

    def test_method(self):
        pass
torsten@horatio:.../testclass_init$ py.test -rsx -v test_init.py 
============================ test session starts =============================
platform linux2 -- Python 2.7.6 -- pytest-2.5.1 -- /opt/dynasdk/loco2-precise/bin/python
plugins: cov, capturelog, xdist
collected 0 items / 1 skipped 

========================== short test summary info ===========================
SKIP [1] /opt/dynasdk/loco2-precise/lib/python2.7/site-packages/_pytest/python.py:503: class test_init.TestInit with __init__ won't get collected
========================= 1 skipped in 0.06 seconds ==========================

Compare this with overriding new:

(loco2-precise)torsten@horatio:~/pytest-bug/testclass_init$ cat test_new.py 
class TestNew(object):

    def __new__(cls):
        print "new called"
        return object.__new__(cls)

    def test_a(self):
        print "TestNew.test_a"

    def test_b(self):
        print "TestNew.test_b"
(loco2-precise)torsten@horatio:~/pytest-bug/testclass_init$ py.test -rsx -v test_new.py 
==================================================================================== test session starts ====================================================================================
platform linux2 -- Python 2.7.6 -- pytest-2.5.1 -- /opt/dynasdk/loco2-precise/bin/python
plugins: cov, capturelog, xdist
collected 2 items 

test_new.py:7: TestNew.test_a PASSED
test_new.py:10: TestNew.test_b PASSED

================================================================================= 2 passed in 0.02 seconds ==================================================================================

What's also interesting is the calling sequence:

(loco2-precise)torsten@horatio:~/pytest-bug/testclass_init$ py.test --capture=no test_new.py 
==================================================================================== test session starts ====================================================================================
platform linux2 -- Python 2.7.6 -- pytest-2.5.1
plugins: cov, capturelog, xdist
collecting 0 itemsnew called
new called
new called
collected 2 items 

test_new.py new called
TestNew.test_a
.new called
TestNew.test_b
.

================================================================================= 2 passed in 0.01 seconds ==================================================================================

So the dunder new method is called three times during collection of the two tests (unexpected to me), while when running the tests each method gets a new instance of the class (as expected).

I would prefer that dunder init (without arguments) works just as dunder new.

@pytestbot pytestbot added the type: enhancement new feature or API change, should be merged into features branch label Jun 15, 2015
@ghostsquad
Copy link

👍

@nicoddemus
Copy link
Member

Current master displays warnings for this case. Consider:

# content of test_foo.py
class Test:

    def __init__(self):
        pass

    def test_foo(self):        
        pass

def test_bar(): 
    pass
py.test test_foo.py
============================= test session starts =============================
platform win32 -- Python 3.4.3, pytest-2.8.0.dev4, py-1.4.30, pluggy-0.3.0
rootdir: X:\pytest, inifile: tox.ini
collected 1 items

test_foo.py .

==================== 1 passed, 1 warnings in 0.01 seconds =====================

(And the footer "1 passed, 1 warnings" even appears in yellow)

Running with -rw, it displays a more verbose message:

test_foo.py .

=============================== warning summary ===============================
WC1 X:\pytest\test_foo.py cannot collect test class 'Test' because it has a __init__ constructor
==================== 1 passed, 1 warnings in 0.01 seconds =====================

So I'm closing this issue. If people would like to discuss if having an __init__ without parameters should be collected, I think a new issue should be opened. 😄

@h-vetinari
Copy link

h-vetinari commented Jan 9, 2019

What's the reason a TestClass with __init__ cannot be collected?

I have a hierarchy of objects and want to emulate this hierarchy on the test side, so that each TestChildClass inherits all the tests from TestParent (just like it inherits the methods on the code side). For organisational purposes, it would be very helpful if I could define an __init__ on my test-class.

@ruslanoid
Copy link

ruslanoid commented Jan 16, 2019

just stumbled upon this behaviour.
i am dealing with older already written tests that are executed by a different test runner and the structure there is having a single test() method for a Test() class - these classes do have the __init__() but nothing is passed to it.

my question is:

  • could i use some pytest hook during collection that would replace the __init__() methods to setup_class() - or something of this sort?

my preferable way would be to avoid changing the code of the Test classes themselves as it still needs to be runnable via the legacy runner and if pytest could collect and run those tests by first running the __init__() as setup_class() and then the test() method as the other runner does - it would help a lot

@nicoddemus
Copy link
Member

Perhaps we can relax this restriction a bit and allow __init__ methods which don't accept any other arguments other than self. Do you see a problem with this @RonnyPfannschmidt?

@pylang
Copy link

pylang commented Mar 10, 2019

I'm not understanding why having a bare __init__ is an issue...

@nicoddemus
Copy link
Member

I think it is a matter of that TestCase classes usually don't have __init__ methods at all, so pytest tried to be conservative initially to not collect classes with that method implemented. As I said, I think it is OK to accept test classes whose __init__ methods don't accept parameters other than self.

@RonnyPfannschmidt
Copy link
Member

someone should demonstrate a good use-case first in particular one that's not covered better with fixtures or xunit setup methods,else i'm -1

@RonnyPfannschmidt
Copy link
Member

to elaborate on that - https://github.com/pytest-dev/pytest/blob/master/src/_pytest/python.py#L194 only opts out of unsafe classes if there is no result, so a testsuite in transition is perfectly fine to have a own pytest_pycollect_makeitem hook that's returning a Class

@pylang
Copy link

pylang commented Mar 12, 2019

I think it would consume a lot of time trying to find a superior use case, as there may not be one. I only stumbled on this warning while transitioning from nose to pytest with a class similar to the following:

# Old -> warning
class TestData:
    def __init__(self):
        self.data = DATA
    ...

# New -> fixed
class TestData:
    data = DATA
    ...   

Luckily the fix was simple, but since the first approach is a valid way to make regular classes (even in nose), it was puzzling why it needed to be changed. I just cannot recall a Python idiom that discourages bare __init__ methods, so its jarring to be warned about a non-obvious issue without a clear rationale. Shouldn't we be able to write simple classes however we wish? If not, is there mention in the documentation or guidelines on how to make a test class that pytest expects?

I don't mind improving idioms provided the rationale is clear and generally pythonic. Many thanks.

@nicoddemus
Copy link
Member

Shouldn't we be able to write simple classes however we wish?

I believe this requirement stems from the fact that TestCase subclasses also don't implement __init__ in general, using setUp and tearDown to initialize its internal data. Another reason might be to avoid collecting classes which are not supposed to be test classes; one of the ways for that is to see if they have an __init__ method.

I also suspect that nose doesn't follow this same guideline on purpose, perhaps by accident or by design, I don't know.

If not, is there mention in the documentation or guidelines on how to make a test class that pytest expects?

Sure, they are mentioned here:

... or methods inside Test prefixed test classes (without an init method)

Not other requirements other than that are needed I believe.

In the end, we could implement the capability of definining __init__ methods in test classes, but I'm not sure we are doing users a favor in that regard given that there are more powerful mechanisms already in place, like fixtures and setup_method. And as you said, if you are converting an existing test suite it is usually simple to get rid of single-parameter __init__ methods. __init__ methods which receive multiple parameters will require more work anyway, so those cases won't benefit from this change.

@johntiger1
Copy link

Was also stuck on this issue for a while!

@graingert
Copy link
Member

graingert commented Jun 19, 2020

@nicoddemus imho __init__ support kinda makes sense for fixtures:

class TestFoo:
    def __init__(self, tmp_path, postgres):
        self.path = tmp_path / "foo"
        self.data = some_data()

maybe it also makes sense for attr.s/dataclasses:

@attr.ib(frozen=True)
class TestFoo:
    tmp_path = attr.ib()
    postgres = attr.ib()

    def test_foo(self):
        path = self.tmp_path / "foo"

although notably less powerful, as you cannot create yield fixtures

@RonnyPfannschmidt
Copy link
Member

@graingert all of those examples can be sorted by a class level fixture, with the advantage of zero ambiguity

kochanczyk added a commit to kochanczyk/PyBNF that referenced this issue Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

No branches or pull requests

9 participants