Do not rebuild a warning whose first argument is not its message

changelog/1372.bugfix.rst

@@ -0,0 +1,6 @@
+Warnings crossing to the controller now keep the state they carry beside their
+``args``. Only ``args`` was transferred, and the warning was rebuilt by calling
+the class with them, so a warning whose message is derived from its own fields
+arrived rendered from defaults — reading like a real message rather than failing.
+The reduce state is now transferred as well, and the instance is rebuilt without
+re-running ``__init__``.

src/xdist/remote.py

@@ -324,6 +324,32 @@ def pytest_warning_recorded(
         )
 
 
+def _serializable_reduce_state(message: Warning) -> Any | None:
+    """Return the state `__reduce__` carries, when it can cross the wire.
+
+    `None` means the state could not be transferred and the controller should
+    not pretend otherwise. A class that replaces `__reduce__` with its own
+    callable is left alone: rebuilding it here would run something we do not
+    understand, and its own protocol already round-trips under pickle.
+    """
+    try:
+        reduced = message.__reduce__()
+    except Exception:
+        return None
+    if not isinstance(reduced, tuple) or len(reduced) < 3:
+        return None
+    if reduced[0] is not type(message):
+        return None
+    state = reduced[2]
+    if state is None:
+        return None
+    try:
+        execnet.dumps(state)
+    except execnet.DumpError:
+        return None
+    return state
+
+
 def serialize_warning_message(
     warning_message: warnings.WarningMessage,
 ) -> dict[str, Any]:
