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

@@ -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(
+        _issue_config_warning(
-            "Module already imported so cannot be rewritten: %s" % name, PytestWarning
+            PytestWarning("Module already imported so cannot be rewritten: %s" % name),
+            self.config,
         )
 
     def load_module(self, name):

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"

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
 

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

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:
+                    if is_legacy:
                         lines = w.message.splitlines()
                         indented = "\n".join("  " + x for x in lines)
-                    self._tw.line(indented.rstrip())
+                        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")
 

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):
+    def test_rewrite_warning(self, testdir):
-        hook = AssertionRewritingHook(pytestconfig)
+        testdir.makeconftest(
-
+            """
-        with pytest.warns(pytest.PytestWarning):
+            import pytest
-            hook.mark_rewrite("_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(

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:6:*record_xml_attribute is an experimental feature"]
-            "test_record_attribute.py::test_record",
-            "*test_record_attribute.py:6:*record_xml_attribute is an experimental feature",
-        ]
     )
 
 

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
+    Check that the warnings section is displayed.
-    all warnings generated by that test node.
     """
     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")