prefer Node.get_closest_marker

rib3 commented 5 years ago

Pytest 4.1 dropped Node.get_marker.

this PR adds a preference to use get_closest_marker, otherwise it falls back to get_marker

Alternate to #5

theY4Kman commented 5 years ago

Ayyyoooo, danke schön again for the PR suite.

This one I close in favour of #7 because I'd rather choose the proper method once at startup, to avoid adding extra jumps to stack traces (though that's a weak point), and in this particular case, a double call to __getattr__.

AFAIK, under the hood, hasattr() is basically implemented as:

def hasattr(o, attr):
    try:
        getattr(o, attr)
    except AttributeError:
        return False
    else:
        return True

So, to avoid an extra call, I might've written the get_closest_marker method in this PR as:

def get_closest_marker(item, *args, **kwargs):
    for attr in 'get_closest_marker', 'get_marker':
        try:
            meth = getattr(item, attr)
        except AttributeError:
            continue
        else:
            return meth(*args, **kwargs)
    else:
        raise RuntimeError('Unable to determine get_closest_marker method.')

rib3 commented 5 years ago

yeah, no problem, there are a few different ways to implement this.

I thought about fetching it once like you ended up doing in #7:

https://github.com/theY4Kman/pytest-only/blob/9e03cdb51cb81b09c9c8f746402f594b14686618/pytest_only/compat.py#L5-L8

But I figured i should just send it your way so you could decide how you wanted to do it.

Thanks for the pytest plugin!

theY4Kman commented 5 years ago

Cheers, man! I think you made a good call; the PR's existence is 80% of the work. I actually updated #7 after rereading this PR, because simply calling Node.get_closest_marker would ignore any overrides by subclasses of Node. So, thank you :P

I released version 1.2.1 on PyPI: https://pypi.org/project/pytest-only/

Holler if you run into any issues

theY4Kman / pytest-only

prefer Node.get_closest_marker #6