fix(ciapp/pytest): do not access XFAIL reason unsafely (#3277) (#3282)

* do not attempt to access wasxfail unsafely

* add changelog

* add unit tests

* improve logic and fix tests

* Apply suggestions from code review

Co-authored-by: Kyle Verhoog <[email protected]>

Co-authored-by: Kyle Verhoog <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 927ff22)

Co-authored-by: Juan Antonio Fernández de Alba <[email protected]>

Author: mergify[bot]

ddtrace/contrib/pytest/plugin.py

@@ -187,28 +187,31 @@ def pytest_runtest_makereport(item, call):
     xfail = hasattr(result, "wasxfail") or "xfail" in result.keywords
     has_skip_keyword = any(x in result.keywords for x in ["skip", "skipif", "skipped"])
 
+    # If run with --runxfail flag, tests behave as if they were not marked with xfail,
+    # that's why no test.XFAIL_REASON or test.RESULT tags will be added.
     if result.skipped:
         if xfail and not has_skip_keyword:
             # XFail tests that fail are recorded skipped by pytest, should be passed instead
-            span.set_tag(test.RESULT, test.Status.XFAIL.value)
-            span.set_tag(test.XFAIL_REASON, result.wasxfail)
             span.set_tag(test.STATUS, test.Status.PASS.value)
+            if not item.config.option.runxfail:
+                span.set_tag(test.RESULT, test.Status.XFAIL.value)
+                span.set_tag(test.XFAIL_REASON, getattr(result, "wasxfail", "XFail"))
         else:
             span.set_tag(test.STATUS, test.Status.SKIP.value)
         reason = _extract_reason(call)
         if reason is not None:
             span.set_tag(test.SKIP_REASON, reason)
     elif result.passed:
         span.set_tag(test.STATUS, test.Status.PASS.value)
-        if xfail and not has_skip_keyword:
+        if xfail and not has_skip_keyword and not item.config.option.runxfail:
             # XPass (strict=False) are recorded passed by pytest
             span.set_tag(test.XFAIL_REASON, getattr(result, "wasxfail", "XFail"))
             span.set_tag(test.RESULT, test.Status.XPASS.value)
     else:
-        if xfail and not has_skip_keyword:
+        span.set_tag(test.STATUS, test.Status.FAIL.value)
+        if xfail and not has_skip_keyword and not item.config.option.runxfail:
             # XPass (strict=True) are recorded failed by pytest, longrepr contains reason
-            span.set_tag(test.XFAIL_REASON, result.longrepr)
+            span.set_tag(test.XFAIL_REASON, getattr(result, "longrepr", "XFail"))
             span.set_tag(test.RESULT, test.Status.XPASS.value)
-        span.set_tag(test.STATUS, test.Status.FAIL.value)
         if call.excinfo:
             span.set_exc_info(call.excinfo.type, call.excinfo.value, call.excinfo.tb)

releasenotes/notes/pytest-fix-xfail-unsafe-access-f579fef50d5bd4fa.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    pytest: fix unsafe access to xfail reason.

tests/contrib/pytest/test_pytest.py

@@ -325,6 +325,44 @@ def test_xfail_conditional():
         assert spans[1].get_tag(test.RESULT) == test.Status.XFAIL.value
         assert spans[1].get_tag(test.XFAIL_REASON) == "test should xfail"
 
+    def test_xfail_runxfail_fails(self):
+        """Test xfail with --runxfail flags should not crash when failing."""
+        py_file = self.testdir.makepyfile(
+            """
+            import pytest
+
+            @pytest.mark.xfail(reason='should fail')
+            def test_should_fail():
+                assert 0
+
+        """
+        )
+        file_name = os.path.basename(py_file.strpath)
+        self.inline_run("--ddtrace", "--runxfail", file_name)
+        spans = self.pop_spans()
+
+        assert len(spans) == 1
+        assert spans[0].get_tag(test.STATUS) == test.Status.FAIL.value
+
+    def test_xfail_runxfail_passes(self):
+        """Test xfail with --runxfail flags should not crash when passing."""
+        py_file = self.testdir.makepyfile(
+            """
+            import pytest
+
+            @pytest.mark.xfail(reason='should fail')
+            def test_should_pass():
+                assert 1
+
+        """
+        )
+        file_name = os.path.basename(py_file.strpath)
+        self.inline_run("--ddtrace", "--runxfail", file_name)
+        spans = self.pop_spans()
+
+        assert len(spans) == 1
+        assert spans[0].get_tag(test.STATUS) == test.Status.PASS.value
+
     def test_xpass_not_strict(self):
         """Test xpass (unexpected passing) with strict=False, should be marked as pass."""
         py_file = self.testdir.makepyfile(