From 56d414177adb0194c52d5e994eef7fe264c5e82a Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 3 Sep 2018 20:13:41 -0300 Subject: [PATCH] Remove nodeid from messages for warnings generated by standard warnings Standard warnings already contain the proper location, so we don't need to also print the node id --- src/_pytest/assertion/rewrite.py | 7 ++++--- src/_pytest/deprecated.py | 4 ++-- src/_pytest/junitxml.py | 5 ++--- src/_pytest/pytester.py | 2 +- src/_pytest/terminal.py | 27 +++++++++++++++++++++------ testing/test_assertrewrite.py | 15 ++++++++++----- testing/test_junitxml.py | 5 +---- testing/test_warnings.py | 7 ++----- 8 files changed, 43 insertions(+), 29 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 9c622213c..4f16750b2 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -209,11 +209,12 @@ class AssertionRewritingHook(object): self._must_rewrite.update(names) def _warn_already_imported(self, name): - import warnings from _pytest.warning_types import PytestWarning + from _pytest.warnings import _issue_config_warning - warnings.warn( - "Module already imported so cannot be rewritten: %s" % name, PytestWarning + _issue_config_warning( + PytestWarning("Module already imported so cannot be rewritten: %s" % name), + self.config, ) def load_module(self, name): diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index a77ebf6c8..82bf1d98d 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -20,7 +20,7 @@ FUNCARG_PREFIX = ( ) FIXTURE_FUNCTION_CALL = ( - "Fixture {name} called directly. Fixtures are not meant to be called directly, " + 'Fixture "{name}" called directly. Fixtures are not meant to be called directly, ' "are created automatically when test functions request them as parameters. " "See https://docs.pytest.org/en/latest/fixture.html for more information." ) @@ -29,7 +29,7 @@ CFG_PYTEST_SECTION = ( "[pytest] section in {filename} files is deprecated, use [tool:pytest] instead." ) -GETFUNCARGVALUE = "use of getfuncargvalue is deprecated, use getfixturevalue" +GETFUNCARGVALUE = "getfuncargvalue is deprecated, use getfixturevalue" RESULT_LOG = ( "--result-log is deprecated and scheduled for removal in pytest 4.0.\n" diff --git a/src/_pytest/junitxml.py b/src/_pytest/junitxml.py index 2f34970a1..776cd935c 100644 --- a/src/_pytest/junitxml.py +++ b/src/_pytest/junitxml.py @@ -258,12 +258,11 @@ def record_property(request): @pytest.fixture -def record_xml_property(record_property): +def record_xml_property(record_property, request): """(Deprecated) use record_property.""" - import warnings from _pytest import deprecated - warnings.warn(deprecated.RECORD_XML_PROPERTY, DeprecationWarning, stacklevel=2) + request.node.std_warn(deprecated.RECORD_XML_PROPERTY, DeprecationWarning) return record_property diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index f88244468..002eb62a5 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -126,7 +126,7 @@ class LsofFdLeakChecker(object): error.append(error[0]) error.append("*** function %s:%s: %s " % item.location) error.append("See issue #2366") - item.warn("", "\n".join(error)) + item.std_warn("", "\n".join(error), pytest.PytestWarning) # XXX copied from execnet's conftest.py - needs to be merged diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 5140741a3..4f6f09537 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -188,17 +188,20 @@ def pytest_report_teststatus(report): @attr.s class WarningReport(object): """ - Simple structure to hold warnings information captured by ``pytest_logwarning``. + Simple structure to hold warnings information captured by ``pytest_logwarning`` and ``pytest_warning_captured``. :ivar str message: user friendly message about the warning :ivar str|None nodeid: node id that generated the warning (see ``get_location``). :ivar tuple|py.path.local fslocation: file system location of the source of the warning (see ``get_location``). + + :ivar bool legacy: if this warning report was generated from the deprecated ``pytest_logwarning`` hook. """ message = attr.ib() nodeid = attr.ib(default=None) fslocation = attr.ib(default=None) + legacy = attr.ib(default=False) def get_location(self, config): """ @@ -211,6 +214,8 @@ class WarningReport(object): if isinstance(self.fslocation, tuple) and len(self.fslocation) >= 2: filename, linenum = self.fslocation[:2] relpath = py.path.local(filename).relto(config.invocation_dir) + if not relpath: + relpath = str(filename) return "%s:%s" % (relpath, linenum) else: return str(self.fslocation) @@ -327,7 +332,9 @@ class TerminalReporter(object): def pytest_logwarning(self, fslocation, message, nodeid): warnings = self.stats.setdefault("warnings", []) - warning = WarningReport(fslocation=fslocation, message=message, nodeid=nodeid) + warning = WarningReport( + fslocation=fslocation, message=message, nodeid=nodeid, legacy=True + ) warnings.append(warning) def pytest_warning_captured(self, warning_message, item): @@ -707,12 +714,20 @@ class TerminalReporter(object): self.write_sep("=", "warnings summary", yellow=True, bold=False) for location, warning_records in grouped: - if location: + # legacy warnings show their location explicitly, while standard warnings look better without + # it because the location is already formatted into the message + warning_records = list(warning_records) + is_legacy = warning_records[0].legacy + if location and is_legacy: self._tw.line(str(location)) for w in warning_records: - lines = w.message.splitlines() - indented = "\n".join(" " + x for x in lines) - self._tw.line(indented.rstrip()) + if is_legacy: + lines = w.message.splitlines() + indented = "\n".join(" " + x for x in lines) + message = indented.rstrip() + else: + message = w.message.rstrip() + self._tw.line(message) self._tw.line() self._tw.line("-- Docs: https://docs.pytest.org/en/latest/warnings.html") diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index c82e1dccf..c4cafcaab 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -759,11 +759,16 @@ def test_rewritten(): testdir.makepyfile("import a_package_without_init_py.module") assert testdir.runpytest().ret == EXIT_NOTESTSCOLLECTED - def test_rewrite_warning(self, pytestconfig): - hook = AssertionRewritingHook(pytestconfig) - - with pytest.warns(pytest.PytestWarning): - hook.mark_rewrite("_pytest") + def test_rewrite_warning(self, testdir): + testdir.makeconftest( + """ + import pytest + pytest.register_assert_rewrite("_pytest") + """ + ) + # needs to be a subprocess because pytester explicitly disables this warning + result = testdir.runpytest_subprocess() + result.stdout.fnmatch_lines("*Module already imported*: _pytest") def test_rewrite_module_imported_from_conftest(self, testdir): testdir.makeconftest( diff --git a/testing/test_junitxml.py b/testing/test_junitxml.py index 04b4ee2d7..3928548a8 100644 --- a/testing/test_junitxml.py +++ b/testing/test_junitxml.py @@ -1024,10 +1024,7 @@ def test_record_attribute(testdir): tnode.assert_attr(bar="1") tnode.assert_attr(foo="<1") result.stdout.fnmatch_lines( - [ - "test_record_attribute.py::test_record", - "*test_record_attribute.py:6:*record_xml_attribute is an experimental feature", - ] + ["*test_record_attribute.py:6:*record_xml_attribute is an experimental feature"] ) diff --git a/testing/test_warnings.py b/testing/test_warnings.py index f0a172196..eb4928a46 100644 --- a/testing/test_warnings.py +++ b/testing/test_warnings.py @@ -40,14 +40,12 @@ def pyfile_with_warnings(testdir, request): @pytest.mark.filterwarnings("default") def test_normal_flow(testdir, pyfile_with_warnings): """ - Check that the warnings section is displayed, containing test node ids followed by - all warnings generated by that test node. + Check that the warnings section is displayed. """ result = testdir.runpytest() result.stdout.fnmatch_lines( [ "*== %s ==*" % WARNINGS_SUMMARY_HEADER, - "*test_normal_flow.py::test_func", "*normal_flow_module.py:3: UserWarning: user warning", '* warnings.warn(UserWarning("user warning"))', "*normal_flow_module.py:4: RuntimeWarning: runtime warning", @@ -55,7 +53,6 @@ def test_normal_flow(testdir, pyfile_with_warnings): "* 1 passed, 2 warnings*", ] ) - assert result.stdout.str().count("test_normal_flow.py::test_func") == 1 @pytest.mark.filterwarnings("always") @@ -343,7 +340,7 @@ def test_collection_warnings(testdir): [ "*== %s ==*" % WARNINGS_SUMMARY_HEADER, "*collection_warnings.py:3: UserWarning: collection warning", - ' warnings.warn(UserWarning("collection warning"))', + ' warnings.warn(UserWarning("collection warning"))', "* 1 passed, 1 warnings*", ] )