Issue1035 getattr

[tomviner]

Fix for #1035

Have confirmed the test fails on Python 2.6 without the fix.

Appreciate feedback as to whether the python.py bit should be tidied away into a function?

[The-Compiler]

Looks good to me, thanks! Guess that's a good reason to build/test the 2.8.2 release again 😉

Could you also add an entry about this to the CHANGELOG please?

[RonnyPfannschmidt]

can we have a python version specific constant

CLASS_TYPES = ...

and just always implement isclass with a note to drop the implementation after python2.6 is dropped?

[The-Compiler]

That'd probably be (type, types.ClassType) for Python2 and type for Python 3. I'm fine with both solutions.

[The-Compiler]

This didn't make it into 2.8.2, sorry - let's just do a 2.8.3 soon-ish instead of further delaying it 😉

[jantman]

This looks good to me - https://travis-ci.org/jantman/awslimitchecker/builds/84219885 and https://github.com/jantman/awslimitchecker/tree/pytest-pr-1121

[tomviner]

Changelog updated after confirming gitiquette. Still want to replace the capability test with a version test @RonnyPfannschmidt ?

Also, should we assess usage of inspect.isclass outside python.py, in case the Python 2.6 bug is waiting to cause issues for people implementing __getattr__?

[RonnyPfannschmidt]

My preference is A check for the python version or unconditional implementation since that's simpler code

[nicoddemus]

Perhaps it would be better to only re-implement isclass on py26?

if  sys.version_info[:2] == (2, 6):
    def isclass(object):
        """Return true if the object is a class. Overrides inspect.isclass for python 2.6
        because it will return True for objects which always return something
        on __getattr__ calls (see #1035).
        Backport of https://hg.python.org/cpython/rev/35bf8f7a8edc
        """
        return isinstance(object, (type, types.ClassType))

This would make it obvious that it could be removed once py26 support is dropped, plus it will have the benefit of not having the chance to introduce collateral side-effects in other Python versions. 😄

[tomviner]

+1 @nicoddemus Support py26 without affecting other versions.

[RonnyPfannschmidt]

@nicoddemus sounds good 👍, we still might need to check other codepaths in pylib (afair it also uses isclass somewhere in py.code)

[tomviner]

PR updated based on @nicoddemus's suggestion.

[nicoddemus]

@RonnyPfannschmidt @hpk42 how about creating a _pytest._compat module to host this kind of "compatibility workaround" into a single place? There are other cases through the code where we do stuff differently based on python version. This would make simpler to remove code once support for a python version is dropped.

(this is out of the scope of this PR, just thought I would take this opportunity to mention it)

[RonnyPfannschmidt]

good idea, please open a issue :)

[nicoddemus]

Cool: #1125

[nicoddemus]

Thanks @tomviner! 😄