Skip to content

Commit

Permalink
Merge pull request #647 from DanielYang59/fix-deprecation
Browse files Browse the repository at this point in the history
Fix `dev.deprecated` and refine its unit tests
  • Loading branch information
shyuep committed Apr 11, 2024
2 parents 3a3bdb5 + 1483615 commit 37d1cae
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 49 deletions.
8 changes: 4 additions & 4 deletions monty/dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ def _is_in_owner_repo() -> bool:
# Only raise warning in code owner's repo CI
if (
_deadline is not None
and os.getenv("CI")
and os.getenv("CI") is not None
and datetime.now() > _deadline
and _is_in_owner_repo()
):
raise DeprecationWarning(
"This function should have been removed on {deadline:%Y-%m-%d}."
f"This function should have been removed on {_deadline:%Y-%m-%d}."
)

def craft_message(
Expand All @@ -91,7 +91,7 @@ def craft_message(
msg = f"{old.__name__} is deprecated"

if deadline is not None:
msg += f", and will be removed on {deadline:%Y-%m-%d}\n"
msg += f", and will be removed on {_deadline:%Y-%m-%d}\n"

if replacement is not None:
if isinstance(replacement, property):
Expand All @@ -117,7 +117,7 @@ def wrapped(*args, **kwargs):
# Convert deadline to datetime type
_deadline = datetime(*deadline) if deadline is not None else None

# Raise a CI warning after removal deadline
# Raise CI warning after removal deadline
raise_deadline_warning()

return deprecated_decorator
Expand Down
88 changes: 53 additions & 35 deletions tests/test_dev.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import unittest
import warnings
import datetime
from unittest.mock import patch

import pytest
from monty.dev import deprecated, install_excepthook, requires
Expand Down Expand Up @@ -88,55 +89,72 @@ def classmethod_b(cls):
with pytest.warns(DeprecationWarning):
assert TestClass_deprecationwarning().classmethod_b() == "b"

def test_deprecated_deadline(self):
@deprecated(deadline=(2000, 1, 1))
def func_old():
pass
def test_deprecated_deadline(self, monkeypatch):
with pytest.raises(DeprecationWarning):
with patch("subprocess.run") as mock_run:
monkeypatch.setenv("CI", "true") # mock CI env

with warnings.catch_warnings(record=True) as warn_msgs:
# Trigger a warning.
func_old()
# Verify message
assert "will be removed on 2000-01-01" in str(warn_msgs[0].message)
# Mock "GITHUB_REPOSITORY"
monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO")
mock_run.return_value.stdout.decode.return_value = (
"git@github.com:TESTOWNER/TESTREPO.git"
)

@deprecated(deadline=(2000, 1, 1))
def func_old():
pass

@pytest.fixture()
def test_deprecated_deadline_no_warn(self, monkeypatch):
"""Test cases where no warning should be raised."""

@deprecated(deadline=(2000, 1, 1))
def func_old():
pass

# No warn case 1: date before deadline
with warnings.catch_warnings(record=True) as warn_msgs:
monkeypatch.setattr(
datetime, "datetime", lambda: datetime.datetime(1999, 1, 1)
)
func_old()
with warnings.catch_warnings():
with patch("subprocess.run") as mock_run:
monkeypatch.setenv("CI", "true") # mock CI env

for warning in warn_msgs:
assert "This function should have been removed on" not in str(
warning.message
# Mock date to 1999-01-01
monkeypatch.setattr(
datetime.datetime, "now", datetime.datetime(1999, 1, 1)
)

# No warn case 2: not in CI env
with warnings.catch_warnings(record=True) as warn_msgs:
monkeypatch.delenv("CI", raising=False)
func_old()
# Mock "GITHUB_REPOSITORY"
monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO")
mock_run.return_value.stdout.decode.return_value = (
"git@github.com:TESTOWNER/TESTREPO.git"
)

@deprecated(deadline=(2000, 1, 1))
def func_old():
pass

for warning in warn_msgs:
assert "This function should have been removed on" not in str(
warning.message
monkeypatch.undo()

# No warn case 2: not in CI env
with warnings.catch_warnings():
with patch("subprocess.run") as mock_run:
monkeypatch.delenv("CI", raising=False)

# Mock "GITHUB_REPOSITORY"
monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO")
mock_run.return_value.stdout.decode.return_value = (
"git@github.com:TESTOWNER/TESTREPO.git"
)

@deprecated(deadline=(2000, 1, 1))
def func_old_1():
pass

monkeypatch.undo()

# No warn case 3: not in code owner repo
with warnings.catch_warnings(record=True) as warn_msgs:
monkeypatch.setenv("GITHUB_REPOSITORY", "NONE/NONE")
func_old()
with warnings.catch_warnings():
monkeypatch.setenv("CI", "true")
monkeypatch.delenv("GITHUB_REPOSITORY", raising=False)

for warning in warn_msgs:
assert "This function should have been removed on" not in str(
warning.message
)
@deprecated(deadline=(2000, 1, 1))
def func_old_2():
pass

def test_requires(self):
try:
Expand Down
5 changes: 0 additions & 5 deletions tests/test_tempfile.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import os
import shutil
import unittest

from monty.tempfile import ScratchDir

Expand Down Expand Up @@ -142,7 +141,3 @@ def test_bad_root(self):
def teardown_method(self):
os.chdir(self.cwd)
shutil.rmtree(self.scratch_root)


if __name__ == "__main__":
unittest.main()
5 changes: 0 additions & 5 deletions tests/test_termcolor.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import os
import sys
import unittest

from monty.termcolor import (
cprint,
Expand Down Expand Up @@ -75,7 +74,3 @@ def test_remove_non_ascii(self):
def test_stream_has_colors(self):
# TODO: not a real test. Need to do a proper test.
stream_has_colours(sys.stdout)


if __name__ == "__main__":
unittest.main()

0 comments on commit 37d1cae

Please sign in to comment.