From 02aa8adae1ccf6b6f758ef798b7eeab697b8c49f Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 11 Feb 2020 13:10:51 +0100 Subject: [PATCH 1/2] cacheprovider: use warnings directly Allows for filtering of PytestCacheWarning. Using `_issue_warning_captured` is not necessary here, and was probably only used because the cacheprovider misses warnings during `pytest_sessionfinish`, which is also fixed here. I think the usage of `_issue_warning_captured` can be removed/reduced further, but also that this is good enough for now. Ref: https://github.com/pytest-dev/pytest/issues/6681. --- src/_pytest/cacheprovider.py | 4 ++-- src/_pytest/warnings.py | 9 +++++++++ testing/test_cacheprovider.py | 4 +--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/_pytest/cacheprovider.py b/src/_pytest/cacheprovider.py index 2f7f8845440..9f94605d4df 100755 --- a/src/_pytest/cacheprovider.py +++ b/src/_pytest/cacheprovider.py @@ -71,10 +71,10 @@ def cache_dir_from_config(config): return resolve_from_str(config.getini("cache_dir"), config.rootdir) def warn(self, fmt, **args): - from _pytest.warnings import _issue_warning_captured + import warnings from _pytest.warning_types import PytestCacheWarning - _issue_warning_captured( + warnings.warn( PytestCacheWarning(fmt.format(**args) if args else fmt), self._config.hook, stacklevel=3, diff --git a/src/_pytest/warnings.py b/src/_pytest/warnings.py index 18e4def2116..2a4d189d573 100644 --- a/src/_pytest/warnings.py +++ b/src/_pytest/warnings.py @@ -136,6 +136,15 @@ def pytest_terminal_summary(terminalreporter): yield +@pytest.hookimpl(hookwrapper=True) +def pytest_sessionfinish(session): + config = session.config + with catch_warnings_for_item( + config=config, ihook=config.hook, when="config", item=None + ): + yield + + def _issue_warning_captured(warning, hook, stacklevel): """ This function should be used instead of calling ``warnings.warn`` directly when we are in the "configure" stage: diff --git a/testing/test_cacheprovider.py b/testing/test_cacheprovider.py index d37f18f0ff5..6dd987b613e 100644 --- a/testing/test_cacheprovider.py +++ b/testing/test_cacheprovider.py @@ -56,9 +56,7 @@ def test_cache_writefail_permissions(self, testdir): testdir.tmpdir.ensure_dir(".pytest_cache").chmod(mode) @pytest.mark.skipif(sys.platform.startswith("win"), reason="no chmod on windows") - @pytest.mark.filterwarnings( - "ignore:could not create cache path:pytest.PytestWarning" - ) + @pytest.mark.filterwarnings("default") def test_cache_failure_warns(self, testdir, monkeypatch): monkeypatch.setenv("PYTEST_DISABLE_PLUGIN_AUTOLOAD", "1") cache_dir = str(testdir.tmpdir.ensure_dir(".pytest_cache")) From 2b5adc88a7a83e03bd680d090314ade92602ff8c Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sat, 15 Feb 2020 02:01:22 +0100 Subject: [PATCH 2/2] Move test_issue4445_cacheprovider_set into test_cache_failure_warns Would need to be adjusted anyway non-trivially, and we can just harden `test_cache_failure_warns` instead. --- testing/test_cacheprovider.py | 10 +++++++++- testing/test_warnings.py | 21 --------------------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/testing/test_cacheprovider.py b/testing/test_cacheprovider.py index 6dd987b613e..ce425e26bd1 100644 --- a/testing/test_cacheprovider.py +++ b/testing/test_cacheprovider.py @@ -68,7 +68,15 @@ def test_cache_failure_warns(self, testdir, monkeypatch): assert result.ret == 1 # warnings from nodeids, lastfailed, and stepwise result.stdout.fnmatch_lines( - ["*could not create cache path*", "*3 warnings*"] + [ + # Validate location/stacklevel of warning from cacheprovider. + "*= warnings summary =*", + "*/cacheprovider.py:314", + " */cacheprovider.py:314: PytestCacheWarning: could not create cache path " + "{}/v/cache/nodeids".format(cache_dir), + ' config.cache.set("cache/nodeids", self.cached_nodeids)', + "*1 failed, 3 warnings in*", + ] ) finally: testdir.tmpdir.ensure_dir(".pytest_cache").chmod(mode) diff --git a/testing/test_warnings.py b/testing/test_warnings.py index e4d95738587..b05816073aa 100644 --- a/testing/test_warnings.py +++ b/testing/test_warnings.py @@ -712,27 +712,6 @@ def test_dummy(): assert "resultlog.py" in file assert func == "pytest_configure" - def test_issue4445_cacheprovider_set(self, testdir, capwarn): - """#4445: Make sure the warning points to a reasonable location - See origin of _issue_warning_captured at: _pytest.cacheprovider.py:59 - """ - testdir.tmpdir.join(".pytest_cache").write("something wrong") - testdir.runpytest(plugins=[capwarn()]) - - # with stacklevel=3 the warning originates from one stacklevel above - # _issue_warning_captured in cacheprovider.Cache.set and is thrown - # when there are errors during cache folder creation - - # set is called twice (in module stepwise and in cacheprovider) so emits - # two warnings when there are errors during cache folder creation. (is this intentional?) - assert len(capwarn.captured) == 2 - warning, location = capwarn.captured.pop() - file, lineno, func = location - - assert "could not create cache path" in str(warning.message) - assert "cacheprovider.py" in file - assert func == "set" - def test_issue4445_issue5928_mark_generator(self, testdir): """#4445 and #5928: Make sure the warning from an unknown mark points to the test file where this mark is used.