From 917ce9aa0102c7f0ec8fdac118058c41ffb603e6 Mon Sep 17 00:00:00 2001
From: Israel Fruchter <israel.fruchter@gmail.com>
Date: Sun, 3 Sep 2023 20:33:54 +0300
Subject: [PATCH] Fix user_properties not saved to XML if fixture errors during
 teardown

Move handling of user_properties to `finalize()`.

Previously if a fixture failed during teardown, `pytest_runtest_logreport` would not be called with "teardown", resulting in the user properties not being saved on the JUnit XML file.

Fixes: #11367
---
 AUTHORS                    |  1 +
 changelog/11367.bugfix.rst |  1 +
 src/_pytest/junitxml.py    |  7 ++++---
 testing/test_junitxml.py   | 30 ++++++++++++++++++++++++++++++
 4 files changed, 36 insertions(+), 3 deletions(-)
 create mode 100644 changelog/11367.bugfix.rst

diff --git a/AUTHORS b/AUTHORS
index 6fa7a41eb8c..52157cc0a36 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -170,6 +170,7 @@ Ian Lesperance
 Ilya Konstantinov
 Ionuț Turturică
 Isaac Virshup
+Israel Fruchter
 Itxaso Aizpurua
 Iwan Briquemont
 Jaap Broekhuizen
diff --git a/changelog/11367.bugfix.rst b/changelog/11367.bugfix.rst
new file mode 100644
index 00000000000..dda40db0fc9
--- /dev/null
+++ b/changelog/11367.bugfix.rst
@@ -0,0 +1 @@
+Fixed bug where `user_properties` where not being saved in the JUnit XML file if a fixture failed during teardown.
diff --git a/src/_pytest/junitxml.py b/src/_pytest/junitxml.py
index 9242d46d9df..ed259e4c41d 100644
--- a/src/_pytest/junitxml.py
+++ b/src/_pytest/junitxml.py
@@ -502,6 +502,10 @@ def finalize(self, report: TestReport) -> None:
         # Local hack to handle xdist report order.
         workernode = getattr(report, "node", None)
         reporter = self.node_reporters.pop((nodeid, workernode))
+
+        for propname, propvalue in report.user_properties:
+            reporter.add_property(propname, str(propvalue))
+
         if reporter is not None:
             reporter.finalize()
 
@@ -599,9 +603,6 @@ def pytest_runtest_logreport(self, report: TestReport) -> None:
             reporter = self._opentestcase(report)
             reporter.write_captured_output(report)
 
-            for propname, propvalue in report.user_properties:
-                reporter.add_property(propname, str(propvalue))
-
             self.finalize(report)
             report_wid = getattr(report, "worker_id", None)
             report_ii = getattr(report, "item_index", None)
diff --git a/testing/test_junitxml.py b/testing/test_junitxml.py
index 690830329c0..3f88c21e2b7 100644
--- a/testing/test_junitxml.py
+++ b/testing/test_junitxml.py
@@ -1228,6 +1228,36 @@ def test_record(record_property, other):
     result.stdout.fnmatch_lines(["*= 1 passed in *"])
 
 
+def test_record_property_on_test_and_teardown_failure(
+    pytester: Pytester, run_and_parse: RunAndParse
+) -> None:
+    pytester.makepyfile(
+        """
+        import pytest
+
+        @pytest.fixture
+        def other(record_property):
+            record_property("bar", 1)
+            yield
+            assert 0
+
+        def test_record(record_property, other):
+            record_property("foo", "<1")
+            assert 0
+    """
+    )
+    result, dom = run_and_parse()
+    node = dom.find_first_by_tag("testsuite")
+    tnodes = node.find_by_tag("testcase")
+    for tnode in tnodes:
+        psnode = tnode.find_first_by_tag("properties")
+        assert psnode, f"testcase didn't had expected properties:\n{tnode}"
+        pnodes = psnode.find_by_tag("property")
+        pnodes[0].assert_attr(name="bar", value="1")
+        pnodes[1].assert_attr(name="foo", value="<1")
+    result.stdout.fnmatch_lines(["*= 1 failed, 1 error *"])
+
+
 def test_record_property_same_name(
     pytester: Pytester, run_and_parse: RunAndParse
 ) -> None: