From 18e87c98319faee46f2becd32cf6518c9efaceaa Mon Sep 17 00:00:00 2001 From: Lesnek Date: Sun, 2 Jul 2023 14:15:50 +0200 Subject: [PATCH 1/9] test(warnings-recorder): Add non working subclass behaviour of pop --- testing/test_recwarn.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/testing/test_recwarn.py b/testing/test_recwarn.py index 16b8d54438f..b15ef2cc111 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -37,6 +37,26 @@ def test_recwarn_captures_deprecation_warning(recwarn: WarningsRecorder) -> None assert recwarn.pop(DeprecationWarning) +class TestSubclassWarningPop: + class ParentWarning(Warning): + pass + + class ChildWarning(ParentWarning): + pass + + def raise_warnings(self): + warnings.warn("Warning Child", self.ChildWarning) + warnings.warn("Warning Parent", self.ParentWarning) + + def test_pop(self): + with pytest.warns((self.ParentWarning, self.ChildWarning)) as record: + self.raise_warnings() + + assert len(record) == 2 + _warn = record.pop(self.ParentWarning) + assert _warn.category is self.ParentWarning + + class TestWarningsRecorderChecker: def test_recording(self) -> None: rec = WarningsRecorder(_ispytest=True) From 8ac3c645fa4a87afb30295d28e8d6fb67166bd01 Mon Sep 17 00:00:00 2001 From: Lesnek Date: Sun, 2 Jul 2023 14:17:06 +0200 Subject: [PATCH 2/9] fix(warnings-recorder): Match also subclass of warning in pop --- src/_pytest/recwarn.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/_pytest/recwarn.py b/src/_pytest/recwarn.py index ff8e7082050..ba5a3311ca1 100644 --- a/src/_pytest/recwarn.py +++ b/src/_pytest/recwarn.py @@ -206,12 +206,18 @@ def __len__(self) -> int: return len(self._list) def pop(self, cls: Type[Warning] = Warning) -> "warnings.WarningMessage": - """Pop the first recorded warning, raise exception if not exists.""" + """Pop the first recorded warning (or subclass of warning), raise exception if not exists.""" + matches = [] for i, w in enumerate(self._list): - if issubclass(w.category, cls): + if w.category == cls: return self._list.pop(i) - __tracebackhide__ = True - raise AssertionError(f"{cls!r} not found in warning list") + if issubclass(w.category, cls): + matches.append((i, w)) + if not matches: + __tracebackhide__ = True + raise AssertionError(f"{cls!r} not found in warning list") + (idx, best), *rest = matches + return self._list.pop(idx) def clear(self) -> None: """Clear the list of recorded warnings.""" From 2706271f66ea1b8d3e931e6e945bb4dbbd758fea Mon Sep 17 00:00:00 2001 From: Lesnek Date: Sun, 2 Jul 2023 15:03:15 +0200 Subject: [PATCH 3/9] test(warnings-recorder): Add another warning --- testing/test_recwarn.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/testing/test_recwarn.py b/testing/test_recwarn.py index b15ef2cc111..72dc34b09a5 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -44,7 +44,11 @@ class ParentWarning(Warning): class ChildWarning(ParentWarning): pass + class RandomWarning(Warning): + pass + def raise_warnings(self): + warnings.warn("Warning Random", self.RandomWarning) warnings.warn("Warning Child", self.ChildWarning) warnings.warn("Warning Parent", self.ParentWarning) From 7b7bd304aafcf9287eafc73bb332e4081f8fdf58 Mon Sep 17 00:00:00 2001 From: Lesnek Date: Sun, 2 Jul 2023 15:16:14 +0200 Subject: [PATCH 4/9] fix(warnings-recorder): Add handling of rest --- src/_pytest/recwarn.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/_pytest/recwarn.py b/src/_pytest/recwarn.py index ba5a3311ca1..a60f346713d 100644 --- a/src/_pytest/recwarn.py +++ b/src/_pytest/recwarn.py @@ -217,6 +217,11 @@ def pop(self, cls: Type[Warning] = Warning) -> "warnings.WarningMessage": __tracebackhide__ = True raise AssertionError(f"{cls!r} not found in warning list") (idx, best), *rest = matches + for i, w in rest: + if issubclass(w.category, best.category) and not issubclass( + best.category, w.category + ): + idx, best = i, w return self._list.pop(idx) def clear(self) -> None: From 3d0dedb5ec7ac56c5f3e8bdbf5b47d7276096151 Mon Sep 17 00:00:00 2001 From: Lesnek Date: Sun, 2 Jul 2023 15:35:04 +0200 Subject: [PATCH 5/9] test(warnings-recorder): Add attribute error test --- testing/test_recwarn.py | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/testing/test_recwarn.py b/testing/test_recwarn.py index 72dc34b09a5..6c2dc4660b7 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -1,5 +1,7 @@ import warnings +from typing import List from typing import Optional +from typing import Type import pytest from _pytest.pytester import Pytester @@ -44,22 +46,37 @@ class ParentWarning(Warning): class ChildWarning(ParentWarning): pass - class RandomWarning(Warning): + class ChildOfChildWarning(ChildWarning): pass - def raise_warnings(self): - warnings.warn("Warning Random", self.RandomWarning) - warnings.warn("Warning Child", self.ChildWarning) - warnings.warn("Warning Parent", self.ParentWarning) + @staticmethod + def raise_warnings_from_list(_warnings: List[Type[Warning]]): + for warn in _warnings: + warnings.warn(f"Warning {warn().__repr__()}", warn) def test_pop(self): with pytest.warns((self.ParentWarning, self.ChildWarning)) as record: - self.raise_warnings() + self.raise_warnings_from_list( + [self.ChildWarning, self.ParentWarning, self.ChildOfChildWarning] + ) - assert len(record) == 2 + assert len(record) == 3 _warn = record.pop(self.ParentWarning) assert _warn.category is self.ParentWarning + def test_pop_raises(self): + with pytest.raises(AssertionError): + with pytest.warns(self.ParentWarning) as record: + self.raise_warnings_from_list([self.ParentWarning]) + record.pop(self.ChildOfChildWarning) + + def test_pop_most_recent(self): + with pytest.warns(self.ParentWarning) as record: + self.raise_warnings_from_list([self.ChildWarning, self.ChildOfChildWarning]) + + _warn = record.pop(self.ParentWarning) + assert _warn.category is self.ChildOfChildWarning + class TestWarningsRecorderChecker: def test_recording(self) -> None: From 4517af1e28083068cfe6d5d441f151dfd27a98b4 Mon Sep 17 00:00:00 2001 From: Lesnek Date: Sun, 2 Jul 2023 15:35:04 +0200 Subject: [PATCH 6/9] test(warnings-recorder): Add attribute error test --- AUTHORS | 1 + changelog/10701.bugfix.rst | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog/10701.bugfix.rst diff --git a/AUTHORS b/AUTHORS index 13ea9469799..062f565c9ce 100644 --- a/AUTHORS +++ b/AUTHORS @@ -263,6 +263,7 @@ Mickey Pashov Mihai Capotă Mike Hoyle (hoylemd) Mike Lundy +Milan Lesnek Miro Hrončok Nathaniel Compton Nathaniel Waisbrot diff --git a/changelog/10701.bugfix.rst b/changelog/10701.bugfix.rst new file mode 100644 index 00000000000..d02ddacab8c --- /dev/null +++ b/changelog/10701.bugfix.rst @@ -0,0 +1 @@ +``pytest.WarningsRecorder.pop`` now also pop most recent subclass of warning. From 6badb6f01e355633eda251d037b48e39a4ed89dc Mon Sep 17 00:00:00 2001 From: Milan Lesnek Date: Tue, 4 Jul 2023 08:59:58 +0200 Subject: [PATCH 7/9] Apply suggestions from code review chore(changelog): describe better the fix Co-authored-by: Zac Hatfield-Dodds --- changelog/10701.bugfix.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/changelog/10701.bugfix.rst b/changelog/10701.bugfix.rst index d02ddacab8c..f33fa7fb28b 100644 --- a/changelog/10701.bugfix.rst +++ b/changelog/10701.bugfix.rst @@ -1 +1,2 @@ -``pytest.WarningsRecorder.pop`` now also pop most recent subclass of warning. +:meth:`pytest.WarningsRecorder.pop` will return the most-closely-matched warning in the list, +rather than the first warning which is an instance of the requested type. From c4876c710628b8bd244ae255aef529c369f8dd63 Mon Sep 17 00:00:00 2001 From: Lesnek Date: Tue, 4 Jul 2023 10:20:50 +0200 Subject: [PATCH 8/9] chore(CR): Add changes from code review --- pyproject.toml | 2 +- src/_pytest/recwarn.py | 31 +++++++++++++++++-------------- testing/test_recwarn.py | 6 ++++-- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index a4139a5c051..8432d5d2bbd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ build-backend = "setuptools.build_meta" write_to = "src/_pytest/_version.py" [tool.pytest.ini_options] -minversion = "2.0" +#minversion = "2.0" addopts = "-rfEX -p pytester --strict-markers" python_files = ["test_*.py", "*_test.py", "testing/python/*.py"] python_classes = ["Test", "Acceptance"] diff --git a/src/_pytest/recwarn.py b/src/_pytest/recwarn.py index a60f346713d..9d8816556e4 100644 --- a/src/_pytest/recwarn.py +++ b/src/_pytest/recwarn.py @@ -206,23 +206,26 @@ def __len__(self) -> int: return len(self._list) def pop(self, cls: Type[Warning] = Warning) -> "warnings.WarningMessage": - """Pop the first recorded warning (or subclass of warning), raise exception if not exists.""" - matches = [] + """Pop the first recorded warning which is an instance of ``cls``. + + But not an instance of a child class of any other match. + Raises ``AssertionError`` if there is no match. + + """ + + best_idx = None for i, w in enumerate(self._list): if w.category == cls: - return self._list.pop(i) - if issubclass(w.category, cls): - matches.append((i, w)) - if not matches: - __tracebackhide__ = True - raise AssertionError(f"{cls!r} not found in warning list") - (idx, best), *rest = matches - for i, w in rest: - if issubclass(w.category, best.category) and not issubclass( - best.category, w.category + return self._list.pop(i) # exact match, stop looking + if issubclass(w.category, cls) and ( + best_idx is None + or not issubclass(w.category, self._list[best_idx].category) # type: ignore[unreachable] ): - idx, best = i, w - return self._list.pop(idx) + best_idx = i + if best_idx is not None: + return self._list.pop(best_idx) + __tracebackhide__ = True + raise AssertionError(f"{cls!r} not found in warning list") def clear(self) -> None: """Clear the list of recorded warnings.""" diff --git a/testing/test_recwarn.py b/testing/test_recwarn.py index 6c2dc4660b7..3a35c255eaf 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -72,10 +72,12 @@ def test_pop_raises(self): def test_pop_most_recent(self): with pytest.warns(self.ParentWarning) as record: - self.raise_warnings_from_list([self.ChildWarning, self.ChildOfChildWarning]) + self.raise_warnings_from_list( + [self.ChildOfChildWarning, self.ChildWarning, self.ChildOfChildWarning] + ) _warn = record.pop(self.ParentWarning) - assert _warn.category is self.ChildOfChildWarning + assert _warn.category is self.ChildWarning class TestWarningsRecorderChecker: From 7775e494b155176b2c7083bce1e25b236401b638 Mon Sep 17 00:00:00 2001 From: Zac Hatfield-Dodds Date: Tue, 4 Jul 2023 10:00:29 -0700 Subject: [PATCH 9/9] Further tweaks from code review --- pyproject.toml | 2 +- src/_pytest/recwarn.py | 11 ++++------- testing/test_recwarn.py | 6 +++--- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 8432d5d2bbd..a4139a5c051 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ build-backend = "setuptools.build_meta" write_to = "src/_pytest/_version.py" [tool.pytest.ini_options] -#minversion = "2.0" +minversion = "2.0" addopts = "-rfEX -p pytester --strict-markers" python_files = ["test_*.py", "*_test.py", "testing/python/*.py"] python_classes = ["Test", "Acceptance"] diff --git a/src/_pytest/recwarn.py b/src/_pytest/recwarn.py index 9d8816556e4..5484d6f3b33 100644 --- a/src/_pytest/recwarn.py +++ b/src/_pytest/recwarn.py @@ -206,20 +206,17 @@ def __len__(self) -> int: return len(self._list) def pop(self, cls: Type[Warning] = Warning) -> "warnings.WarningMessage": - """Pop the first recorded warning which is an instance of ``cls``. - - But not an instance of a child class of any other match. + """Pop the first recorded warning which is an instance of ``cls``, + but not an instance of a child class of any other match. Raises ``AssertionError`` if there is no match. - """ - - best_idx = None + best_idx: Optional[int] = None for i, w in enumerate(self._list): if w.category == cls: return self._list.pop(i) # exact match, stop looking if issubclass(w.category, cls) and ( best_idx is None - or not issubclass(w.category, self._list[best_idx].category) # type: ignore[unreachable] + or not issubclass(w.category, self._list[best_idx].category) ): best_idx = i if best_idx is not None: diff --git a/testing/test_recwarn.py b/testing/test_recwarn.py index 3a35c255eaf..8b70c8afffa 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -54,7 +54,7 @@ def raise_warnings_from_list(_warnings: List[Type[Warning]]): for warn in _warnings: warnings.warn(f"Warning {warn().__repr__()}", warn) - def test_pop(self): + def test_pop_finds_exact_match(self): with pytest.warns((self.ParentWarning, self.ChildWarning)) as record: self.raise_warnings_from_list( [self.ChildWarning, self.ParentWarning, self.ChildOfChildWarning] @@ -64,13 +64,13 @@ def test_pop(self): _warn = record.pop(self.ParentWarning) assert _warn.category is self.ParentWarning - def test_pop_raises(self): + def test_pop_raises_if_no_match(self): with pytest.raises(AssertionError): with pytest.warns(self.ParentWarning) as record: self.raise_warnings_from_list([self.ParentWarning]) record.pop(self.ChildOfChildWarning) - def test_pop_most_recent(self): + def test_pop_finds_best_inexact_match(self): with pytest.warns(self.ParentWarning) as record: self.raise_warnings_from_list( [self.ChildOfChildWarning, self.ChildWarning, self.ChildOfChildWarning]