Skip to content

Review backward compatibility policy #3983

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.

Sign up for GitHub

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

Merged
RonnyPfannschmidt merged 3 commits into from
Sep 16, 2018
Merged

Review backward compatibility policy #3983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fa78da3
Update backward compatibility policy with new practices
nicoddemus Sep 14, 2018
cbb41f1
Ignore Sphinx's .doctrees folder
nicoddemus Sep 14, 2018
a4dd6ee
Fix linting
nicoddemus Sep 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ src/_pytest/_version.py
.eggs/

doc/*/_build
doc/*/.doctrees
build/
dist/
*.egg-info
Expand Down
8 changes: 4 additions & 4 deletions doc/en/backwards-compatibility.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ Keeping backwards compatibility has a very high priority in the pytest project.

With the pytest 3.0 release we introduced a clear communication scheme for when we will actually remove the old busted joint and politely ask you to use the new hotness instead, while giving you enough time to adjust your tests or raise concerns if there are valid reasons to keep deprecated functionality around.

To communicate changes we are already issuing deprecation warnings, but they are not displayed by default. In pytest 3.0 we changed the default setting so that pytest deprecation warnings are displayed if not explicitly silenced (with ``--disable-pytest-warnings``).
To communicate changes we issue deprecation warnings using a custom warning hierarchy (see :ref:`internal-warnings`). These warnings may be suppressed using the standard means: ``-W`` command-line flag or ``filterwarnings`` ini options (see :ref:`warnings`), but we suggest to use these sparingly and temporarily, and heed the warnings when possible.
Copy link
Member

@obestwalter obestwalter Sep 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much more informative like that, even with a touch of philosophy about how to deal with warnings.

Just a question about --disable-pytest-warnings is this planned to be phased out then and be replaced with the standard means? I just ran a test with newest pytest using that flag and did at least not get a deprecation warning for it. If we don't need this anymore all the better - the amount of command line options for pytest is staggeringly vast, so a bit of pruning would not hurt :)

Copy link
Member Author

@nicoddemus nicoddemus Sep 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remove the mention about --disable-pytest-warnings because when this was written, it only suppressed pytest's own internal warnings. The standard warnings plugin was even introduced yet at the time.

Now --disable-pytest-warnings is an alias to --disable-warnings, which disables the entire warning summary (internal and external warnings). I thought I would remove it from here as it no longer is deprecation-specific, and mentioning the warning filters seems more productive as it is a more flexible and generic mechanism.

Copy link
Member

@obestwalter obestwalter Sep 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes perfect sense. So this alias could then be deprecated, I guess :)


We will only remove deprecated functionality in major releases (e.g. if we deprecate something in 3.0 we will remove it in 4.0), and keep it around for at least two minor releases (e.g. if we deprecate something in 3.9 and 4.0 is the next release, we will not remove it in 4.0 but in 5.0).
We will only start the removal of deprecated functionality in major releases (e.g. if we deprecate something in 3.0 we will start to remove it in 4.0), and keep it around for at least two minor releases (e.g. if we deprecate something in 3.9 and 4.0 is the next release, we start to remove it in 5.0, not in 4.0).
Copy link
Member Author

@nicoddemus nicoddemus Sep 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and keep it around for at least two minor releases

@RonnyPfannschmidt because of this requirement, I think we should after 4.0 use a different scheme than we are using now with RemovedInPytest4Warning.

When we deprecate something we don't know exactly when the next major release will be. This makes it hard to track which of the functionalities marked with RemovedInPytest4Warning can be removed when pytest 4.0 comes around. For example, suppose we release 3.9 and then 4.0. We need to track mentally what has been deprecated in 3.9, because they can't really be removed in 4.0. Worse, they will carry the RemovedInPytest4Warning name with them until the 5.0 release, because we explicitly can only remove things in major releases. Because this, we can't deprecate anything else from now on, because this will lock us into getting 4.0 out if we want to avoid slipping RemovedInPytest4Warning into the 4.X series until 5.0.

To avoid this problem, here's what I propose:

We add a version parameter to PytestDeprecationWarning. When we deprecate something, we always know what the next minor release will be (for example at the moment we know the next release is 3.9). When a new major release happens, we just need to go over the deprecation warning instances and see which have been around for more than 2 minor releases, those are the ones we should turn into errors and remove later.

The more we refactor the internal code and improve the API, the more I see getting a very tight and consistent deprecation process is important to avoid disrupting our users.

Please let me know your thoughts. 😁

Copy link
Member

@obestwalter obestwalter Sep 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid this problem, here's what I propose:

We add a version parameter to PytestDeprecationWarning. When we deprecate something, we always know what the next minor release will be (for example at the moment we know the next release is 3.9). When a new major release happens, we just need to go over the deprecation warning instances and see which have been around for more than 2 minor releases, those are the ones we should turn into errors and remove later.

Very good - I think that would make it much clearer.

One suggestion: how about naming the parameter in a clearer way, that even the casual source code reader knows what it is about. Something like deprecated_since[_version]?

If the docstring for the deprecation would also contain the "rules" then, this would be clear as day - e.g.

class PytestDeprecationWarning(PytestWarning, DeprecationWarning):
    """
    [Same stuff as before]

    Removals only happen in major releases, after a deprecation warning has been emitted for at least two minor versions.
    """

Don't know if this is the best way to phrase it, but I hope you get my drift.

Copy link
Member Author

@nicoddemus nicoddemus Sep 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All around good suggestions. 👍


When the deprecation expires (e.g. 4.0 is released), we won't remove the deprecated functionality immediately, but will use the standard warning filters to turn them into **errors** by default. This approach makes it explicit that removal is imminent, and still gives you time to turn the deprecated feature into a warning instead of an error so it can be dealt with in your own time. In the next minor release (e.g. 4.1), the feature will be effectively removed.


Deprecation Roadmap
-------------------

We track deprecation and removal of features using milestones and the `deprecation <https://github.com/pytest-dev/pytest/issues?q=label%3A%22type%3A+deprecation%22>`_ and `removal <https://github.com/pytest-dev/pytest/labels/type%3A%20removal>`_ labels on GitHub.

Following our deprecation policy, after starting issuing deprecation warnings we keep features for *at least* two minor versions before considering removal.
8 changes: 5 additions & 3 deletions doc/en/warnings.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,9 @@ You can also use it as a contextmanager::
myobject.deprecated_method()



.. _internal-warnings:

Internal pytest warnings
------------------------

Expand Down Expand Up @@ -363,9 +366,8 @@ defines an ``__init__`` constructor, as this prevents the class from being insta

These warnings might be filtered using the same builtin mechanisms used to filter other types of warnings.

Following our :ref:`backwards-compatibility`, deprecated features will be kept *at least* two minor releases. After that,
they will changed so they by default raise errors instead of just warnings, so users can adapt to it on their own time
if not having done so until now. In a later release the deprecated feature will be removed completely.
Please read our :ref:`backwards-compatibility` to learn how we proceed about deprecating and eventually removing
features.

The following warning types ares used by pytest and are part of the public API:

Expand Down