From 30dde3a267dede39a06b1160636ce7ba93c895db Mon Sep 17 00:00:00 2001 From: Naufan Rizal Date: Thu, 27 Mar 2025 17:39:22 +1100 Subject: [PATCH] Fix the hooks handling order --- .../collector/payload.py | 4 +- .../pytest_plugin/buildkite_plugin.py | 40 ++++++++++++------- .../collector/test_payload.py | 8 ++-- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/buildkite_test_collector/collector/payload.py b/src/buildkite_test_collector/collector/payload.py index d69df04..d3397ac 100644 --- a/src/buildkite_test_collector/collector/payload.py +++ b/src/buildkite_test_collector/collector/payload.py @@ -141,7 +141,9 @@ def tag_execution(self, key: str, val: str) -> 'TestData': if not isinstance(key, str) or not isinstance(val, str): raise TypeError("Expected string for key and value") - self.tags[key] = val + new_tags = self.tags.copy() + new_tags[key] = val + return replace(self, tags=new_tags) def finish(self) -> 'TestData': """Set the end_at and duration on this test""" diff --git a/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py b/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py index c1c97c1..3e11624 100644 --- a/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py +++ b/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py @@ -1,6 +1,5 @@ """Buildkite test collector plugin for Pytest""" import json - from uuid import uuid4 from ..collector.payload import TestData @@ -29,26 +28,21 @@ def pytest_runtest_logstart(self, nodeid, location): ) self.in_flight[nodeid] = test_data - def pytest_runtest_teardown(self, item): - """pytest_runtest_hook hook callback to collect execution_tag""" - test_data = self.in_flight.get(item.nodeid) - - if test_data: - tags = item.iter_markers("execution_tag") - for tag in tags: - test_data.tag_execution(tag.args[0], tag.args[1]) - def pytest_runtest_logreport(self, report): - """pytest_runtest_logreport hook callback""" - if report.when != 'teardown': + """pytest_runtest_logreport hook callback to get test outcome after test call""" + + # This hook is called three times during the lifecycle of a test: + # after the setup phase, the call phase, and the teardown phase. + # Since we want to capture the outcome from the call phase, + # we only proceed when this hook is triggered following the call phase. + # See: https://github.com/buildkite/test-collector-python/pull/45 + if report.when != 'call': return nodeid = report.nodeid test_data = self.in_flight.get(nodeid) if test_data: - test_data = test_data.finish() - if report.passed: test_data = test_data.passed() @@ -58,7 +52,23 @@ def pytest_runtest_logreport(self, report): if report.skipped: test_data = test_data.skipped() - del self.in_flight[nodeid] + # TestData is immutable. + # We need to replace the test_data in `in_flight` with updated test_data, + # so we can get the correct result when we process it during the teardown hook. + self.in_flight[nodeid] = test_data + + def pytest_runtest_teardown(self, item): + """pytest_runtest_hook hook callback to mark test as finished and add it to the payload""" + test_data = self.in_flight.get(item.nodeid) + + if test_data: + test_data = test_data.finish() + + tags = item.iter_markers("execution_tag") + for tag in tags: + test_data = test_data.tag_execution(tag.args[0], tag.args[1]) + + del self.in_flight[item.nodeid] self.payload = self.payload.push_test_data(test_data) def save_payload_as_json(self, path): diff --git a/tests/buildkite_test_collector/collector/test_payload.py b/tests/buildkite_test_collector/collector/test_payload.py index e70fdf9..2ed4fe6 100644 --- a/tests/buildkite_test_collector/collector/test_payload.py +++ b/tests/buildkite_test_collector/collector/test_payload.py @@ -171,14 +171,14 @@ def test_test_data_as_json_when_skipped(skipped_test): class TestTestDataTagExecution: def test_test_data_tag_execution(self, successful_test): - successful_test.tag_execution("owner", "test-engine") - successful_test.tag_execution("python.version", "3.12.3") + test_data = successful_test.tag_execution("owner", "test-engine") + test_data = test_data.tag_execution("python.version", "3.12.3") expected_tags = {"owner": "test-engine", "python.version": "3.12.3"} - assert successful_test.tags == expected_tags + assert test_data.tags == expected_tags - json = successful_test.as_json(Instant.now()) + json = test_data.as_json(Instant.now()) assert json["tags"] == {"owner": "test-engine", "python.version": "3.12.3"} def test_test_data_tag_execution_non_string(self, successful_test):