@@ -339,11 +365,18 @@ def serialize_warning_message(
             message_args = None
         else:
             message_args = warning_message.message.args
+        # `args` alone does not describe a warning. `BaseException.__reduce__`
+        # carries the instance dictionary as a third element, and a class may
+        # replace `__reduce__` entirely. Sending only `args` drops whatever the
+        # instance keeps beside them, and the controller then rebuilds something
+        # that renders plausibly and is not the same warning.
+        message_state = _serializable_reduce_state(warning_message.message)
     else:
         message_str = warning_message.message
         message_module = None
         message_class_name = None
         message_args = None
+        message_state = None
     if warning_message.category:
         category_module = warning_message.category.__module__
         category_class_name = warning_message.category.__name__
@@ -356,6 +389,7 @@ def serialize_warning_message(
         "message_module": message_module,
         "message_class_name": message_class_name,
         "message_args": message_args,
+        "message_state": message_state,
         "category_module": category_module,
         "category_class_name": category_class_name,
     }

src/xdist/workermanage.py

@@ -476,6 +476,30 @@ def process_from_remote(
             self.notify_inproc("errordown", node=self, error=excinfo)
 
 
+def _restore_warning_state(message: BaseException, state: Any) -> None:
+    """Apply the state `__reduce__` carried, in the two shapes it comes in.
+
+    `BaseException` implements `__setstate__` as a `setattr` per key, which is
+    the branch pickle takes for exceptions. A class using `__slots__` reduces to
+    a `(dict, slots)` two-tuple instead, and that one has to be applied by hand.
+    """
+    setstate = getattr(message, "__setstate__", None)
+    if isinstance(state, tuple) and len(state) == 2:
+        instance_dict, slot_state = state
+        if instance_dict:
+            if setstate is not None:
+                setstate(instance_dict)
+            else:
+                message.__dict__.update(instance_dict)
+        for key, value in (slot_state or {}).items():
+            setattr(message, key, value)
+        return
+    if setstate is not None:
+        setstate(state)
+    else:
+        message.__dict__.update(state)
+
+
 def unserialize_warning_message(data: dict[str, Any]) -> warnings.WarningMessage:
     import importlib
 
@@ -484,10 +508,20 @@ def unserialize_warning_message(data: dict[str, Any]) -> warnings.WarningMessage
         cls = getattr(mod, data["message_class_name"])
         message = None
         if data["message_args"] is not None:
+            # Rebuilt without running `__init__`. A warning is free to derive its
+            # message from its own fields, and calling the class with the args
+            # would hand it back its rendered text as if it were input: the
+            # instance then renders around its own output, or silently falls back
+            # to whatever its defaults say. `BaseException.__setstate__` restores
+            # the rest, which is the half we used not to send at all.
             try:
-                message = cls(*data["message_args"])
-            except TypeError:
-                pass
+                message = cls.__new__(cls)
+                BaseException.__init__(message, *data["message_args"])
+                state = data.get("message_state")
+                if state is not None:
+                    _restore_warning_state(message, state)
+            except Exception:
+                message = None
         if message is None:
             # could not recreate the original warning instance;
             # create a generic Warning instance with the original

testing/acceptance_test.py

@@ -867,6 +867,40 @@ def test_func(request):
         result = pytester.runpytest(n)
         result.stdout.fnmatch_lines(["*MyWarning*", "*1 passed, 1 warning*"])
 
+    @pytest.mark.parametrize("n", ["-n0", "-n1"])
+    def test_state_beyond_args_survives(
+        self, pytester: pytest.Pytester, n: str
+    ) -> None:
+        """A warning keeping state beside its args must report the same either way.
+
+        `-n0` is the reference: no serialization happens, so whatever it prints is
+        what the warning says. `-n1` must match it. The class below renders its
+        message from `self.code`, which `args` does not carry, so rebuilding it by
+        calling the class reports the default instead — and reads like a real
+        message while doing it.
+        """
+        pytester.makepyfile(
+            """
+            import warnings
+
+            class CodeWarning(UserWarning):
+
+                def __init__(self, code=None):
+                    self.code = code
+                    super().__init__()
+
+                def __str__(self):
+                    return "code {} tripped".format(self.code or "unknown")
+
+            def test_func():
+                warnings.warn(CodeWarning(42))
+        """
+        )
+        pytester.syspathinsert()
+        result = pytester.runpytest(n)
+        result.stdout.fnmatch_lines(["*code 42 tripped*", "*1 passed, 1 warning*"])
+        result.stdout.no_fnmatch_line("*code unknown tripped*")
+
     @pytest.mark.parametrize("n", ["-n0", "-n1"])
     def test_unserializable_arguments(self, pytester: pytest.Pytester, n: str) -> None:
         """Check that warnings with unserializable arguments are handled correctly (#349)."""

testing/test_workermanage.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+from collections.abc import Callable
 from pathlib import Path
 import shutil
 import textwrap
@@ -497,6 +498,65 @@ class MyWarning2(UserWarning):
             assert v1 == v2
 
 
+class WarningWithFieldFirst(UserWarning):
+    """First argument is a field; the message is rendered from it."""
+
+    def __init__(self, resource: str) -> None:
+        self.resource = resource
+        super().__init__(f"{resource!r} is not available")
+
+
+class WarningWithoutArgs(UserWarning):
+    """No args at all; the text is derived from state."""
+
+    def __init__(self, code: int | None = None) -> None:
+        self.code = code
+        super().__init__()
+
+    def __str__(self) -> str:
+        return f"code {self.code or 'unknown'} tripped"
+
+
+@pytest.mark.parametrize(
+    ("factory", "expected_text", "attribute", "expected_value"),
+    [
+        (
+            lambda: WarningWithFieldFirst("gpu"),
+            "'gpu' is not available",
+            "resource",
+            "gpu",
+        ),
+        (lambda: WarningWithoutArgs(42), "code 42 tripped", "code", 42),
+    ],
+    ids=["field-first", "state-only"],
+)
+def test_warning_state_survives_the_round_trip(
+    factory: Callable[[], UserWarning],
+    expected_text: str,
+    attribute: str,
+    expected_value: object,
+) -> None:
+    """The rebuilt warning must be the same warning, not one that reads like it.
+
+    Calling the class with its own `args` re-runs `__init__`, which for these two
+    shapes either renders around the rendered text or falls back to a default.
+    Both survive `pickle` and `copy` untouched, so the loss was ours.
+    """
+    with pytest.warns(UserWarning) as w:
+        warnings.warn(factory())
+
+    assert len(w) == 1
+    w_msg = w[0]
+    assert str(w_msg.message) == expected_text
+
+    data = serialize_warning_message(w_msg)
+    rebuilt = unserialize_warning_message(data).message
+
+    assert type(rebuilt) is type(w_msg.message)
+    assert str(rebuilt) == expected_text
+    assert getattr(rebuilt, attribute) == expected_value
+
+
 class MyWarningUnknown(UserWarning):
     # Changing the __module__ attribute is only safe if class can be imported
     # from there