Skip to content

Commit 64dc4ac

Browse files
authored
fix(test-plugins-filler): aggregate --verify-traces results across xdist workers (#2664)
* Fix bug in verify-traces * Convert to pydantic models * Add Ignore stack and gas mode * Allow differences in return_data for no-stack comparators
1 parent d206d4f commit 64dc4ac

6 files changed

Lines changed: 686 additions & 63 deletions

File tree

packages/testing/src/execution_testing/cli/pytest_commands/plugins/filler/tests/test_verify_traces.py

Lines changed: 273 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,22 @@
22

33
import json
44
from pathlib import Path
5+
from typing import Any
6+
from unittest.mock import MagicMock
57

68
from execution_testing.cli.pytest_commands.plugins.filler.verify_traces import ( # noqa: E501
9+
TraceVerifier,
710
_load_traces_from_dump_dir,
11+
pytest_testnodedown,
812
)
913
from execution_testing.client_clis.cli_types import (
1014
Traces,
1115
)
16+
from execution_testing.client_clis.trace_comparators import (
17+
TraceComparisonResult,
18+
TraceDifference,
19+
TransactionCountMismatch,
20+
)
1221

1322

1423
def _write_trace_file(
@@ -93,3 +102,267 @@ def test_numeric_sorting_not_lexical(self, tmp_path: Path) -> None:
93102
assert len(result) == 3
94103
# Verify they are in order 0, 2, 10 by checking the list
95104
# length — ordering is guaranteed by the implementation
105+
106+
107+
def _make_trace_verifier(
108+
json_formatter: Any = None,
109+
) -> TraceVerifier:
110+
"""Construct a minimally-configured TraceVerifier for unit tests."""
111+
return TraceVerifier(
112+
config=MagicMock(),
113+
comparators=[],
114+
formatter=MagicMock(),
115+
baseline_dir=Path("/tmp/baseline"),
116+
filler_path=Path("/tmp/filler"),
117+
json_formatter=json_formatter,
118+
)
119+
120+
121+
def _make_session(workeroutput: dict | None) -> Any:
122+
"""
123+
Build a fake pytest.Session.
124+
125+
``workeroutput=None`` simulates the controller (the attribute is
126+
absent); a dict simulates a worker.
127+
"""
128+
config = MagicMock()
129+
if workeroutput is None:
130+
# Controller — `workeroutput` must NOT exist on config.
131+
del config.workeroutput
132+
else:
133+
config.workeroutput = workeroutput
134+
session = MagicMock()
135+
session.config = config
136+
return session
137+
138+
139+
class TestXdistAggregation:
140+
"""
141+
Test xdist worker→controller aggregation.
142+
143+
Background: ``TraceVerifier`` keeps results in an instance dict.
144+
Under ``pytest-xdist -n N`` each worker has its own subprocess and
145+
its own plugin instance, so the controller's instance is empty
146+
unless workers explicitly forward their results. The plugin uses
147+
``config.workeroutput`` on workers and ``pytest_testnodedown`` on
148+
the controller — these tests exercise that path without spinning
149+
up real xdist subprocesses.
150+
"""
151+
152+
def test_worker_writes_results_to_workeroutput(self) -> None:
153+
"""Worker's pytest_sessionfinish stores results in workeroutput."""
154+
verifier = _make_trace_verifier()
155+
verifier.test_results = {
156+
"test_a": {
157+
"exact": TraceComparisonResult(equivalent=True),
158+
},
159+
"test_b": {
160+
"exact": TraceComparisonResult(
161+
equivalent=False,
162+
differences=[
163+
TraceDifference(
164+
transaction_index=0,
165+
trace_line_index=3,
166+
baseline="ADD",
167+
current="MUL",
168+
),
169+
],
170+
),
171+
},
172+
}
173+
workeroutput: dict = {}
174+
verifier.pytest_sessionfinish(
175+
_make_session(workeroutput=workeroutput), 0
176+
)
177+
178+
payload = workeroutput["trace_verifier_results"]
179+
assert set(payload.keys()) == {"test_a", "test_b"}
180+
assert payload["test_a"]["exact"]["equivalent"] is True
181+
assert payload["test_b"]["exact"]["equivalent"] is False
182+
assert (
183+
payload["test_b"]["exact"]["differences"][0]["baseline"] == "ADD"
184+
)
185+
186+
def test_controller_does_not_write_workeroutput(self) -> None:
187+
"""On the controller (no workeroutput attribute), hook is a no-op."""
188+
verifier = _make_trace_verifier()
189+
verifier.test_results = {
190+
"test_a": {"exact": TraceComparisonResult(equivalent=True)},
191+
}
192+
# Should not raise even though config has no `workeroutput`.
193+
verifier.pytest_sessionfinish(_make_session(workeroutput=None), 0)
194+
195+
def test_worker_with_empty_results_does_not_write(self) -> None:
196+
"""A worker that ran no comparisons should not write a payload."""
197+
verifier = _make_trace_verifier()
198+
workeroutput: dict = {}
199+
verifier.pytest_sessionfinish(
200+
_make_session(workeroutput=workeroutput), 0
201+
)
202+
assert "trace_verifier_results" not in workeroutput
203+
204+
def test_pytest_testnodedown_merges_payload(self) -> None:
205+
"""Controller hook merges a worker's payload into the plugin."""
206+
controller_plugin = _make_trace_verifier()
207+
208+
# Simulate the data a worker would send back.
209+
worker_payload = {
210+
"test_a": {
211+
"exact": TraceComparisonResult(equivalent=True).model_dump(
212+
mode="json"
213+
),
214+
},
215+
"test_b": {
216+
"exact-no-stack": TraceComparisonResult(
217+
equivalent=False,
218+
differences=[
219+
TransactionCountMismatch(
220+
baseline_count=2, current_count=1
221+
),
222+
],
223+
).model_dump(mode="json"),
224+
},
225+
}
226+
node = MagicMock()
227+
node.workeroutput = {"trace_verifier_results": worker_payload}
228+
node.config.pluginmanager.get_plugin.return_value = controller_plugin
229+
230+
pytest_testnodedown(node, error=None)
231+
232+
assert set(controller_plugin.test_results.keys()) == {
233+
"test_a",
234+
"test_b",
235+
}
236+
assert (
237+
controller_plugin.test_results["test_a"]["exact"].equivalent
238+
is True
239+
)
240+
diff_b = controller_plugin.test_results["test_b"][
241+
"exact-no-stack"
242+
].differences[0]
243+
assert isinstance(diff_b, TransactionCountMismatch)
244+
assert diff_b.baseline_count == 2
245+
assert diff_b.current_count == 1
246+
247+
def test_pytest_testnodedown_multiple_workers_aggregate(self) -> None:
248+
"""Two workers' payloads both land in the controller plugin."""
249+
controller_plugin = _make_trace_verifier()
250+
251+
def _send(node_results: dict[str, TraceComparisonResult]) -> None:
252+
payload = {
253+
nodeid: {"exact": result.model_dump(mode="json")}
254+
for nodeid, result in node_results.items()
255+
}
256+
node = MagicMock()
257+
node.workeroutput = {"trace_verifier_results": payload}
258+
node.config.pluginmanager.get_plugin.return_value = (
259+
controller_plugin
260+
)
261+
pytest_testnodedown(node, error=None)
262+
263+
_send(
264+
{
265+
"test_w0_a": TraceComparisonResult(equivalent=True),
266+
"test_w0_b": TraceComparisonResult(equivalent=True),
267+
}
268+
)
269+
_send(
270+
{
271+
"test_w1_a": TraceComparisonResult(equivalent=True),
272+
"test_w1_b": TraceComparisonResult(equivalent=True),
273+
"test_w1_c": TraceComparisonResult(equivalent=True),
274+
}
275+
)
276+
277+
# The bug being fixed: prior to aggregation, the controller saw
278+
# only one worker's slice (or none). All five nodeids must be
279+
# present after merging both workers.
280+
assert set(controller_plugin.test_results.keys()) == {
281+
"test_w0_a",
282+
"test_w0_b",
283+
"test_w1_a",
284+
"test_w1_b",
285+
"test_w1_c",
286+
}
287+
288+
def test_pytest_testnodedown_no_payload_is_noop(self) -> None:
289+
"""Worker with no payload (didn't run any tests) is a no-op."""
290+
controller_plugin = _make_trace_verifier()
291+
node = MagicMock()
292+
node.workeroutput = {}
293+
node.config.pluginmanager.get_plugin.return_value = controller_plugin
294+
295+
pytest_testnodedown(node, error=None)
296+
297+
assert controller_plugin.test_results == {}
298+
299+
def test_pytest_testnodedown_no_plugin_registered(self) -> None:
300+
"""If the trace-verifier plugin isn't registered, hook is a no-op."""
301+
node = MagicMock()
302+
node.workeroutput = {
303+
"trace_verifier_results": {
304+
"test_a": {
305+
"exact": TraceComparisonResult(equivalent=True).model_dump(
306+
mode="json"
307+
),
308+
},
309+
},
310+
}
311+
node.config.pluginmanager.get_plugin.return_value = None
312+
# Should not raise.
313+
pytest_testnodedown(node, error=None)
314+
315+
316+
class TestWorkerTerminalSummarySkipped:
317+
"""Workers must not write the JSON / text report."""
318+
319+
def test_worker_skips_terminal_summary(self) -> None:
320+
"""`pytest_terminal_summary` is a no-op when workerinput is set."""
321+
json_formatter = MagicMock()
322+
json_formatter.output_path = Path("/tmp/report.json")
323+
verifier = _make_trace_verifier(json_formatter=json_formatter)
324+
verifier.test_results = {
325+
"test_a": {"exact": TraceComparisonResult(equivalent=True)},
326+
}
327+
328+
terminalreporter = MagicMock()
329+
worker_config = MagicMock()
330+
# Workers have `workerinput`; controllers don't.
331+
worker_config.workerinput = {"workerid": "gw0"}
332+
333+
verifier.pytest_terminal_summary(
334+
terminalreporter, exitstatus=0, config=worker_config
335+
)
336+
337+
json_formatter.write.assert_not_called()
338+
terminalreporter.write_sep.assert_not_called()
339+
340+
def test_controller_writes_terminal_summary(self) -> None:
341+
"""Controller (no workerinput) writes the report normally."""
342+
json_formatter = MagicMock()
343+
json_formatter.output_path = Path("/tmp/report.json")
344+
text_formatter = MagicMock()
345+
text_formatter.format_summary.return_value = "summary line"
346+
verifier = TraceVerifier(
347+
config=MagicMock(),
348+
comparators=[],
349+
formatter=text_formatter,
350+
baseline_dir=Path("/tmp/baseline"),
351+
filler_path=Path("/tmp/filler"),
352+
json_formatter=json_formatter,
353+
)
354+
verifier.test_results = {
355+
"test_a": {"exact": TraceComparisonResult(equivalent=True)},
356+
}
357+
358+
terminalreporter = MagicMock()
359+
controller_config = MagicMock()
360+
# Controllers don't have `workerinput`.
361+
del controller_config.workerinput
362+
363+
verifier.pytest_terminal_summary(
364+
terminalreporter, exitstatus=0, config=controller_config
365+
)
366+
367+
json_formatter.write.assert_called_once_with(verifier.test_results)
368+
terminalreporter.write_sep.assert_called()

0 commit comments

Comments
 (0)