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

Fails collecting tests if test module level obj has __getattr__() #1035

Closed
Suor opened this issue Sep 21, 2015 · 21 comments
Closed

Fails collecting tests if test module level obj has __getattr__() #1035

Suor opened this issue Sep 21, 2015 · 21 comments
Assignees
Labels
status: critical grave problem or usability issue that affects lots of users type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog type: bug problem that needs to be addressed
Milestone

Comments

@Suor
Copy link

Suor commented Sep 21, 2015

The issue is in this line. Object returns something arbitrary and then it's required to be int lineno.

Breaks this for example.

Suor added a commit to Suor/funcy that referenced this issue Sep 21, 2015
@RonnyPfannschmidt RonnyPfannschmidt added this to the 2.8.1 milestone Sep 21, 2015
@RonnyPfannschmidt RonnyPfannschmidt added type: bug problem that needs to be addressed type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog labels Sep 21, 2015
@alfredodeza
Copy link
Contributor

This only happens for me on Python 2.6. Python 2.7 is unaffected.

@nicoddemus
Copy link
Member

Thanks for the report! 😄

@RonnyPfannschmidt
Copy link
Member

Since its only a proble!m on python 2.6 I'm considering leaving it be

@Suor
Copy link
Author

Suor commented Sep 25, 2015

So pytest no longer supports Python 2.6?

@RonnyPfannschmidt
Copy link
Member

its still supported, but since python 2.6 is eol'd since years we should think about how much code and maintenance burdens we invest in it as volunteer work, in particular with libs that are somehow broken only on it

@hpk42 whats your oppinion on that matter

@RonnyPfannschmidt
Copy link
Member

just discussed with @hpk42 on irc

result: since we officially support it we should fix it

@jantman
Copy link

jantman commented Sep 25, 2015

FWIW, I can confirm what @alfredodeza said - https://travis-ci.org/jantman/awslimitchecker/builds/82210772 tests against 26, 27, 32, 33, 34, pypy and pypy3; only 2.6 breaks.

@RonnyPfannschmidt
Copy link
Member

at unit-test level all python packages seem to be required

@RonnyPfannschmidt
Copy link
Member

i finished a first attempt to solving it, resulting in the python interpreter oom-ing my machine,

@RonnyPfannschmidt RonnyPfannschmidt modified the milestones: 2.8.2, 2.8.1 Sep 27, 2015
codenrhoden pushed a commit to codenrhoden/ceph-deploy that referenced this issue Sep 30, 2015
To avoid pytest-dev/pytest#1035

Signed-off-by: Travis Rhoden <trhoden@redhat.com>
codenrhoden pushed a commit to codenrhoden/ceph-deploy that referenced this issue Sep 30, 2015
To avoid pytest-dev/pytest#1035

Signed-off-by: Travis Rhoden <trhoden@redhat.com>
codenrhoden pushed a commit to codenrhoden/ceph-deploy that referenced this issue Sep 30, 2015
To avoid pytest-dev/pytest#1035

Signed-off-by: Travis Rhoden <trhoden@redhat.com>
@The-Compiler
Copy link
Member

It seems this is caused by 3497aa0 / #921

/cc @tomviner

@qwcode
Copy link

qwcode commented Oct 2, 2015

to be clear, this started with 2.8.0, correct?

@The-Compiler
Copy link
Member

Yes, it seems to work fine with 2.7.3 which is before that commit.

@tomviner
Copy link
Contributor

tomviner commented Oct 6, 2015

The difference in Python versions seems to be with inspect.isclass:

$ python2.7
Python 2.7.9 (default, Apr  2 2015, 15:33:21)
[GCC 4.9.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from inspect import isclass
>>> from whatever import _
>>> isclass(_)
False

which is the correct answer btw - _ is <whatever.Whatever object at 0x7ff296b57210>, which is an instance not a class. But in Python 2.6:

$ python2.6
Python 2.6.9 (default, Apr 16 2015, 18:31:03)
[GCC 4.9.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from inspect import isclass
>>> from whatever import _
>>> isclass(_)
True

My commit, mentioned above, amends the collector.istestclass line below, but as you can see it's the isclass line above it that's, in the first instance, behaving erroneously in Py2.6:

def pytest_pycollect_makeitem(collector, name, obj):
    ...
    if isclass(obj):
        if collector.istestclass(obj, name):
            Class = collector._getcustomclass("Class")
            outcome.force_result(Class(name, parent=collector))

See pytest_pycollect_makeitem.

So I propose the priority fix, is for us to provide a fixed isclass function when Python version == 2.6. I'd be happy to write a patch for that.

There's a second, and more questionable issue raised here. In my patch, I added support to respect nose's Test.__test__ attribute to mark a class as a test despite not matching any naming rule. But is a class that responds True to getattr(cls, anything) really signalling anything about tests?

The good news is that if we fix the first issue, we're not forced to answer the second (in this ticket).

@nicoddemus
Copy link
Member

Thanks for the investigation work @tomviner! It seems your solution of using a fixed isclass function for Python 2.6 would work just fine and would not break anything for Python versions > 2.6, so I'm 👍 on it.

@The-Compiler
Copy link
Member

The probably related CPython commit: https://hg.python.org/cpython/rev/35bf8f7a8edc?revcount=1920

@tomviner
Copy link
Contributor

tomviner commented Oct 7, 2015

Comments on this patch please #1121

@nicoddemus
Copy link
Member

@Suor, could you please try #1121 in your test suite to ensure it works and does not break anything else by accident?

@The-Compiler
Copy link
Member

@alfredodeza @jantman @Renstrom @Daikiri @jmoldow @trhoden any chance some of you could test that PR as well? 😉

@dairiki
Copy link

dairiki commented Oct 8, 2015

PR #1121 works for me. Thank you all!

@nicoddemus
Copy link
Member

Fixed in #1121

codenrhoden pushed a commit to codenrhoden/ceph-deploy that referenced this issue Oct 9, 2015
To avoid pytest-dev/pytest#1035

Signed-off-by: Travis Rhoden <trhoden@redhat.com>
codenrhoden pushed a commit to codenrhoden/ceph-deploy that referenced this issue Sep 27, 2016
To avoid pytest-dev/pytest#1035

Signed-off-by: Travis Rhoden <trhoden@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: critical grave problem or usability issue that affects lots of users type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog type: bug problem that needs to be addressed
Projects
None yet
Development

No branches or pull requests

9 participants