From d3f1805146b90679f31fefbddf6176700d062aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Jendrysik?= Date: Fri, 21 Nov 2025 11:10:06 +0100 Subject: [PATCH 1/7] Fix span detail formatting for Test Engine UI Format spans in format expected by Test Engine. --- .../collector/payload.py | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/buildkite_test_collector/collector/payload.py b/src/buildkite_test_collector/collector/payload.py index 6940c4b..f0de73d 100644 --- a/src/buildkite_test_collector/collector/payload.py +++ b/src/buildkite_test_collector/collector/payload.py @@ -55,7 +55,30 @@ def as_json(self, started_at: Instant) -> JsonDict: } if self.detail is not None: - attrs["detail"] = self.detail + # Format detail based on section type to match the expected Avro schema + # See: https://buildkite.com/docs/test-analytics/importing-json#json-test-results-data-reference-span-objects + if self.section == "sql": + attrs["detail"] = {"query": self.detail} + elif self.section == "annotation": + attrs["detail"] = {"content": self.detail} + elif self.section == "http": + # For HTTP spans, the detail should be a dict with method, url, lib + # If it's a string, handle it gracefully + if isinstance(self.detail, dict): + attrs["detail"] = self.detail + else: + # Fallback: if provided a string, treat it as a URL + attrs["detail"] = { + "method": "GET", + "url": self.detail, + "lib": "unknown" + } + elif self.section == "sleep": + # Sleep spans don't need detail + pass + else: + # For unknown/custom sections, keep detail as-is + attrs["detail"] = self.detail if self.start_at is not None: attrs["start_at"] = (self.start_at - started_at).total_seconds() From a03b99b97cc0be6b115a5e0fbfdd05cbd339afd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Jendrysik?= Date: Fri, 21 Nov 2025 11:44:56 +0100 Subject: [PATCH 2/7] Disable pylint long line check for doc link --- src/buildkite_test_collector/collector/payload.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/buildkite_test_collector/collector/payload.py b/src/buildkite_test_collector/collector/payload.py index f0de73d..c917184 100644 --- a/src/buildkite_test_collector/collector/payload.py +++ b/src/buildkite_test_collector/collector/payload.py @@ -56,6 +56,7 @@ def as_json(self, started_at: Instant) -> JsonDict: if self.detail is not None: # Format detail based on section type to match the expected Avro schema + # pylint: disable=C0301 # See: https://buildkite.com/docs/test-analytics/importing-json#json-test-results-data-reference-span-objects if self.section == "sql": attrs["detail"] = {"query": self.detail} From 0ea0fa416d155ed65098abaa1f310040417587bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Jendrysik?= Date: Tue, 25 Nov 2025 09:36:32 +0100 Subject: [PATCH 3/7] Change details type to dict Align type with Test Engine, simplify implementation. Make pylint happy --- .../collector/payload.py | 36 ++++++------------- .../pytest_plugin/span_collector.py | 15 ++++++-- .../pytest_plugin/test_span_collector.py | 3 +- 3 files changed, 25 insertions(+), 29 deletions(-) diff --git a/src/buildkite_test_collector/collector/payload.py b/src/buildkite_test_collector/collector/payload.py index c917184..9e8ac34 100644 --- a/src/buildkite_test_collector/collector/payload.py +++ b/src/buildkite_test_collector/collector/payload.py @@ -40,12 +40,20 @@ class TestSpan: Buildkite Test Analtics supports some basic tracing to allow insight into the runtime performance your tests. + + The detail field should be a dict matching the expected format for each section type: + - sql: {"query": str} + - annotation: {"content": str} + - http: {"method": str, "url": str, "lib": str} + - sleep: no detail required + + See: https://buildkite.com/docs/test-engine/test-collection/importing-json#json-test-results-data-reference-detail-objects # pylint: disable=C0301 """ section: Literal['http', 'sql', 'sleep', 'annotation'] duration: timedelta start_at: Optional[Instant] = None end_at: Optional[Instant] = None - detail: Optional[str] = None + detail: Optional[Dict[str, str]] = None def as_json(self, started_at: Instant) -> JsonDict: """Convert this span into a Dict for eventual serialisation into JSON""" @@ -55,31 +63,7 @@ def as_json(self, started_at: Instant) -> JsonDict: } if self.detail is not None: - # Format detail based on section type to match the expected Avro schema - # pylint: disable=C0301 - # See: https://buildkite.com/docs/test-analytics/importing-json#json-test-results-data-reference-span-objects - if self.section == "sql": - attrs["detail"] = {"query": self.detail} - elif self.section == "annotation": - attrs["detail"] = {"content": self.detail} - elif self.section == "http": - # For HTTP spans, the detail should be a dict with method, url, lib - # If it's a string, handle it gracefully - if isinstance(self.detail, dict): - attrs["detail"] = self.detail - else: - # Fallback: if provided a string, treat it as a URL - attrs["detail"] = { - "method": "GET", - "url": self.detail, - "lib": "unknown" - } - elif self.section == "sleep": - # Sleep spans don't need detail - pass - else: - # For unknown/custom sections, keep detail as-is - attrs["detail"] = self.detail + attrs["detail"] = self.detail if self.start_at is not None: attrs["start_at"] = (self.start_at - started_at).total_seconds() diff --git a/src/buildkite_test_collector/pytest_plugin/span_collector.py b/src/buildkite_test_collector/pytest_plugin/span_collector.py index cfc81ab..0e6e3cc 100644 --- a/src/buildkite_test_collector/pytest_plugin/span_collector.py +++ b/src/buildkite_test_collector/pytest_plugin/span_collector.py @@ -32,17 +32,28 @@ def record(self, span: TestSpan) -> None: @contextmanager def measure(self, section: Literal['http', 'sql', 'sleep', 'annotation'], - detail: Optional[str] = None) -> Any: + detail: Optional[dict] = None) -> Any: """ Measure the execution time of some code and record it as a span. + The detail parameter should be a dict matching the expected format for each section type: + - sql: {"query": str} + - annotation: {"content": str} + - http: {"method": str, "url": str, "lib": str} + - sleep: no detail required + Example: .. code-block:: python def test_measure_http_request(spans): - with spans.measure('http', 'The koan of Github'): + detail = {'method': 'GET', 'url': 'https://api.github.com/zen', 'lib': 'requests'} + with spans.measure('http', detail): requests.get("https://api.github.com/zen") + + def test_measure_sql_query(spans): + with spans.measure('sql', {'query': 'SELECT * FROM users'}): + db.execute('SELECT * FROM users') """ start_at = Instant.now() try: diff --git a/tests/buildkite_test_collector/pytest_plugin/test_span_collector.py b/tests/buildkite_test_collector/pytest_plugin/test_span_collector.py index 48cf91d..6390b58 100644 --- a/tests/buildkite_test_collector/pytest_plugin/test_span_collector.py +++ b/tests/buildkite_test_collector/pytest_plugin/test_span_collector.py @@ -7,7 +7,8 @@ def test_record_adds_span_to_plugin(span_collector): span_collector.record(TestSpan( section='http', - duration=timedelta(seconds=3))) + duration=timedelta(seconds=3), + detail={'method': 'GET', 'url': 'https://example.com', 'lib': 'requests'})) assert len(span_collector.current_test().history.children) == 1 From 9412cdf1ea87b723a184e4ca6910c3a042215ebf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Jendrysik?= Date: Wed, 26 Nov 2025 14:39:26 +0100 Subject: [PATCH 4/7] Add validation for span payload --- .../collector/payload.py | 23 ++++ .../collector/test_payload.py | 104 +++++++++++++++++- 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/src/buildkite_test_collector/collector/payload.py b/src/buildkite_test_collector/collector/payload.py index 9e8ac34..2381907 100644 --- a/src/buildkite_test_collector/collector/payload.py +++ b/src/buildkite_test_collector/collector/payload.py @@ -55,6 +55,29 @@ class TestSpan: end_at: Optional[Instant] = None detail: Optional[Dict[str, str]] = None + def __post_init__(self): + """Validate detail structure matches the section type requirements""" + if self.detail is None: + return + + if not isinstance(self.detail, dict): + raise TypeError(f"detail must be a dict, got {type(self.detail).__name__}") + + if self.section == 'sql': + if 'query' not in self.detail: + raise ValueError("SQL span detail must contain 'query' field") + elif self.section == 'annotation': + if 'content' not in self.detail: + raise ValueError("Annotation span detail must contain 'content' field") + elif self.section == 'http': + required = {'method', 'url', 'lib'} + missing = required - set(self.detail.keys()) + if missing: + raise ValueError(f"HTTP span detail missing required fields: {missing}") + elif self.section == 'sleep': + # Sleep spans don't need detail, but if provided we'll allow it + pass + def as_json(self, started_at: Instant) -> JsonDict: """Convert this span into a Dict for eventual serialisation into JSON""" attrs = { diff --git a/tests/buildkite_test_collector/collector/test_payload.py b/tests/buildkite_test_collector/collector/test_payload.py index c4c1ba3..9b9300c 100644 --- a/tests/buildkite_test_collector/collector/test_payload.py +++ b/tests/buildkite_test_collector/collector/test_payload.py @@ -3,7 +3,7 @@ import pytest -from buildkite_test_collector.collector.payload import Payload, TestHistory, TestData, TestResultFailed, TestResultPassed, TestResultSkipped +from buildkite_test_collector.collector.payload import Payload, TestHistory, TestData, TestResultFailed, TestResultPassed, TestResultSkipped, TestSpan from buildkite_test_collector.collector.instant import Instant @@ -188,3 +188,105 @@ def test_test_data_tag_execution_non_string(self, successful_test): with pytest.raises(TypeError): successful_test.tag_execution(777, "lucky") + + +class TestSpanValidation: + """Tests for TestSpan detail validation""" + + def test_sql_span_with_valid_detail(self): + """SQL span with correct detail structure should succeed""" + span = TestSpan( + section='sql', + duration=timedelta(seconds=1), + detail={'query': 'SELECT * FROM users'} + ) + assert span.detail == {'query': 'SELECT * FROM users'} + + def test_sql_span_without_query_field_fails(self): + """SQL span without 'query' field should raise ValueError""" + with pytest.raises(ValueError, match="SQL span detail must contain 'query' field"): + TestSpan( + section='sql', + duration=timedelta(seconds=1), + detail={'wrong_field': 'SELECT * FROM users'} + ) + + def test_annotation_span_with_valid_detail(self): + """Annotation span with correct detail structure should succeed""" + span = TestSpan( + section='annotation', + duration=timedelta(seconds=1), + detail={'content': 'Test annotation'} + ) + assert span.detail == {'content': 'Test annotation'} + + def test_annotation_span_without_content_field_fails(self): + """Annotation span without 'content' field should raise ValueError""" + with pytest.raises(ValueError, match="Annotation span detail must contain 'content' field"): + TestSpan( + section='annotation', + duration=timedelta(seconds=1), + detail={'wrong_field': 'Test annotation'} + ) + + def test_http_span_with_valid_detail(self): + """HTTP span with all required fields should succeed""" + span = TestSpan( + section='http', + duration=timedelta(seconds=1), + detail={'method': 'GET', 'url': 'https://example.com', 'lib': 'requests'} + ) + assert span.detail == {'method': 'GET', 'url': 'https://example.com', 'lib': 'requests'} + + def test_http_span_missing_method_fails(self): + """HTTP span missing 'method' field should raise ValueError""" + with pytest.raises(ValueError, match="HTTP span detail missing required fields"): + TestSpan( + section='http', + duration=timedelta(seconds=1), + detail={'url': 'https://example.com', 'lib': 'requests'} + ) + + def test_http_span_missing_multiple_fields_fails(self): + """HTTP span missing multiple fields should raise ValueError""" + with pytest.raises(ValueError, match="HTTP span detail missing required fields"): + TestSpan( + section='http', + duration=timedelta(seconds=1), + detail={'method': 'GET'} + ) + + def test_sleep_span_without_detail(self): + """Sleep span without detail should succeed""" + span = TestSpan( + section='sleep', + duration=timedelta(seconds=1) + ) + assert span.detail is None + + def test_sleep_span_with_detail_is_allowed(self): + """Sleep span with detail (though not required) should be allowed""" + span = TestSpan( + section='sleep', + duration=timedelta(seconds=1), + detail={'reason': 'rate limiting'} + ) + assert span.detail == {'reason': 'rate limiting'} + + def test_span_with_none_detail(self): + """Span with None detail should succeed""" + span = TestSpan( + section='sql', + duration=timedelta(seconds=1), + detail=None + ) + assert span.detail is None + + def test_span_with_string_detail_fails(self): + """Span with string instead of dict should raise TypeError""" + with pytest.raises(TypeError, match="detail must be a dict, got str"): + TestSpan( + section='sql', + duration=timedelta(seconds=1), + detail='SELECT * FROM users' + ) From 7205afd1800b2c4f168724ced59d1724957bcb8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Jendrysik?= <10155318+scadu@users.noreply.github.com> Date: Thu, 27 Nov 2025 08:47:28 +0100 Subject: [PATCH 5/7] Omit sleep in details validation Co-authored-by: Naufan P. Rizal --- src/buildkite_test_collector/collector/payload.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/buildkite_test_collector/collector/payload.py b/src/buildkite_test_collector/collector/payload.py index 2381907..4a26766 100644 --- a/src/buildkite_test_collector/collector/payload.py +++ b/src/buildkite_test_collector/collector/payload.py @@ -57,8 +57,12 @@ class TestSpan: def __post_init__(self): """Validate detail structure matches the section type requirements""" + if self.section == 'sleep' + # Sleep spans don't need detail, so no validation is required + pass + if self.detail is None: - return + raise TypeError(f"detail is requred for 'sql', 'annotation' and 'http' spans") if not isinstance(self.detail, dict): raise TypeError(f"detail must be a dict, got {type(self.detail).__name__}") From ebdb129698d8ff93fc28042f0c43d44e62445477 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Jendrysik?= Date: Thu, 27 Nov 2025 08:49:35 +0100 Subject: [PATCH 6/7] Remove sleep --- .../collector/payload.py | 117 +++++++++--------- 1 file changed, 58 insertions(+), 59 deletions(-) diff --git a/src/buildkite_test_collector/collector/payload.py b/src/buildkite_test_collector/collector/payload.py index 4a26766..dbe05c3 100644 --- a/src/buildkite_test_collector/collector/payload.py +++ b/src/buildkite_test_collector/collector/payload.py @@ -10,7 +10,7 @@ from .instant import Instant from .run_env import RunEnv -JsonValue = Union[str, int, float, bool, 'JsonDict', Tuple['JsonValue']] +JsonValue = Union[str, int, float, bool, "JsonDict", Tuple["JsonValue"]] JsonDict = Dict[str, JsonValue] # pylint: disable=C0103 disable=W0622 disable=R0913 @@ -24,6 +24,7 @@ class TestResultPassed: @dataclass(frozen=True) class TestResultFailed: """Represents a failed test result""" + failure_reason: Optional[str] failure_expanded: Optional[Iterable[Mapping[str, Iterable[str]]]] = None @@ -49,7 +50,8 @@ class TestSpan: See: https://buildkite.com/docs/test-engine/test-collection/importing-json#json-test-results-data-reference-detail-objects # pylint: disable=C0301 """ - section: Literal['http', 'sql', 'sleep', 'annotation'] + + section: Literal["http", "sql", "sleep", "annotation"] duration: timedelta start_at: Optional[Instant] = None end_at: Optional[Instant] = None @@ -57,37 +59,33 @@ class TestSpan: def __post_init__(self): """Validate detail structure matches the section type requirements""" - if self.section == 'sleep' + if self.section == "sleep": # Sleep spans don't need detail, so no validation is required pass if self.detail is None: - raise TypeError(f"detail is requred for 'sql', 'annotation' and 'http' spans") + raise TypeError( + "detail is requred for 'sql', 'annotation' and 'http' spans" + ) if not isinstance(self.detail, dict): raise TypeError(f"detail must be a dict, got {type(self.detail).__name__}") - if self.section == 'sql': - if 'query' not in self.detail: + if self.section == "sql": + if "query" not in self.detail: raise ValueError("SQL span detail must contain 'query' field") - elif self.section == 'annotation': - if 'content' not in self.detail: + elif self.section == "annotation": + if "content" not in self.detail: raise ValueError("Annotation span detail must contain 'content' field") - elif self.section == 'http': - required = {'method', 'url', 'lib'} + elif self.section == "http": + required = {"method", "url", "lib"} missing = required - set(self.detail.keys()) if missing: raise ValueError(f"HTTP span detail missing required fields: {missing}") - elif self.section == 'sleep': - # Sleep spans don't need detail, but if provided we'll allow it - pass def as_json(self, started_at: Instant) -> JsonDict: """Convert this span into a Dict for eventual serialisation into JSON""" - attrs = { - "section": self.section, - "duration": self.duration.total_seconds() - } + attrs = {"section": self.section, "duration": self.duration.total_seconds()} if self.detail is not None: attrs["detail"] = self.detail @@ -110,16 +108,17 @@ class TestHistory: the runtime performance your tests. This object is the top-level of that tracing tree. """ + 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?""" return self.end_at is not None - def push_span(self, span: TestSpan) -> 'TestHistory': + def push_span(self, span: TestSpan) -> "TestHistory": """Add a new span to the children""" return replace(self, children=self.children + tuple([span])) @@ -127,7 +126,7 @@ def as_json(self, started_at: Instant) -> JsonDict: """Convert this trace into a Dict for eventual serialisation into JSON""" attrs = { "section": "top", - "children": list(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: @@ -145,6 +144,7 @@ def as_json(self, started_at: Instant) -> JsonDict: @dataclass(frozen=True) class TestData: """An individual test execution""" + # 8 attributes for this class seems reasonable # pylint: disable=too-many-instance-attributes id: UUID @@ -153,17 +153,19 @@ class TestData: history: TestHistory location: Optional[str] = None file_name: Optional[str] = None - tags: Dict[str,str] = field(default_factory=dict) - result: Union[TestResultPassed, TestResultFailed, - TestResultSkipped, None] = None + tags: Dict[str, str] = field(default_factory=dict) + result: Union[TestResultPassed, TestResultFailed, TestResultSkipped, None] = None @classmethod - def start(cls, id: UUID, - *, - scope: str, - name: str, - location: Optional[str] = None, - file_name: Optional[str] = None) -> 'TestData': + def start( + cls, + id: UUID, + *, + scope: str, + name: str, + location: Optional[str] = None, + file_name: Optional[str] = None, + ) -> "TestData": """Build a new instance with it's start_at time set to now""" return cls( id=id, @@ -171,10 +173,10 @@ def start(cls, id: UUID, name=name, location=location, file_name=file_name, - history=TestHistory(start_at=Instant.now()) + history=TestHistory(start_at=Instant.now()), ) - def tag_execution(self, key: str, val: str) -> 'TestData': + def tag_execution(self, key: str, val: str) -> "TestData": """Set tag to test execution""" if not isinstance(key, str) or not isinstance(val, str): raise TypeError("Expected string for key and value") @@ -183,27 +185,29 @@ def tag_execution(self, key: str, val: str) -> 'TestData': new_tags[key] = val return replace(self, tags=new_tags) - def finish(self) -> 'TestData': + def finish(self) -> "TestData": """Set the end_at and duration on this test""" if self.is_finished(): return self end_at = Instant.now() duration = end_at - self.history.start_at - return replace(self, history=replace(self.history, - end_at=end_at, - duration=duration)) + return replace( + self, history=replace(self.history, end_at=end_at, duration=duration) + ) - def passed(self) -> 'TestData': + def passed(self) -> "TestData": """Mark this test as passed""" return replace(self, result=TestResultPassed()) - def failed(self, failure_reason=None, failure_expanded=None) -> 'TestData': + def failed(self, failure_reason=None, failure_expanded=None) -> "TestData": """Mark this test as failed""" - result = TestResultFailed(failure_reason=failure_reason, failure_expanded=failure_expanded) + result = TestResultFailed( + failure_reason=failure_reason, failure_expanded=failure_expanded + ) return replace(self, result=result) - def skipped(self) -> 'TestData': + def skipped(self) -> "TestData": """Mark this test as skipped""" return replace(self, result=TestResultSkipped()) @@ -211,7 +215,7 @@ def is_finished(self) -> bool: """Does this test have an end_at time?""" return self.history and self.history.is_finished() - def push_span(self, span: TestSpan) -> 'TestData': + def push_span(self, span: TestSpan) -> "TestData": """Add a span to the test history""" return replace(self, history=self.history.push_span(span)) @@ -223,7 +227,7 @@ def as_json(self, started_at: Instant) -> JsonDict: "name": self.name, "location": self.location, "file_name": self.file_name, - "history": self.history.as_json(started_at) + "history": self.history.as_json(started_at), } if len(self.tags) > 0: @@ -248,31 +252,26 @@ def as_json(self, started_at: Instant) -> JsonDict: @dataclass(frozen=True) class Payload: """The full test analytics payload""" + run_env: RunEnv data: Tuple[TestData] started_at: Optional[Instant] finished_at: Optional[Instant] @classmethod - def init(cls, run_env: RunEnv) -> 'Payload': + def init(cls, run_env: RunEnv) -> "Payload": """Create a new instance of payload with the provided RunEnv""" - return cls( - run_env=run_env, - data=(), - started_at=None, - finished_at=None - ) + return cls(run_env=run_env, data=(), started_at=None, finished_at=None) def as_json(self) -> JsonDict: """Convert into a Dict suitable for eventual serialisation to JSON""" - finished_data = list(filter( - lambda td: td.is_finished(), - self.data - )) + finished_data = list(filter(lambda td: td.is_finished(), self.data)) if len(finished_data) < len(self.data): - logger.warning('Unexpected unfinished test data, skipping unfinished test records...') + logger.warning( + "Unexpected unfinished test data, skipping unfinished test records..." + ) return { "format": "json", @@ -280,7 +279,7 @@ def as_json(self) -> JsonDict: "data": tuple(map(lambda td: td.as_json(self.started_at), finished_data)), } - def push_test_data(self, report: TestData) -> 'Payload': + def push_test_data(self, report: TestData) -> "Payload": """Append a test-data to the payload""" return replace(self, data=self.data + tuple([report])) @@ -288,20 +287,20 @@ def is_started(self) -> bool: """Returns true of the payload has been started""" return self.started_at is not None - def started(self) -> 'Payload': + def started(self) -> "Payload": """Mark the payload as started (ie the suite has started)""" return replace(self, started_at=Instant.now()) - def into_batches(self, batch_size=100) -> Tuple['Payload']: + def into_batches(self, batch_size=100) -> Tuple["Payload"]: """Convert the payload into a collection of payloads based on the batch size""" return self.__into_batches(self.data, tuple(), batch_size) - def __into_batches(self, data, batches, batch_size) -> Tuple['Payload']: + def __into_batches(self, data, batches, batch_size) -> Tuple["Payload"]: if len(data) <= batch_size: return batches + tuple([replace(self, data=data)]) next_batch = data[0:batch_size] next_data = data[batch_size:] return self.__into_batches( - next_data, - batches + tuple([replace(self, data=next_batch)]), batch_size) + next_data, batches + tuple([replace(self, data=next_batch)]), batch_size + ) From fa1952e762815ea2451a5a988770eb9bc6477209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Jendrysik?= Date: Thu, 27 Nov 2025 09:08:58 +0100 Subject: [PATCH 7/7] Fix tests --- .../collector/payload.py | 2 +- .../collector/test_payload.py | 133 ++++++++++-------- .../pytest_plugin/test_span_collector.py | 2 +- 3 files changed, 74 insertions(+), 63 deletions(-) diff --git a/src/buildkite_test_collector/collector/payload.py b/src/buildkite_test_collector/collector/payload.py index dbe05c3..dae5b03 100644 --- a/src/buildkite_test_collector/collector/payload.py +++ b/src/buildkite_test_collector/collector/payload.py @@ -61,7 +61,7 @@ def __post_init__(self): """Validate detail structure matches the section type requirements""" if self.section == "sleep": # Sleep spans don't need detail, so no validation is required - pass + return if self.detail is None: raise TypeError( diff --git a/tests/buildkite_test_collector/collector/test_payload.py b/tests/buildkite_test_collector/collector/test_payload.py index 9b9300c..bf87b5b 100644 --- a/tests/buildkite_test_collector/collector/test_payload.py +++ b/tests/buildkite_test_collector/collector/test_payload.py @@ -3,8 +3,16 @@ import pytest -from buildkite_test_collector.collector.payload import Payload, TestHistory, TestData, TestResultFailed, TestResultPassed, TestResultSkipped, TestSpan from buildkite_test_collector.collector.instant import Instant +from buildkite_test_collector.collector.payload import ( + Payload, + TestData, + TestHistory, + TestResultFailed, + TestResultPassed, + TestResultSkipped, + TestSpan, +) def test_payload_init_has_empty_data(fake_env): @@ -24,8 +32,9 @@ def test_payload_started_sets_started_at_time(fake_env): def test_payload_into_batches_works_as_advertised(payload, successful_test): - payload = reduce(lambda p, _: p.push_test_data( - successful_test), range(100), payload) + payload = reduce( + lambda p, _: p.push_test_data(successful_test), range(100), payload + ) payloads = payload.into_batches(33) @@ -57,10 +66,7 @@ def test_payload_as_json(payload, successful_test): def test_test_history_with_no_end_at_is_not_finished(): - hist = TestHistory( - start_at=Instant.now(), - end_at=None, - duration=None) + hist = TestHistory(start_at=Instant.now(), end_at=None, duration=None) assert hist.is_finished() is not True @@ -70,10 +76,7 @@ def test_test_history_with_end_at_is_finished(): duration = timedelta(minutes=2, seconds=18) end_at = start_at + duration - hist = TestHistory( - start_at=start_at, - end_at=end_at, - duration=duration) + hist = TestHistory(start_at=start_at, end_at=end_at, duration=duration) assert hist.is_finished() is True @@ -84,10 +87,7 @@ def test_test_history_as_json(): duration = timedelta(minutes=2, seconds=18) end_at = start_at + duration - hist = TestHistory( - start_at=start_at, - end_at=end_at, - duration=duration) + hist = TestHistory(start_at=start_at, end_at=end_at, duration=duration) json = hist.as_json(now) @@ -99,13 +99,16 @@ def test_test_history_as_json(): def test_test_data_start(successful_test): - test_data = TestData.start(id=successful_test.id, - scope=successful_test.scope, - name=successful_test.name, - location=successful_test.location) + test_data = TestData.start( + id=successful_test.id, + scope=successful_test.scope, + name=successful_test.name, + location=successful_test.location, + ) assert test_data.history.start_at.seconds == pytest.approx( - Instant.now().seconds, 1.0) + Instant.now().seconds, 1.0 + ) def test_test_data_finish_when_already_finished_is_a_noop(successful_test): @@ -115,8 +118,7 @@ def test_test_data_finish_when_already_finished_is_a_noop(successful_test): def test_test_data_finish(incomplete_test): test_data = incomplete_test.finish() - assert test_data.history.end_at.seconds == pytest.approx( - Instant.now().seconds, 1.0) + assert test_data.history.end_at.seconds == pytest.approx(Instant.now().seconds, 1.0) assert test_data.history.duration.total_seconds() == pytest.approx(0, abs=0.5) @@ -162,7 +164,9 @@ def test_test_data_as_json_when_failed(failed_test): assert json["result"] == "failed" assert json["failure_reason"] == "bogus" - assert json["failure_expanded"] == [{'expanded': ['test failed'], 'backtrace': ['test.py:1']}] + assert json["failure_expanded"] == [ + {"expanded": ["test failed"], "backtrace": ["test.py:1"]} + ] def test_test_data_as_json_when_skipped(skipped_test): @@ -170,6 +174,7 @@ def test_test_data_as_json_when_skipped(skipped_test): assert json["result"] == "skipped" + class TestTestDataTagExecution: def test_test_data_tag_execution(self, successful_test): test_data = successful_test.tag_execution("owner", "test-engine") @@ -196,97 +201,103 @@ class TestSpanValidation: def test_sql_span_with_valid_detail(self): """SQL span with correct detail structure should succeed""" span = TestSpan( - section='sql', + section="sql", duration=timedelta(seconds=1), - detail={'query': 'SELECT * FROM users'} + detail={"query": "SELECT * FROM users"}, ) - assert span.detail == {'query': 'SELECT * FROM users'} + assert span.detail == {"query": "SELECT * FROM users"} def test_sql_span_without_query_field_fails(self): """SQL span without 'query' field should raise ValueError""" - with pytest.raises(ValueError, match="SQL span detail must contain 'query' field"): + with pytest.raises( + ValueError, match="SQL span detail must contain 'query' field" + ): TestSpan( - section='sql', + section="sql", duration=timedelta(seconds=1), - detail={'wrong_field': 'SELECT * FROM users'} + detail={"wrong_field": "SELECT * FROM users"}, ) def test_annotation_span_with_valid_detail(self): """Annotation span with correct detail structure should succeed""" span = TestSpan( - section='annotation', + section="annotation", duration=timedelta(seconds=1), - detail={'content': 'Test annotation'} + detail={"content": "Test annotation"}, ) - assert span.detail == {'content': 'Test annotation'} + assert span.detail == {"content": "Test annotation"} def test_annotation_span_without_content_field_fails(self): """Annotation span without 'content' field should raise ValueError""" - with pytest.raises(ValueError, match="Annotation span detail must contain 'content' field"): + with pytest.raises( + ValueError, match="Annotation span detail must contain 'content' field" + ): TestSpan( - section='annotation', + section="annotation", duration=timedelta(seconds=1), - detail={'wrong_field': 'Test annotation'} + detail={"wrong_field": "Test annotation"}, ) def test_http_span_with_valid_detail(self): """HTTP span with all required fields should succeed""" span = TestSpan( - section='http', + section="http", duration=timedelta(seconds=1), - detail={'method': 'GET', 'url': 'https://example.com', 'lib': 'requests'} + detail={"method": "GET", "url": "https://example.com", "lib": "requests"}, ) - assert span.detail == {'method': 'GET', 'url': 'https://example.com', 'lib': 'requests'} + assert span.detail == { + "method": "GET", + "url": "https://example.com", + "lib": "requests", + } def test_http_span_missing_method_fails(self): """HTTP span missing 'method' field should raise ValueError""" - with pytest.raises(ValueError, match="HTTP span detail missing required fields"): + with pytest.raises( + ValueError, match="HTTP span detail missing required fields" + ): TestSpan( - section='http', + section="http", duration=timedelta(seconds=1), - detail={'url': 'https://example.com', 'lib': 'requests'} + detail={"url": "https://example.com", "lib": "requests"}, ) def test_http_span_missing_multiple_fields_fails(self): """HTTP span missing multiple fields should raise ValueError""" - with pytest.raises(ValueError, match="HTTP span detail missing required fields"): + with pytest.raises( + ValueError, match="HTTP span detail missing required fields" + ): TestSpan( - section='http', - duration=timedelta(seconds=1), - detail={'method': 'GET'} + section="http", duration=timedelta(seconds=1), detail={"method": "GET"} ) def test_sleep_span_without_detail(self): """Sleep span without detail should succeed""" - span = TestSpan( - section='sleep', - duration=timedelta(seconds=1) - ) + span = TestSpan(section="sleep", duration=timedelta(seconds=1)) assert span.detail is None def test_sleep_span_with_detail_is_allowed(self): """Sleep span with detail (though not required) should be allowed""" span = TestSpan( - section='sleep', + section="sleep", duration=timedelta(seconds=1), - detail={'reason': 'rate limiting'} + detail={"reason": "rate limiting"}, ) - assert span.detail == {'reason': 'rate limiting'} + assert span.detail == {"reason": "rate limiting"} def test_span_with_none_detail(self): - """Span with None detail should succeed""" - span = TestSpan( - section='sql', - duration=timedelta(seconds=1), - detail=None - ) - assert span.detail is None + """Span with None detail should raise TypeError for non-sleep sections""" + with pytest.raises( + TypeError, + match="detail is requred for 'sql', 'annotation' and 'http' spans", + ): + TestSpan(section="sql", duration=timedelta(seconds=1), detail=None) def test_span_with_string_detail_fails(self): """Span with string instead of dict should raise TypeError""" with pytest.raises(TypeError, match="detail must be a dict, got str"): TestSpan( - section='sql', + section="sql", duration=timedelta(seconds=1), - detail='SELECT * FROM users' + detail="SELECT * FROM users", ) diff --git a/tests/buildkite_test_collector/pytest_plugin/test_span_collector.py b/tests/buildkite_test_collector/pytest_plugin/test_span_collector.py index 6390b58..42b0118 100644 --- a/tests/buildkite_test_collector/pytest_plugin/test_span_collector.py +++ b/tests/buildkite_test_collector/pytest_plugin/test_span_collector.py @@ -14,7 +14,7 @@ def test_record_adds_span_to_plugin(span_collector): def test_measure_adds_span_to_plugin(span_collector): - with span_collector.measure('annotation'): + with span_collector.measure('annotation', {'content': 'test annotation'}): time.sleep(0.001) assert len(span_collector.current_test().history.children) == 1