Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit cf2576e

Browse filesBrowse files
EliahKaganEliahKagan
committed
Make/use test.deprecation.lib; abandon idea to filter by module
This creates a module test.deprecation.lib in the test suite for a couple general helpers used in deprecation tests, one of which is now used in two of the test modules and the other of which happens only to be used in one but is concepually related to the first. The assert_no_deprecation_warning context manager function fixes two different flawed approaches to that, which were in use earlier: - In test_basic, only DeprecationWarning and not the less significant PendingDeprecationWarning had been covere, which it should, at least for symmetry, since pytest.deprecated_call() treats it like DeprecationWarning. There was also a comment expressing a plan to make it filter only for warnings originating from GitPython, which was a flawed idea, as detailed below. - In test_cmd_git, the flawed idea of attempting to filter only for warnings originating from GitPython was implemented. The problem with this is that it is heavily affected by stacklevel and its interaction with the pattern of calls through which the warning arises: warnings could actually emanate from code in GitPython's git module, but be registered as having come from test code, a callback in gitdb, smmap, or the standard library, or even the pytest test runner. Some of these are more likely than others, but all are possible especially considering the possibility of a bug in the value passed to warning.warn as stacklevel. (It may be valuable for such bugs to cause tests to fail, but they should not cause otherwise failing tests to wrongly pass.) It is probably feasible to implement such filtering successfully, but I don't think it's worthwhile for the small reduction in likelihood of failing later on an unrealted DeprecationWarning from somewhere else, especially considering that GitPython's main dependencies are so minimal.
1 parent 7cd3aa9 commit cf2576e
Copy full SHA for cf2576e

File tree

3 files changed

+40
-37
lines changed
Filter options

3 files changed

+40
-37
lines changed

‎test/deprecation/lib.py

Copy file name to clipboard
+27Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# This module is part of GitPython and is released under the
2+
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
3+
4+
"""Support library for deprecation tests."""
5+
6+
__all__ = ["assert_no_deprecation_warning", "suppress_deprecation_warning"]
7+
8+
import contextlib
9+
import warnings
10+
11+
from typing import Generator
12+
13+
14+
@contextlib.contextmanager
15+
def assert_no_deprecation_warning() -> Generator[None, None, None]:
16+
"""Context manager to assert that code does not issue any deprecation warnings."""
17+
with warnings.catch_warnings():
18+
warnings.simplefilter("error", DeprecationWarning)
19+
warnings.simplefilter("error", PendingDeprecationWarning)
20+
yield
21+
22+
23+
@contextlib.contextmanager
24+
def suppress_deprecation_warning() -> Generator[None, None, None]:
25+
with warnings.catch_warnings():
26+
warnings.simplefilter("ignore", DeprecationWarning)
27+
yield

‎test/deprecation/test_basic.py

Copy file name to clipboardExpand all lines: test/deprecation/test_basic.py
+8-18Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,15 @@
1515
It is inapplicable to the deprecations whose warnings are tested in this module.
1616
"""
1717

18-
import contextlib
19-
import warnings
20-
2118
import pytest
2219

2320
from git.diff import NULL_TREE
2421
from git.objects.util import Traversable
2522
from git.repo import Repo
2623
from git.util import Iterable as _Iterable, IterableObj
2724

25+
from .lib import assert_no_deprecation_warning
26+
2827
# typing -----------------------------------------------------------------
2928

3029
from typing import Generator, TYPE_CHECKING
@@ -38,15 +37,6 @@
3837
# ------------------------------------------------------------------------
3938

4039

41-
@contextlib.contextmanager
42-
def _assert_no_deprecation_warning() -> Generator[None, None, None]:
43-
"""Context manager to assert that code does not issue any deprecation warnings."""
44-
with warnings.catch_warnings():
45-
# FIXME: Refine this to filter for deprecation warnings from GitPython.
46-
warnings.simplefilter("error", DeprecationWarning)
47-
yield
48-
49-
5040
@pytest.fixture
5141
def commit(tmp_path: "Path") -> Generator["Commit", None, None]:
5242
"""Fixture to supply a one-commit repo's commit, enough for deprecation tests."""
@@ -72,7 +62,7 @@ def test_diff_renamed_warns(diff: "Diff") -> None:
7262

7363
def test_diff_renamed_file_does_not_warn(diff: "Diff") -> None:
7464
"""The preferred Diff.renamed_file property issues no deprecation warning."""
75-
with _assert_no_deprecation_warning():
65+
with assert_no_deprecation_warning():
7666
diff.renamed_file
7767

7868

@@ -84,13 +74,13 @@ def test_commit_trailers_warns(commit: "Commit") -> None:
8474

