Replace internal warnings system by standard warnings and use them to make deprecations visible and later enforced

[RonnyPfannschmidt]

in order to support users in finding bad code, we should introduce Warnings subclasses similar to django

for example RemovedInPytest40Warning(DeprecationWarning)

we can then use the normal warnings modules to start and show those warnings, and even error out once we do a release

this enables a few positives

1. control over what deprecation show
2. disconnect breaking a feature from removing its code
   (a warning error can be disabled by users or subsequent releases depending on need)

[nicoddemus]

That's a great idea!

Particularly not a big fan of adding the deprecation name to the class, but it might be a matter of opinion because I can't point out a concrete problem with it at the moment.

One thing I wonder is what to do about the current config.warn function and the pytest_logwarning hook.

Here's config.warn implementation:

    def warn(self, code, message, fslocation=None, nodeid=None):
        """ generate a warning for this test session. """
        self.hook.pytest_logwarning.call_historic(kwargs=dict(
            code=code, message=message,
            fslocation=fslocation, nodeid=nodeid))

So it directly invokes the hook implementations.

Also the hook signature:

@hookspec(historic=True)
def pytest_logwarning(message, code, nodeid, fslocation):
    """ process a warning specified by a message, a code string,
    a nodeid and fslocation (both of which may be None
    if the warning is not tied to a partilar node/location)."""

Does not deal with any warning class at all.

Do you have an idea/proposal on what to do with the config.warn (and Node.warn for that matter) and the hook, and how those new classes will play out with the warnings plugin?

[RonnyPfannschmidt]

my oppinion is that config.warn was a mistake and i'd like to shift torwards using warnings and eventually deprecate it

[RonnyPfannschmidt]

btw, the introduction of the classes itself can be used in minor invisible refactorings, and then we can add the code to make them visible in the features branch as followup

[nicoddemus]

I agree with the general idea of deprecating config.warn and Node.warn and use plain warnings instead, with pytest-specific subclasses.

How about:

3.2

1. Create pytest-specific warnings:

   class PytestWarning(UserWarning):
   
   class PytestUsageWarning(PytestWarning):
       """Warnings about usage, for example "Test" prefixed classes
          with __init__ method.
       """
   
   class PytestDeprecationWarning(PytestWarning, DeprecationWarning):
       """
       For deprecating functionality. The functionality will be removed accordingly to our
       deprecation policy: either major version or major version + at least two minor versions.
       """
   
   

2. Create a new hook, pytest_warning_captured(warning, when, item), with warning being the warning instance, when being either "collect" (for warnings during collection) and "runtest" (in which case item is the running item). We can expand the when parameter later, but those two feels like they should cover a good ground.

3. Update warnings plugin to emit pytest_warning_captured and pytest_logwarning for backward compatibility. Also capture warnings during collection.

4. Update terminal plugin to use the new hook.

5. Show PytestUsageWarning and PytestPendingDeprecation by default.

6. Deprecate config.warn and Node.warn by emitting PytestDeprecationWarning from them.

7. Emit PytestDeprecationWarning if there are implementators for the pytest_logwarning hook.

4.0

1. Remove config.warn, Node.warn and pytest_logwarning.

What do you think?

[RonnyPfannschmidt]

@nicoddemus sounds fabulous - i do wonder about doing something like django and having RemovedInPytest40 warnings

as for the deprecation cycle, i would like to propose turning them into errors in 4.0 and removing the code in 4.1 (so we can easily allow users to reenable the affected bits or backpedal on removing something when a good case is made to keep it another iteration)

[RonnyPfannschmidt]

#2517 introduced this one

[nicoddemus]

Reopening to track adding PytestWarning subclasses and changing config.warn to raise it instead.

[nicoddemus]

After some more thought I'm inclined on keeping config.warn and item.warn as shortcuts that raise an appropriate warning subclass but add useful information by default (for example the item node id and fslocation for item.warn).

[RonnyPfannschmidt]

@nicoddemus that needs a new hook and a new name, the old api is completely missfit and it will be a major mess for any consumer to change the behavior that way

python warnings differ in behaviour wrt encoding location, relation and types - at first glance it seems to unify those apis would be to create a major pain in some way