diff --git a/src/buildkite_test_collector/collector/payload.py b/src/buildkite_test_collector/collector/payload.py index 6940c4b..dae5b03 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 @@ -40,19 +41,51 @@ 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'] + + 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 __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 + return + + if self.detail is None: + 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: + 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}") 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 @@ -75,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])) @@ -92,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: @@ -110,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 @@ -118,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, @@ -136,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") @@ -148,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()) @@ -176,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)) @@ -188,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: @@ -213,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", @@ -245,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])) @@ -253,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 + ) 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/collector/test_payload.py b/tests/buildkite_test_collector/collector/test_payload.py index c4c1ba3..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 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") @@ -188,3 +193,111 @@ 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 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", + duration=timedelta(seconds=1), + 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 48cf91d..42b0118 100644 --- a/tests/buildkite_test_collector/pytest_plugin/test_span_collector.py +++ b/tests/buildkite_test_collector/pytest_plugin/test_span_collector.py @@ -7,13 +7,14 @@ 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 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