From 0b54e454708f854b196317e47137252f3c7e4eca Mon Sep 17 00:00:00 2001 From: Jason Barnett Date: Sat, 31 May 2025 05:50:09 -0600 Subject: [PATCH 1/4] add --merge-json feature --- setup.py | 2 +- .../collector/payload.py | 4 +- .../pytest_plugin/__init__.py | 9 +- .../pytest_plugin/buildkite_plugin.py | 26 +++- .../pytest_plugin/test_plugin.py | 122 +++++++++++++++++- 5 files changed, 149 insertions(+), 14 deletions(-) diff --git a/setup.py b/setup.py index 60f4def..d68236d 100644 --- a/setup.py +++ b/setup.py @@ -21,7 +21,7 @@ zip_safe=False, package_dir={'': 'src'}, packages=find_packages(where='src'), - install_requires=["requests>=2", "pytest>=7"], + install_requires=["requests>=2", "pytest>=7", "filelock>=3"], extras_require={ "dev": [ "mock>=4", diff --git a/src/buildkite_test_collector/collector/payload.py b/src/buildkite_test_collector/collector/payload.py index a5a2284..2961154 100644 --- a/src/buildkite_test_collector/collector/payload.py +++ b/src/buildkite_test_collector/collector/payload.py @@ -77,7 +77,7 @@ class TestHistory: start_at: Optional[Instant] = None end_at: Optional[Instant] = None duration: Optional[timedelta] = None - children: Tuple['TestSpan'] = () + children: list['TestSpan'] = () def is_finished(self) -> bool: """Is there an end_at time present?""" @@ -91,7 +91,7 @@ def as_json(self, started_at: Instant) -> JsonDict: """Convert this trace into a Dict for eventual serialisation into JSON""" attrs = { "section": "top", - "children": tuple(map(lambda span: span.as_json(started_at), self.children)) + "children": list(map(lambda span: span.as_json(started_at), self.children)) } if self.start_at is not None: diff --git a/src/buildkite_test_collector/pytest_plugin/__init__.py b/src/buildkite_test_collector/pytest_plugin/__init__.py index 608db60..7684b11 100644 --- a/src/buildkite_test_collector/pytest_plugin/__init__.py +++ b/src/buildkite_test_collector/pytest_plugin/__init__.py @@ -59,7 +59,7 @@ def pytest_unconfigure(config): # Note that when xdist is used, this JSON output file will NOT contain tags. jsonpath = config.option.jsonpath if jsonpath: - plugin.save_payload_as_json(jsonpath) + plugin.save_payload_as_json(jsonpath, merge=config.option.mergejson) del config._buildkite config.pluginmanager.unregister(plugin) @@ -75,3 +75,10 @@ def pytest_addoption(parser): metavar="path", help='save json file at given path' ) + group.addoption( + '--merge-json', + default=False, + action='store_true', + dest="mergejson", + help='merge json output with existing file, if it exists' + ) diff --git a/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py b/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py index 21a2da3..d39a944 100644 --- a/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py +++ b/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py @@ -1,7 +1,10 @@ """Buildkite test collector plugin for Pytest""" import json +import os from uuid import uuid4 +from filelock import Timeout, FileLock + from ..collector.payload import TestData from .logger import logger @@ -108,7 +111,22 @@ def finalize_test(self, nodeid): return True return False - def save_payload_as_json(self, path): - """ Save payload into a json file """ - with open(path, "w", encoding="utf-8") as f: - json.dump(self.payload.as_json()["data"], f) + def save_payload_as_json(self, path, merge=False): + """Save payload into a json file, merging with existing data if merge is True""" + lock = FileLock(f"{path}.lock") + with lock: + data = list(self.payload.as_json()["data"]) + if merge and os.path.exists(path): + try: + with open(path, "r", encoding="utf-8") as f: + existing_data = json.load(f) + except json.JSONDecodeError: + existing_data = [] + # Merge existing data with current payload + merged_data = existing_data + data + else: + # If file does not exist or merge is False, use current payload + merged_data = data + + with open(path, "w", encoding="utf-8") as f: + json.dump(merged_data, f) diff --git a/tests/buildkite_test_collector/pytest_plugin/test_plugin.py b/tests/buildkite_test_collector/pytest_plugin/test_plugin.py index ccb60d6..c98272b 100644 --- a/tests/buildkite_test_collector/pytest_plugin/test_plugin.py +++ b/tests/buildkite_test_collector/pytest_plugin/test_plugin.py @@ -1,8 +1,8 @@ -from buildkite_test_collector.pytest_plugin import BuildkitePlugin +import json + from buildkite_test_collector.collector.payload import Payload -from pathlib import Path +from buildkite_test_collector.pytest_plugin import BuildkitePlugin -import json def test_runtest_logstart_with_unstarted_payload(fake_env): payload = Payload.init(fake_env) @@ -15,7 +15,28 @@ def test_runtest_logstart_with_unstarted_payload(fake_env): assert plugin.payload.started_at is not None -def test_save_json_payload(fake_env, tmp_path, successful_test): +def test_save_json_payload_without_merge(fake_env, tmp_path, successful_test): + payload = Payload.init(fake_env) + payload = Payload.started(payload) + payload = payload.push_test_data(successful_test) + + plugin = BuildkitePlugin(payload) + + path = tmp_path / "result.json" + + # Create an existing file with some data + existing_data = [{"existing": "data"}] + path.write_text(json.dumps(existing_data)) + + # Save without merge option + plugin.save_payload_as_json(path, merge=False) + + # Check if the data was not merged + expected_data = [successful_test.as_json(payload.started_at)] + assert json.loads(path.read_text()) == expected_data + + +def test_save_json_payload_with_merge(fake_env, tmp_path, successful_test): payload = Payload.init(fake_env) payload = Payload.started(payload) payload = payload.push_test_data(successful_test) @@ -23,6 +44,95 @@ def test_save_json_payload(fake_env, tmp_path, successful_test): plugin = BuildkitePlugin(payload) path = tmp_path / "result.json" - plugin.save_payload_as_json(path) - assert path.read_text() == json.dumps([successful_test.as_json(payload.started_at)]) + # Create an existing file with some data + existing_data = [{"existing": "data"}] + path.write_text(json.dumps(existing_data)) + + # Save with merge option + plugin.save_payload_as_json(path, merge=True) + + # Check if the data was merged + expected_data = existing_data + [successful_test.as_json(payload.started_at)] + assert json.loads(path.read_text()) == expected_data + + +def test_save_json_payload_with_non_existent_file(fake_env, tmp_path, successful_test): + payload = Payload.init(fake_env) + payload = Payload.started(payload) + payload = payload.push_test_data(successful_test) + + plugin = BuildkitePlugin(payload) + + path = tmp_path / "non_existent.json" + + # Ensure the file does not exist + assert not path.exists() + + # Save with merge option + plugin.save_payload_as_json(path, merge=True) + + # Check if the data was saved correctly + expected_data = [successful_test.as_json(payload.started_at)] + assert json.loads(path.read_text()) == expected_data + + +def test_save_json_payload_with_empty_file(fake_env, tmp_path, successful_test): + payload = Payload.init(fake_env) + payload = Payload.started(payload) + payload = payload.push_test_data(successful_test) + + plugin = BuildkitePlugin(payload) + + path = tmp_path / "empty.json" + + # Create an empty file + path.write_text("") + + # Save with merge option + plugin.save_payload_as_json(path, merge=True) + + # Check if the data was saved correctly + expected_data = [successful_test.as_json(payload.started_at)] + assert json.loads(path.read_text()) == expected_data + + +def test_save_json_payload_with_invalid_file(fake_env, tmp_path, successful_test): + payload = Payload.init(fake_env) + payload = Payload.started(payload) + payload = payload.push_test_data(successful_test) + + plugin = BuildkitePlugin(payload) + + path = tmp_path / "invalid.json" + + # Create a file with invalid JSON + path.write_text("{invalid: json}") + + # Save with merge option + plugin.save_payload_as_json(path, merge=True) + + # Check if the data was saved correctly + expected_data = [successful_test.as_json(payload.started_at)] + assert json.loads(path.read_text()) == expected_data + + +def test_save_json_payload_with_large_data(fake_env, tmp_path, successful_test): + payload = Payload.init(fake_env) + payload = Payload.started(payload) + payload = payload.push_test_data(successful_test) + + plugin = BuildkitePlugin(payload) + + path = tmp_path / "large_data.json" + + # Create an existing file with a large amount of data + existing_data = [{"test": f"data_{i}"} for i in range(1000)] + path.write_text(json.dumps(existing_data)) + + # Save with merge option + plugin.save_payload_as_json(path, merge=True) + + # Check if the data was merged correctly + expected_data = existing_data + [successful_test.as_json(payload.started_at)] + assert json.loads(path.read_text()) == expected_data From dfd322503f6314139dcffc819a76a725a591b9e8 Mon Sep 17 00:00:00 2001 From: Jason Barnett Date: Mon, 9 Jun 2025 15:37:17 -0600 Subject: [PATCH 2/4] refactor based on feedback from @nprizal --- .../pytest_plugin/buildkite_plugin.py | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py b/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py index d39a944..dccaf38 100644 --- a/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py +++ b/src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py @@ -3,7 +3,7 @@ import os from uuid import uuid4 -from filelock import Timeout, FileLock +from filelock import FileLock from ..collector.payload import TestData from .logger import logger @@ -113,20 +113,16 @@ def finalize_test(self, nodeid): def save_payload_as_json(self, path, merge=False): """Save payload into a json file, merging with existing data if merge is True""" - lock = FileLock(f"{path}.lock") - with lock: - data = list(self.payload.as_json()["data"]) - if merge and os.path.exists(path): - try: + data = list(self.payload.as_json()["data"]) + + if merge: + lock = FileLock(f"{path}.lock") + with lock: + if os.path.exists(path): with open(path, "r", encoding="utf-8") as f: existing_data = json.load(f) - except json.JSONDecodeError: - existing_data = [] - # Merge existing data with current payload - merged_data = existing_data + data - else: - # If file does not exist or merge is False, use current payload - merged_data = data - - with open(path, "w", encoding="utf-8") as f: - json.dump(merged_data, f) + # Merge existing data with current payload + data = existing_data + data + + with open(path, "w", encoding="utf-8") as f: + json.dump(data, f) From 1a56341b4cf9ffe044a83bbd651f41459d9f2dbe Mon Sep 17 00:00:00 2001 From: Jason Barnett Date: Mon, 9 Jun 2025 16:42:19 -0600 Subject: [PATCH 3/4] fix typing --- src/buildkite_test_collector/collector/payload.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/buildkite_test_collector/collector/payload.py b/src/buildkite_test_collector/collector/payload.py index 2961154..bab6223 100644 --- a/src/buildkite_test_collector/collector/payload.py +++ b/src/buildkite_test_collector/collector/payload.py @@ -1,7 +1,7 @@ """Buildkite Test Analytics payload""" from dataclasses import dataclass, replace, field -from typing import Dict, Tuple, Optional, Union, Literal +from typing import Dict, Tuple, Optional, Union, Literal, List from datetime import timedelta from uuid import UUID @@ -77,7 +77,7 @@ class TestHistory: start_at: Optional[Instant] = None end_at: Optional[Instant] = None duration: Optional[timedelta] = None - children: list['TestSpan'] = () + children: List['TestSpan'] = () def is_finished(self) -> bool: """Is there an end_at time present?""" From 47baa268bc1602de7c109e58443cf4ecbf07b30d Mon Sep 17 00:00:00 2001 From: Jason Barnett Date: Mon, 9 Jun 2025 16:45:23 -0600 Subject: [PATCH 4/4] fix unit tests --- .../pytest_plugin/test_plugin.py | 30 +++---------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/tests/buildkite_test_collector/pytest_plugin/test_plugin.py b/tests/buildkite_test_collector/pytest_plugin/test_plugin.py index c98272b..de9b04c 100644 --- a/tests/buildkite_test_collector/pytest_plugin/test_plugin.py +++ b/tests/buildkite_test_collector/pytest_plugin/test_plugin.py @@ -1,4 +1,5 @@ import json +import pytest from buildkite_test_collector.collector.payload import Payload from buildkite_test_collector.pytest_plugin import BuildkitePlugin @@ -77,26 +78,6 @@ def test_save_json_payload_with_non_existent_file(fake_env, tmp_path, successful assert json.loads(path.read_text()) == expected_data -def test_save_json_payload_with_empty_file(fake_env, tmp_path, successful_test): - payload = Payload.init(fake_env) - payload = Payload.started(payload) - payload = payload.push_test_data(successful_test) - - plugin = BuildkitePlugin(payload) - - path = tmp_path / "empty.json" - - # Create an empty file - path.write_text("") - - # Save with merge option - plugin.save_payload_as_json(path, merge=True) - - # Check if the data was saved correctly - expected_data = [successful_test.as_json(payload.started_at)] - assert json.loads(path.read_text()) == expected_data - - def test_save_json_payload_with_invalid_file(fake_env, tmp_path, successful_test): payload = Payload.init(fake_env) payload = Payload.started(payload) @@ -109,12 +90,9 @@ def test_save_json_payload_with_invalid_file(fake_env, tmp_path, successful_test # Create a file with invalid JSON path.write_text("{invalid: json}") - # Save with merge option - plugin.save_payload_as_json(path, merge=True) - - # Check if the data was saved correctly - expected_data = [successful_test.as_json(payload.started_at)] - assert json.loads(path.read_text()) == expected_data + # Save with merge option, expect JSONDecodeError + with pytest.raises(json.decoder.JSONDecodeError): + plugin.save_payload_as_json(path, merge=True) def test_save_json_payload_with_large_data(fake_env, tmp_path, successful_test):