8575
def test_commit_trailers_list_does_not_warn(commit: "Commit") -> None:
8676
"""The nondeprecated Commit.trailers_list property issues no deprecation warning."""
87-
with _assert_no_deprecation_warning():
77+
with assert_no_deprecation_warning():
8878
commit.trailers_list
8979

9080

9181
def test_commit_trailers_dict_does_not_warn(commit: "Commit") -> None:
9282
"""The nondeprecated Commit.trailers_dict property issues no deprecation warning."""
93-
with _assert_no_deprecation_warning():
83+
with assert_no_deprecation_warning():
9484
commit.trailers_dict
9585

9686

@@ -102,7 +92,7 @@ def test_traverse_list_traverse_in_base_class_warns(commit: "Commit") -> None:
10292

10393
def test_traversable_list_traverse_override_does_not_warn(commit: "Commit") -> None:
10494
"""Calling list_traverse on concrete subclasses is not deprecated, does not warn."""
105-
with _assert_no_deprecation_warning():
95+
with assert_no_deprecation_warning():
10696
commit.list_traverse()
10797

10898

@@ -114,7 +104,7 @@ def test_traverse_traverse_in_base_class_warns(commit: "Commit") -> None:
114104

115105
def test_traverse_traverse_override_does_not_warn(commit: "Commit") -> None:
116106
"""Calling traverse on concrete subclasses is not deprecated, does not warn."""
117-
with _assert_no_deprecation_warning():
107+
with assert_no_deprecation_warning():
118108
commit.traverse()
119109

120110

@@ -128,7 +118,7 @@ class Derived(_Iterable):
128118

129119
def test_iterable_obj_inheriting_does_not_warn() -> None:
130120
"""Subclassing git.util.IterableObj is not deprecated, does not warn."""
131-
with _assert_no_deprecation_warning():
121+
with assert_no_deprecation_warning():
132122

133123
class Derived(IterableObj):
134124
pass

‎test/deprecation/test_cmd_git.py

Copy file name to clipboardExpand all lines: test/deprecation/test_cmd_git.py
+5-19Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,10 @@
5656
metaclass marginal enough, to justify introducing a metaclass to issue the warnings.
5757
"""
5858

59-
import contextlib
6059
import logging
6160
import sys
6261
from typing import Generator
6362
import unittest.mock
64-
import warnings
6563

6664
if sys.version_info >= (3, 11):
6765
from typing import assert_type
@@ -73,6 +71,8 @@
7371

7472
from git.cmd import Git
7573

74+
from .lib import assert_no_deprecation_warning, suppress_deprecation_warning
75+
7676
_USE_SHELL_DEPRECATED_FRAGMENT = "Git.USE_SHELL is deprecated"
7777
"""Text contained in all USE_SHELL deprecation warnings, and starting most of them."""
7878

@@ -82,13 +82,6 @@
8282
_logger = logging.getLogger(__name__)
8383

8484

85-
@contextlib.contextmanager
86-
def _suppress_deprecation_warning() -> Generator[None, None, None]:
87-
with warnings.catch_warnings():
88-
warnings.simplefilter("ignore", DeprecationWarning)
89-
yield
90-
91-
9285
@pytest.fixture
9386
def restore_use_shell_state() -> Generator[None, None, None]:
9487
"""Fixture to attempt to restore state associated with the USE_SHELL attribute.
@@ -129,15 +122,15 @@ def restore_use_shell_state() -> Generator[None, None, None]:
129122
# Try to save the original public value. Rather than attempt to restore a state
130123
# where the attribute is not set, if we cannot do this we allow AttributeError
131124
# to propagate out of the fixture, erroring the test case before its code runs.
132-
with _suppress_deprecation_warning():
125+
with suppress_deprecation_warning():
133126
old_public_value = Git.USE_SHELL
134127

135128
# This doesn't have its own try-finally because pytest catches exceptions raised
136129
# during the yield. (The outer try-finally catches exceptions in this fixture.)
137130
yield
138131

139132
# Try to restore the original public value.
140-
with _suppress_deprecation_warning():
133+
with suppress_deprecation_warning():
141134
Git.USE_SHELL = old_public_value
142135
finally:
143136
# Try to restore the original private state.
@@ -323,14 +316,7 @@ def test_execute_without_shell_arg_does_not_warn() -> None:
323316
USE_SHELL is to be used, the way Git.execute itself accesses USE_SHELL does not
324317
issue a deprecation warning.
325318
"""
326-
with warnings.catch_warnings():
327-
for category in DeprecationWarning, PendingDeprecationWarning:
328-
warnings.filterwarnings(
329-
action="error",
330-
category=category,
331-
module=r"git(?:\.|$)",
332-
)
333-
319+
with assert_no_deprecation_warning():
334320
Git().version()
335321

336322

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.