-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Actually deprecate long standing features #3978
Actually deprecate long standing features #3978
Conversation
src/_pytest/nodes.py
Outdated
# "usage of {owner!r}.{name} is deprecated, please use pytest.{name} instead".format( | ||
# name=self.name, owner=type(owner).__name__), | ||
# PendingDeprecationWarning, stacklevel=2) | ||
warnings.warn( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i had massive issue about one part of those warnings being triggered from pytest internals by accident which is why i deffered it on the old warnings system
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
catch_warnings
to the rescue. 😁
It won't show the warnings for this specific access, but I guess that's OK because the whole class will be kicked out anyway. 👍
src/_pytest/python.py
Outdated
@@ -800,7 +800,7 @@ def collect(self): | |||
"%r generated tests with non-unique name %r" % (self, name) | |||
) | |||
seen[name] = True | |||
values.append(self.Function(name, self, args=args, callobj=call)) | |||
values.append(Function(name, self, args=args, callobj=call)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a breaking change, not doing it is why i had massive trouble with the warnings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move the actual warning texts/instances to the deprecation modules
also we should take a look at creating a minimal tool for creating warnings that will be formatted at display time
Argh I missed that point. Definitely, will do.
Can you please clarify what you mean here? |
we do have more and more warnings where we format in code - its not nice to use them in code as one has to import the message and still pick the correct category instead of |
I suppose we can have Could be simply: class PytestWarning:
def format(self, *args, **kwargs):
return type(self)(self.args[0].format(*args, **kwargs))
FUNCARG_PREFIX = RemovedInPytest4Warning(
'{name}: declaring fixtures using "pytest_funcarg__" prefix is deprecated') Usage: warn(FUNCARG_PREFIX.format(name=f.name)) |
instead i would like to see a @attr.s
class UnformattedWarning(object):
category = attr.ib()
template = attr.ib()
def format(self, **kw):
return self.category(self.template.format(**kw))
FUNCARG_PREFIX = UnformattedWarning(
RemovedInPytest4Warning,
'{name}: declaring fixtures using "pytest_funcarg__" prefix is deprecated',
) that way we have a much better indicator of missuse |
Oh great idea. I will implement it then. 👍 |
Implemented @RonnyPfannschmidt! The result is much better, thanks for the suggestion. I also found one warning which was using the wrong category, I fixed and noted in the CHANGELOG. 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fabulous work, thanks
This issues proper warnings for:
cached_setup
This fixes #3616. It also supports a new type of changelog entry, "deprecations", so we can show
Deprecations
separated fromRemovals
.@RonnyPfannschmidt are other features you remember of that we should issue deprecation warnings? Would love to add to them as well.
Also it follows that we should not merge #3898, as now we have a proper way to turn the warnings into errors in pytest-4.0, to effectively remove them in 4.1. 👍