Skip to content
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

Improve deprecation warnings #6549

Merged
merged 5 commits into from
May 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 news/6549.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve deprecation messages to include the version in which the functionality will be removed.
23 changes: 15 additions & 8 deletions src/pip/_internal/utils/deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,23 @@ def deprecated(reason, replacement, gone_in, issue=None):
"""

# Construct a nice message.
# This is purposely eagerly formatted as we want it to appear as if someone
# typed this entire message out.
message = DEPRECATION_MSG_PREFIX + reason
if replacement is not None:
message += " A possible replacement is {}.".format(replacement)
if issue is not None:
url = "https:/pypa/pip/issues/" + str(issue)
message += " You can find discussion regarding this at {}.".format(url)
# This is eagerly formatted as we want it to get logged as if someone
# typed this entire message out.
sentences = [
(reason, DEPRECATION_MSG_PREFIX + "{}"),
(gone_in, "pip {} will remove support for this functionality."),
(replacement, "A possible replacement is {}."),
(issue, (
"You can find discussion regarding this at "
"https:/pypa/pip/issues/{}."
)),
]
message = " ".join(
template.format(val) for val, template in sentences if val is not None
)

# Raise as an error if it has to be removed.
if gone_in is not None and parse(current_version) >= parse(gone_in):
raise PipDeprecationWarning(message)

warnings.warn(message, category=PipDeprecationWarning, stacklevel=2)
77 changes: 77 additions & 0 deletions tests/unit/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from pip._internal.exceptions import (
HashMismatch, HashMissing, InstallationError,
)
from pip._internal.utils.deprecation import PipDeprecationWarning, deprecated
from pip._internal.utils.encoding import BOMS, auto_decode
from pip._internal.utils.glibc import check_glibc_version
from pip._internal.utils.hashes import Hashes, MissingHashes
Expand Down Expand Up @@ -1068,3 +1069,79 @@ def test_remove_auth_from_url(auth_url, expected_url):
def test_redact_password_from_url(auth_url, expected_url):
url = redact_password_from_url(auth_url)
assert url == expected_url


@pytest.fixture()
def patch_deprecation_check_version():
# We do this, so that the deprecation tests are easier to write.
import pip._internal.utils.deprecation as d
old_version = d.current_version
d.current_version = "1.0"
yield
d.current_version = old_version


@pytest.mark.usefixtures("patch_deprecation_check_version")
@pytest.mark.parametrize("replacement", [None, "a magic 8 ball"])
@pytest.mark.parametrize("gone_in", [None, "2.0"])
@pytest.mark.parametrize("issue", [None, 988])
def test_deprecated_message_contains_information(gone_in, replacement, issue):
with pytest.warns(PipDeprecationWarning) as record:
deprecated(
"Stop doing this!",
replacement=replacement,
gone_in=gone_in,
issue=issue,
)

assert len(record) == 1
message = record[0].message.args[0]

assert "DEPRECATION: Stop doing this!" in message
# Ensure non-None values are mentioned.
for item in [gone_in, replacement, issue]:
if item is not None:
assert str(item) in message


@pytest.mark.usefixtures("patch_deprecation_check_version")
@pytest.mark.parametrize("replacement", [None, "a magic 8 ball"])
@pytest.mark.parametrize("issue", [None, 988])
def test_deprecated_raises_error_if_too_old(replacement, issue):
with pytest.raises(PipDeprecationWarning) as exception:
deprecated(
"Stop doing this!",
gone_in="1.0", # this matches the patched version.
replacement=replacement,
issue=issue,
)

message = exception.value.args[0]

assert "DEPRECATION: Stop doing this!" in message
assert "1.0" in message
# Ensure non-None values are mentioned.
for item in [replacement, issue]:
if item is not None:
assert str(item) in message


@pytest.mark.usefixtures("patch_deprecation_check_version")
def test_deprecated_message_reads_well():
with pytest.raises(PipDeprecationWarning) as exception:
deprecated(
"Stop doing this!",
gone_in="1.0", # this matches the patched version.
replacement="to be nicer",
issue="100000", # I hope we never reach this number.
)

message = exception.value.args[0]

assert message == (
"DEPRECATION: Stop doing this! "
"pip 1.0 will remove support for this functionality. "
"A possible replacement is to be nicer. "
"You can find discussion regarding this at "
"https:/pypa/pip/issues/100000."
)