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 triggering deprecation warnings in 3.0.5 #2118

Closed
nicoddemus opened this Issue Dec 5, 2016 · 24 comments

Comments

Projects
None yet
5 participants
@nicoddemus
Member

nicoddemus commented Dec 5, 2016

As commented by @nedbat in https://bitbucket.org/ned/coveragepy/pull-requests/94, it seems pytest itself is making use of the deprecated compatproperty names:

/Users/ned/coverage/trunk/.tox/py27/lib/python2.7/site-packages/_pytest/fixtures.py:1071: PendingDeprecationWarning: This usage is deprecated, please use pytest.Class instead
  obj = getattr(holderobj, name, None)
/Users/ned/coverage/trunk/.tox/py27/lib/python2.7/site-packages/_pytest/fixtures.py:1071: PendingDeprecationWarning: This usage is deprecated, please use pytest.File instead
  obj = getattr(holderobj, name, None)
/Users/ned/coverage/trunk/.tox/py27/lib/python2.7/site-packages/_pytest/fixtures.py:1071: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  obj = getattr(holderobj, name, None)
/Users/ned/coverage/trunk/.tox/py27/lib/python2.7/site-packages/_pytest/fixtures.py:1071: PendingDeprecationWarning: This usage is deprecated, please use pytest.Instance instead
  obj = getattr(holderobj, name, None)
/Users/ned/coverage/trunk/.tox/py27/lib/python2.7/site-packages/_pytest/fixtures.py:1071: PendingDeprecationWarning: This usage is deprecated, please use pytest.Item instead
  obj = getattr(holderobj, name, None)
/Users/ned/coverage/trunk/.tox/py27/lib/python2.7/site-packages/_pytest/fixtures.py:1071: PendingDeprecationWarning: This usage is deprecated, please use pytest.Module instead
  obj = getattr(holderobj, name, None)

cc @nmundar

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Dec 5, 2016

this may prove tricky, it happens in parse_factories

@nedbat

This comment has been minimized.

Contributor

nedbat commented Dec 5, 2016

Couldn't you put a "with warnings.catch_warnings():" around line 1071? https://docs.python.org/3/library/warnings.html#temporarily-suppressing-warnings

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Dec 5, 2016

i'd rather investigate why we run into those objects, and whether we can avoid the situation to begin with (as we could be hiding other deprecations as well)

@nedbat

This comment has been minimized.

Contributor

nedbat commented Dec 5, 2016

A quick test in my tox virtualenv seems to show that the context manager doesn't prevent the warnings..?

BTW, I run my test suite with:

# We want to see all warnings while we are running tests.  But we also need to
# disable warnings for some of the more complex setting up of tests.
warnings.simplefilter("default")

# Silence specific warnings that are not our fault.
warnings.filterwarnings("ignore", module="xdist", message="type argument to addoption")

So I try to see warnings, but that last line is to suppress a warning that pytest-xdist causes.

@nedbat

This comment has been minimized.

Contributor

nedbat commented Dec 5, 2016

Oops, I hadn't tried catch_warnings correctly. This works, and should only silence this specific warning:

with warnings.catch_warnings():
    warnings.filterwarnings("ignore", message="This usage is deprecated, please use pytest.* instead")
    obj = getattr(holderobj, name, None)
@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Dec 5, 2016

@nedbat if i recall correctly that operation will reset the once filter cache of the warnings module in various python version

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Dec 5, 2016

@nedbat the real bug is, that it tries to operate on a Node object as test object

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Dec 5, 2016

@nicoddemus @nedbat the problem lies in _pytest/python.py:360 - getcustomclass triggers this

i will fix this one

@The-Compiler

This comment has been minimized.

Member

The-Compiler commented Dec 5, 2016

I can confirm this, breaks my testsuite since I use -Werror 😉

Bisected to b38fad4 - cc @nmundar

The-Compiler added a commit to qutebrowser/qutebrowser that referenced this issue Dec 5, 2016

@nicoddemus

This comment has been minimized.

Member

nicoddemus commented Dec 5, 2016

After @RonnyPfannschmidt fixes it, should we do a 3.0.6 release or it can wait a few weeks?

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Dec 5, 2016

@The-Compiler you should never error on pending ones tho ^^

@The-Compiler

This comment has been minimized.

Member

The-Compiler commented Dec 6, 2016

I just blacklisted 3.0.5, so no hurry from my side.

@RonnyPfannschmidt I'll have to disagree - if pytest failed on PendingDeprecationWarnings in its testsuite, I'm pretty we'd have noticed that it's issuing them during normal operation. And in my own testsuite, IMHO it's good to know about deprecated stuff as early as possible 😉

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Dec 6, 2016

@The-Compiler pendingdeprecations are really silent by default

also while investigating we added pending deprecation in a really bad way
i put a real deprecation there because it should fail hard on this one since a while (namely 2.x)

@The-Compiler

This comment has been minimized.

Member

The-Compiler commented Dec 6, 2016

DeprecationWarnings are also silent by default 😉

Your PR still does PendingDeprecationWarning though, right? FWIW I don't think we should change that to a DeprecationWarning with a patch release.

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Dec 6, 2016

i still do the pending ones for the old usage site, i changed the other usage side that uses a old api to log the deprecation

@rgant

This comment has been minimized.

rgant commented Dec 20, 2016

Wish I'd known about this error before I upgraded pytest to 3.0.5. I don't think that 3.0.5 should be available without this issue resolved. I guess I'm going to figure out how to downgrade my pytest install.

hubot pushed a commit to PyCQA/flake8 that referenced this issue Dec 24, 2016

@The-Compiler

This comment has been minimized.

Member

The-Compiler commented Jan 20, 2017

I haven't really had time for much pytest work recently, and just happened to rediscover today that I blacklisted pytest 3.0.5 for qutebrowser 😆

Judging from the commits referencing this issue, others did the same - so I think this should really be fixed for a 3.0.6.

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Jan 20, 2017

i have a related pr in the work, it will be fixed by 3.0.6

@The-Compiler

This comment has been minimized.

Member

The-Compiler commented Jan 20, 2017

I agree with @nicoddemus (this comment and the following ones) that we should rather revert patch-level deprecations and deprecate things properly for the next feature release.

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Jan 20, 2017

@The-Compiler the triggering deprecation will be removed, there is an original deprecation that simply doesnt cause warnings that i want to ressurrect at the same time

@nicoddemus

This comment has been minimized.

Member

nicoddemus commented Jan 20, 2017

IMHO just moving the deprecation that was introduced to the features branch is enough, no need to ressurect the other deprecation because I suspect users don't see it anyway.

This was referenced Mar 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment