Name the check fields qualityId and failedSamples

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Changed
+- Test results name the check fields `qualityId` and `failedSamples` instead of `quality_id` and `failed_samples`; the old names are deprecated, but still accepted as input and still written next to the new ones
 - `datacontract import unity` no longer writes the `databricksType` custom property, which duplicated `physicalType`
 - `datacontract export sql --server databricks` keeps the declared length of `varchar(n)` and `char(n)` instead of exporting `STRING`
 

datacontract/engines/datacontract/check_azure_blob_file.py

@@ -629,7 +629,7 @@ def _append_check(
             name=name,
             model=model,
             field=field,
-            quality_id=quality_id,
+            qualityId=quality_id,
             tags=tags,
             engine="datacontract",
             language="python",

datacontract/engines/ibis/ibis_check_execute.py

@@ -66,7 +66,7 @@ def build_check_stubs(specs: List[CheckSpec]) -> List[Check]:
                 name=spec.name,
                 model=spec.model,
                 field=spec.field,
-                quality_id=spec.quality_id,
+                qualityId=spec.quality_id,
                 tags=spec.tags,
                 engine="ibis",
                 implementation=_describe(spec),
@@ -365,7 +365,7 @@ def _collect_failed_samples(run, t, columns, schema, model, specs, data_contract
             logger.debug("Could not collect failed samples for '%s': %s", spec.key, e)
             continue
         if samples:
-            check.failed_samples = samples
+            check.failedSamples = samples
 
 
 def _sample_field_meta(data_contract, server, model):

datacontract/model/run.py

@@ -1,11 +1,12 @@
 import logging
+import warnings
 from datetime import datetime, timezone
 from enum import Enum
 from importlib import metadata
 from typing import List
 from uuid import UUID, uuid4
 
-from pydantic import BaseModel
+from pydantic import AliasChoices, BaseModel, Field, SerializerFunctionWrapHandler, model_serializer
 
 
 def _cli_version() -> str:
@@ -15,6 +16,32 @@ def _cli_version() -> str:
         return "unknown"
 
 
+# Check fields renamed to camelCase, mapped to the snake_case name they had
+# before. Both spellings are accepted as input, readable on the model, and
+# written to the test results.
+_DEPRECATED_CHECK_ALIASES = {"qualityId": "quality_id", "failedSamples": "failed_samples"}
+
+
+def _deprecated_alias(old_name: str, new_name: str) -> property:
+    """Read/write access to a renamed check field under its former snake_case name.
+
+    The check fields were renamed to camelCase to match the rest of the
+    test-results model (``runId``, ``dataContractId``, ...). Code written against
+    the old names keeps working and warns.
+    """
+    message = f"Check.{old_name} is deprecated, use Check.{new_name} instead."
+
+    def getter(self):
+        warnings.warn(message, DeprecationWarning, stacklevel=2)
+        return getattr(self, new_name)
+
+    def setter(self, value):
+        warnings.warn(message, DeprecationWarning, stacklevel=2)
+        setattr(self, new_name, value)
+
+    return property(getter, setter, doc=message)
+
+
 class ResultEnum(str, Enum):
     passed = "passed"
     warning = "warning"
@@ -30,12 +57,12 @@ class Check(BaseModel):
     category: str | None = None
     type: str
     name: str | None = None
-    model: str | None = None
-    field: str | None = None
+    model: str | None = None  # naming for historic reasons. Should rather be named schema
+    field: str | None = None  # naming for historic reasons. Should rather be named property
     # The ODCS `quality.id` / `quality.tags` of the rule this check comes from,
     # so a check can be traced back to (and re-run through `test --quality-id`
     # / `test --tag`) the rule that declared it. Empty for built-in checks.
-    quality_id: str | None = None
+    qualityId: str | None = Field(default=None, validation_alias=AliasChoices("qualityId", "quality_id"))
     tags: list[str] | None = None
 
     engine: str | None = None
@@ -49,7 +76,26 @@ class Check(BaseModel):
     # `datacontract test --include-failed-samples` is set). Each entry is a row
     # restricted to identifier + offending columns, with sensitive columns
     # omitted. The full failed count lives in `diagnostics`, not here.
-    failed_samples: list | None = None
+    failedSamples: list | None = Field(default=None, validation_alias=AliasChoices("failedSamples", "failed_samples"))
+
+    # Deprecated former names of the two fields above. They still read, write and
+    # validate, so `check.failed_samples` and older test-results JSON keep working.
+    quality_id = _deprecated_alias("quality_id", "qualityId")
+    failed_samples = _deprecated_alias("failed_samples", "failedSamples")
+
+    @model_serializer(mode="wrap")
+    def _serialize_with_deprecated_aliases(self, handler: SerializerFunctionWrapHandler) -> dict:
+        """Write a set field under its deprecated name too.
+
+        The test results are published to the `/api/test-results` API, so a
+        consumer still reading `quality_id` / `failed_samples` keeps working. A
+        field that is not set stays absent under both names.
+        """
+        data = handler(self)
+        for new_name, old_name in _DEPRECATED_CHECK_ALIASES.items():
+            if new_name in data and getattr(self, new_name) is not None:
+                data[old_name] = data[new_name]
+        return data
 
 
 class Log(BaseModel):

datacontract/output/junit_test_results.py

@@ -123,8 +123,8 @@ def to_failure_text(check):
         f"Reason: {check.reason}\n"
         f"Diagnostics:\n{yaml.dump(check.diagnostics, default_flow_style=False)}"
     )
-    if check.failed_samples:
-        text += f"Failed samples:\n{yaml.dump(check.failed_samples, default_flow_style=False)}"
+    if check.failedSamples:
+        text += f"Failed samples:\n{yaml.dump(check.failedSamples, default_flow_style=False)}"
     return text
 
 

docs/docs/release-notes.md

@@ -27,6 +27,7 @@ marked as such in the entry.
 ## Unreleased {#unreleased}
 
 ### Changed
+- Test results name the check fields `qualityId` and `failedSamples` instead of `quality_id` and `failed_samples`; the old names are deprecated, but still accepted as input and still written next to the new ones
 - `datacontract import unity` no longer writes the `databricksType` custom property, which duplicated `physicalType`
 - `datacontract export sql --server databricks` keeps the declared length of `varchar(n)` and `char(n)` instead of exporting `STRING`
 

tests/test_run_check_deprecated_fields.py

@@ -0,0 +1,65 @@
+"""The check fields renamed to camelCase keep working under their old names.
+
+`quality_id` / `failed_samples` were renamed to `qualityId` / `failedSamples` to
+match the rest of the test-results model (`runId`, `dataContractId`, ...). Code
+and stored test results written against the old names must keep working.
+"""
+
+import pytest
+
+from datacontract.model.run import Check
+
+
+def test_reading_the_old_name_returns_the_new_value():
+    check = Check(type="field_type", qualityId="orders_not_empty", failedSamples=[{"id": 1}])
+
+    with pytest.warns(DeprecationWarning, match="Check.quality_id is deprecated"):
+        assert check.quality_id == "orders_not_empty"
+    with pytest.warns(DeprecationWarning, match="Check.failed_samples is deprecated"):
+        assert check.failed_samples == [{"id": 1}]
+
+
+def test_writing_the_old_name_sets_the_new_field():
+    check = Check(type="field_type")
+
+    with pytest.warns(DeprecationWarning, match="Check.quality_id is deprecated"):
+        check.quality_id = "orders_not_empty"
+    with pytest.warns(DeprecationWarning, match="Check.failed_samples is deprecated"):
+        check.failed_samples = [{"id": 1}]
+
+    assert check.qualityId == "orders_not_empty"
+    assert check.failedSamples == [{"id": 1}]
+
+
+def test_the_old_names_are_still_accepted_as_input():
+    # keyword construction, and test results serialized by an older version
+    assert Check(type="field_type", quality_id="r", failed_samples=[1]).qualityId == "r"
+    restored = Check.model_validate({"type": "field_type", "quality_id": "r", "failed_samples": [1]})
+    assert (restored.qualityId, restored.failedSamples) == ("r", [1])
+
+
+def test_a_set_field_is_serialized_under_both_names():
+    # the test results are published to /api/test-results, so a consumer still
+    # reading the old names keeps working
+    check = Check(type="field_type", quality_id="r", failed_samples=[1])
+
+    serialized = check.model_dump(exclude_none=True)
+
+    assert serialized == {
+        "type": "field_type",
+        "qualityId": "r",
+        "failedSamples": [1],
+        "quality_id": "r",
+        "failed_samples": [1],
+    }
+
+
+def test_an_unset_field_is_not_serialized_under_the_old_name():
+    check = Check(type="field_type")
+
+    serialized = check.model_dump()
+
+    assert serialized["qualityId"] is None
+    assert serialized["failedSamples"] is None
+    assert "quality_id" not in serialized
+    assert "failed_samples" not in serialized

tests/test_test_azure_blob_file.py

@@ -418,7 +418,7 @@ def _run(self, **kwargs) -> Run:
     def test_quality_id_selects_a_single_rule(self):
         run = self._run(quality_ids={"enough_files"})
         assert [c.type for c in run.checks] == ["azure_file_count_quality"]
-        assert run.checks[0].quality_id == "enough_files"
+        assert run.checks[0].qualityId == "enough_files"
 
     def test_tag_selects_every_rule_carrying_it(self):
         run = self._run(tags={"critical"})

tests/test_test_failed_samples.py

@@ -58,17 +58,17 @@ def _check(run, type_, field=None):
 def test_no_samples_collected_without_flag():
     run = _run(include_failed_samples=False)
     assert run.result == ResultEnum.failed  # there are real violations
-    assert all(c.failed_samples is None for c in run.checks)
+    assert all(c.failedSamples is None for c in run.checks)
 
 
 def test_missing_samples_have_identifier_and_offending_column():
     run = _run(include_failed_samples=True)
     check = _check(run, "field_required", field="region")
     assert check.result == ResultEnum.failed
-    assert check.failed_samples is not None
+    assert check.failedSamples is not None
     # region is empty for ids 3 and 5.
-    assert {s["id"] for s in check.failed_samples} == {3, 5}
-    for s in check.failed_samples:
+    assert {s["id"] for s in check.failedSamples} == {3, 5}
+    for s in check.failedSamples:
         assert set(s.keys()) == {"id", "region"}
         assert s["region"] is None
 
@@ -77,22 +77,22 @@ def test_invalid_range_sample_includes_offending_value():
     run = _run(include_failed_samples=True)
     check = _check(run, "field_maximum", field="amount")
     assert check.result == ResultEnum.failed
-    assert check.failed_samples == [{"id": 3, "amount": 200}]
+    assert check.failedSamples == [{"id": 3, "amount": 200}]
 
 
 def test_samples_respect_the_limit():
     run = _run(include_failed_samples=True)
     check = _check(run, "field_regex", field="email")
     assert check.result == ResultEnum.failed
     # 6 rows fail the email pattern, but samples are capped at 5.
-    assert len(check.failed_samples) == 5
+    assert len(check.failedSamples) == 5
 
 
 def test_sensitive_column_is_omitted_from_samples():
     run = _run(include_failed_samples=True)
     check = _check(run, "field_regex", field="email")
     # email is classified PII, so its value must not appear; only the identifier.
-    for s in check.failed_samples:
+    for s in check.failedSamples:
         assert "email" not in s
         assert set(s.keys()) == {"id"}
 
@@ -101,4 +101,4 @@ def test_duplicate_samples_report_key_and_count():
     run = _run(include_failed_samples=True)
     check = _check(run, "field_unique", field="id")
     assert check.result == ResultEnum.failed
-    assert check.failed_samples == [{"id": 2, "duplicate_count": 2}]
+    assert check.failedSamples == [{"id": 2, "duplicate_count": 2}]

tests/test_test_quality_id_and_tag_filter.py

@@ -123,11 +123,11 @@ def test_checks_report_the_id_and_tags_of_their_rule():
     run = DataContract(data_contract_str=CONTRACT).test()
     print(run.pretty())
     by_type = {check.type: check for check in run.checks}
-    assert by_type["row_count"].quality_id == "orders_not_empty"
+    assert by_type["row_count"].qualityId == "orders_not_empty"
     assert by_type["row_count"].tags == ["critical", "cheap"]
-    assert by_type["field_null_values"].quality_id is None
+    assert by_type["field_null_values"].qualityId is None
     assert by_type["field_null_values"].tags is None
-    assert by_type["field_is_present"].quality_id is None
+    assert by_type["field_is_present"].qualityId is None
 
 
 def test_quality_id_cli_option():