From 05c20847c0dbaccd10f900bd18deafc0c75e77e8 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Thu, 14 May 2026 14:30:31 -0700 Subject: [PATCH] add step metadata support --- .../low_level_wrappers/test_results.py | 4 ++ .../_tests/resources/test_test_results.py | 8 ++- .../_tests/sift_types/test_results.py | 56 +++++++++++++++++++ .../_tests/util/test_test_results_utils.py | 12 +++- .../lib/sift_client/sift_types/test_report.py | 15 +++++ .../util/test_results/context_manager.py | 30 ++++++++-- 6 files changed, 118 insertions(+), 7 deletions(-) diff --git a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py index b98670208..d15f86c48 100644 --- a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py +++ b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py @@ -203,6 +203,7 @@ def simulate_update_test_step_response( from datetime import timezone from sift_client.sift_types.test_report import ErrorInfo, TestStatus + from sift_client.util.metadata import metadata_proto_to_dict update_mask_paths = set(request.update_mask.paths) proto = request.test_step @@ -226,6 +227,8 @@ def simulate_update_test_step_response( ) else: updates["error_info"] = None + if "metadata" in update_mask_paths: + updates["metadata"] = metadata_proto_to_dict(proto.metadata) if proto.metadata else None # type: ignore[arg-type] return existing.model_copy(update=updates) @@ -1247,6 +1250,7 @@ def _step_create_from_simulated( parent_step_id=real_parent_step_id, description=simulated.description, error_info=simulated.error_info, + metadata=simulated.metadata, ) @staticmethod diff --git a/python/lib/sift_client/_tests/resources/test_test_results.py b/python/lib/sift_client/_tests/resources/test_test_results.py index 1600b2e10..d0ccf4d1b 100644 --- a/python/lib/sift_client/_tests/resources/test_test_results.py +++ b/python/lib/sift_client/_tests/resources/test_test_results.py @@ -53,6 +53,7 @@ def compare_test_step_fields(simulated: TestStep, actual: TestStep) -> None: assert simulated.status == actual.status assert simulated.start_time == actual.start_time assert simulated.end_time == actual.end_time + assert simulated.metadata == actual.metadata def compare_test_measurement_fields(simulated: TestMeasurement, actual: TestMeasurement) -> None: @@ -135,6 +136,7 @@ def test_create_test_steps(self, sift_client, tmp_path): status=TestStatus.PASSED, start_time=simulated_time, end_time=simulated_time + timedelta(seconds=10), + metadata={"phase": "init", "iteration": 1}, ) # Create simulated step first @@ -256,10 +258,14 @@ def test_update_test_steps(self, sift_client, tmp_path): # Update the step using class function. step3_1 = step3_1.update( - {"description": "Error demo w/ updated description"}, + { + "description": "Error demo w/ updated description", + "metadata": {"phase": "validation", "retry": 2}, + }, ) assert step3.status == TestStatus.PASSED assert step3_1.description == "Error demo w/ updated description" + assert step3_1.metadata == {"phase": "validation", "retry": 2} def test_create_test_measurements(self, sift_client, tmp_path): step1 = self.test_steps.get("step1") diff --git a/python/lib/sift_client/_tests/sift_types/test_results.py b/python/lib/sift_client/_tests/sift_types/test_results.py index 0207b3eb4..333dca602 100644 --- a/python/lib/sift_client/_tests/sift_types/test_results.py +++ b/python/lib/sift_client/_tests/sift_types/test_results.py @@ -11,6 +11,9 @@ from sift.test_reports.v1.test_reports_pb2 import ( TestMeasurement as TestMeasurementProto, ) +from sift.test_reports.v1.test_reports_pb2 import ( + TestStep as TestStepProto, +) from sift_client.sift_types.channel import Channel, ChannelDataType from sift_client.sift_types.test_report import ( @@ -22,6 +25,7 @@ TestReport, TestStatus, TestStep, + TestStepCreate, TestStepType, ) @@ -70,6 +74,7 @@ def mock_test_step(mock_client): error_code=1, error_message="Demo error message", ), + metadata={"fixture": "step", "iteration": 1.0}, ) test_step._apply_client_to_instance(mock_client) return test_step @@ -439,3 +444,54 @@ def test_measurement_from_proto_handles_absent_new_fields(self): assert measurement.description is None assert measurement.metadata is None assert measurement.channel_names is None + + def test_step_create_to_proto_writes_metadata(self): + """TestStepCreate.to_proto carries metadata onto the proto.""" + now = datetime.now(timezone.utc) + create = TestStepCreate( + test_report_id="report_789", + name="Step", + step_type=TestStepType.ACTION, + step_path="1", + status=TestStatus.IN_PROGRESS, + start_time=now, + end_time=now, + metadata={"pn": "PN-001", "count": 3, "flag": True}, + ) + proto = create.to_proto() + proto_keys = {m.key.name for m in proto.metadata} + assert proto_keys == {"pn", "count", "flag"} + + def test_step_from_proto_round_trips_metadata(self): + """A proto with metadata populated round-trips into TestStep.""" + now = datetime.now(timezone.utc) + source = TestStepCreate( + test_report_id="report_789", + name="Step", + step_type=TestStepType.ACTION, + step_path="1", + status=TestStatus.IN_PROGRESS, + start_time=now, + end_time=now, + metadata={"pn": "PN-001", "count": 3}, + ).to_proto() + source.test_step_id = "step_456" + + step = TestStep._from_proto(source) + + assert step.metadata == {"pn": "PN-001", "count": 3} + + def test_step_from_proto_handles_absent_metadata(self): + """Proto with unset metadata yields None on the model.""" + proto = TestStepProto( + test_step_id="step_abc", + test_report_id="report_789", + name="Step", + step_type=TestStepType.ACTION.value, + step_path="1", + status=TestStatus.IN_PROGRESS.value, + ) + proto.start_time.FromDatetime(datetime.now(timezone.utc)) + proto.end_time.FromDatetime(datetime.now(timezone.utc)) + step = TestStep._from_proto(proto) + assert step.metadata is None diff --git a/python/lib/sift_client/_tests/util/test_test_results_utils.py b/python/lib/sift_client/_tests/util/test_test_results_utils.py index e23003d2f..256803769 100644 --- a/python/lib/sift_client/_tests/util/test_test_results_utils.py +++ b/python/lib/sift_client/_tests/util/test_test_results_utils.py @@ -64,8 +64,12 @@ def test_new_step(self, report_context): prefix = f"{'.'.join(first_step_path_parts[:-1])}." second_step_path = f"{prefix}{int(first_step_path_parts[-1]) + 1}" test_step = None + expected_step_metadata = {"phase": "setup", "retry": 1, "instrumented": True} + expected_substep_metadata = {"phase": "assert"} # Test NewStep as a context manager directly - with NewStep(report_context, "Test Step", "Test Description") as new_step: + with NewStep( + report_context, "Test Step", "Test Description", metadata=expected_step_metadata + ) as new_step: test_step = new_step.current_step assert test_step.test_report_id == report_context.report.id_ assert test_step.name == "Test Step" @@ -76,8 +80,11 @@ def test_new_step(self, report_context): assert test_step.step_path == first_step_path assert test_step.step_type == TestStepType.ACTION assert test_step.error_info == None + assert test_step.metadata == expected_step_metadata - with new_step.substep("Substep", "Substep Description") as substep: + with new_step.substep( + "Substep", "Substep Description", metadata=expected_substep_metadata + ) as substep: current_substep = substep.current_step assert current_substep.test_report_id == report_context.report.id_ assert current_substep.name == "Substep" @@ -89,6 +96,7 @@ def test_new_step(self, report_context): assert current_substep.status == TestStatus.IN_PROGRESS assert current_substep.step_type == TestStepType.ACTION assert current_substep.error_info == None + assert current_substep.metadata == expected_substep_metadata with substep.substep( "nested substep", "Nested substep Description" diff --git a/python/lib/sift_client/sift_types/test_report.py b/python/lib/sift_client/sift_types/test_report.py index 413aa87ba..ecc24f52f 100644 --- a/python/lib/sift_client/sift_types/test_report.py +++ b/python/lib/sift_client/sift_types/test_report.py @@ -86,6 +86,13 @@ class TestStepBase(ModelCreateUpdateBase): parent_step_id: str | None = None description: str | None = None error_info: ErrorInfo | None = None + metadata: dict[str, str | float | bool] | None = None + + _to_proto_helpers: ClassVar[dict[str, MappingHelper]] = { + "metadata": MappingHelper( + proto_attr_path="metadata", update_field="metadata", converter=metadata_dict_to_proto + ), + } def _get_proto_class(self) -> type[TestStepProto]: return TestStepProto @@ -140,6 +147,9 @@ def to_proto(self) -> TestStepProto: if self.error_info: proto.error_info.CopyFrom(self.error_info._to_proto()) + if self.metadata: + proto.metadata.extend(metadata_dict_to_proto(self.metadata)) + return proto @@ -156,6 +166,7 @@ class TestStep(BaseType[TestStepProto, "TestStep"], FileAttachmentsMixin): start_time: datetime end_time: datetime error_info: ErrorInfo | None = None + metadata: dict[str, str | float | bool] | None = None # Set by the resource layer when this instance was produced from a logging-mode call _log_file: str | Path | None = None @@ -176,6 +187,7 @@ def _from_proto(cls, proto: TestStepProto, sift_client: SiftClient | None = None error_info=ErrorInfo._from_proto(proto.error_info, sift_client) if proto.HasField("error_info") else None, + metadata=metadata_proto_to_dict(proto.metadata) if proto.metadata else None, # type: ignore[arg-type] _client=sift_client, ) @@ -202,6 +214,9 @@ def _to_proto(self) -> TestStepProto: if self.error_info: proto.error_info.CopyFrom(self.error_info._to_proto()) + if self.metadata: + proto.metadata.extend(metadata_dict_to_proto(self.metadata)) + return proto def update( diff --git a/python/lib/sift_client/util/test_results/context_manager.py b/python/lib/sift_client/util/test_results/context_manager.py index 7dad92b63..354f8564d 100644 --- a/python/lib/sift_client/util/test_results/context_manager.py +++ b/python/lib/sift_client/util/test_results/context_manager.py @@ -211,7 +211,11 @@ def __exit__(self, exc_type, exc_value, traceback): return True def new_step( - self, name: str, description: str | None = None, assertion_as_fail_not_error: bool = True + self, + name: str, + description: str | None = None, + assertion_as_fail_not_error: bool = True, + metadata: dict[str, str | float | bool] | None = None, ) -> NewStep: """Alias to return a new step context manager from this report context. Use create_step for actually creating a TestStep in the current context.""" return NewStep( @@ -219,6 +223,7 @@ def new_step( name=name, description=description, assertion_as_fail_not_error=assertion_as_fail_not_error, + metadata=metadata, ) def get_next_step_path(self) -> str: @@ -229,12 +234,20 @@ def get_next_step_path(self) -> str: prefix = f"{step_path}." if step_path else "" return f"{prefix}{next_step_number}" - def create_step(self, name: str, description: str | None = None) -> TestStep: + def create_step( + self, + name: str, + description: str | None = None, + metadata: dict[str, str | float | bool] | None = None, + ) -> TestStep: """Create a new step in the report context. Args: name: The name of the step. description: The description of the step. + metadata: [Optional] Structured key/value metadata to attach to the step. For + metadata shared across every step in a report, prefer the `metadata` attribute + of the enclosing `TestReport`. Returns: The created step. @@ -253,6 +266,7 @@ def create_step(self, name: str, description: str | None = None) -> TestStep: end_time=datetime.now(timezone.utc), description=description, parent_step_id=parent_step.id_ if parent_step else None, + metadata=metadata, ), log_file=self.log_file, ) @@ -324,6 +338,7 @@ def __init__( name: str, description: str | None = None, assertion_as_fail_not_error: bool = True, + metadata: dict[str, str | float | bool] | None = None, ): """Initialize a new step context. @@ -332,10 +347,11 @@ def __init__( name: The name of the step. description: The description of the step. assertion_as_fail_not_error: Mark steps with assertion errors as failed instead of error+traceback (some users want assertions to work as simple failures especially when using pytest). + metadata: [Optional] Structured key/value metadata to attach to the step. """ self.report_context = report_context self.client = report_context.client - self.current_step = self.report_context.create_step(name, description) + self.current_step = self.report_context.create_step(name, description, metadata=metadata) self.assertion_as_fail_not_error = assertion_as_fail_not_error def __enter__(self): @@ -602,10 +618,16 @@ def report_outcome(self, name: str, result: bool, reason: str | None = None) -> self.report_context.record_step_outcome(result, substep.current_step) return result - def substep(self, name: str, description: str | None = None) -> NewStep: + def substep( + self, + name: str, + description: str | None = None, + metadata: dict[str, str | float | bool] | None = None, + ) -> NewStep: """Alias to return a new step context manager from the current step. The ReportContext will manage nesting of steps.""" return self.report_context.new_step( name=name, description=description, assertion_as_fail_not_error=self.assertion_as_fail_not_error, + metadata=metadata, )