From 610dd1e63efa72ccbabbcffe785f6008b7434d21 Mon Sep 17 00:00:00 2001 From: Oluwajuwon Date: Fri, 31 Jul 2026 19:17:01 +0100 Subject: [PATCH 1/2] update test parameter handling and improve entity reference normalization --- .../unit/test_entity_id_normalize.py | 117 ++++++++++++++++++ dot/utils/configuration_management.py | 12 +- dot/utils/configuration_utils.py | 72 +++++++++++ 3 files changed, 198 insertions(+), 3 deletions(-) create mode 100644 dot/self_tests/unit/test_entity_id_normalize.py mode change 100644 => 100755 dot/utils/configuration_management.py mode change 100644 => 100755 dot/utils/configuration_utils.py diff --git a/dot/self_tests/unit/test_entity_id_normalize.py b/dot/self_tests/unit/test_entity_id_normalize.py new file mode 100644 index 0000000..47576fa --- /dev/null +++ b/dot/self_tests/unit/test_entity_id_normalize.py @@ -0,0 +1,117 @@ +"""Tests for bare entity_id normalization in test_parameters.""" + +from utils.configuration_utils import ( + prepare_test_parameters, + to_bare_entity_id, + to_dbt_ref, + to_dbt_table, +) + + +class TestToBareEntityId: + """to_bare_entity_id accepts bare, prefixed, and ref() forms.""" + + def test_bare_id(self): + assert to_bare_entity_id("all_airports_data") == "all_airports_data" + + def test_prefixed_table(self): + assert to_bare_entity_id("dot_model__all_airports_data") == "all_airports_data" + + def test_dbt_ref_single_quotes(self): + assert ( + to_bare_entity_id("ref('dot_model__all_airports_data')") + == "all_airports_data" + ) + + def test_dbt_ref_double_quotes(self): + assert ( + to_bare_entity_id('ref("dot_model__all_airports_data")') + == "all_airports_data" + ) + + def test_empty_and_none(self): + assert to_bare_entity_id(None) is None + assert to_bare_entity_id("") is None + assert to_bare_entity_id(" ") is None + + +class TestToDbtFormats: + """Wrap bare/legacy values into DBT artifact forms.""" + + def test_to_dbt_ref_from_bare(self): + assert ( + to_dbt_ref("all_airports_data") + == "ref('dot_model__all_airports_data')" + ) + + def test_to_dbt_ref_from_legacy_ref(self): + assert ( + to_dbt_ref("ref('dot_model__all_airports_data')") + == "ref('dot_model__all_airports_data')" + ) + + def test_to_dbt_table_from_bare(self): + assert to_dbt_table("all_flight_data") == "dot_model__all_flight_data" + + def test_to_dbt_table_from_prefixed(self): + assert ( + to_dbt_table("dot_model__all_flight_data") == "dot_model__all_flight_data" + ) + + +class TestPrepareTestParameters: + """prepare_test_parameters adapts keys and entity formats per test type.""" + + def test_relationships_bare_to(self): + result = prepare_test_parameters( + "relationships", + {"to": "all_airports_data", "field": "airport", "name": "x"}, + ) + assert result["to"] == "ref('dot_model__all_airports_data')" + assert result["field"] == "airport" + + def test_relationships_legacy_ref_to(self): + result = prepare_test_parameters( + "relationships", + { + "to": "ref('dot_model__all_airports_data')", + "field": "airport", + }, + ) + assert result["to"] == "ref('dot_model__all_airports_data')" + + def test_relationships_reference_alias(self): + result = prepare_test_parameters( + "relationships", + {"reference": "ancview_pregnancy", "field": "uuid"}, + ) + assert "reference" not in result + assert result["to"] == "ref('dot_model__ancview_pregnancy')" + + def test_expect_similar_means_bare_data_table(self): + result = prepare_test_parameters( + "expect_similar_means_across_reporters", + { + "key": "airline", + "data_table": "all_flight_data", + "target_table": "airlines_data", + }, + ) + assert result["data_table"] == "dot_model__all_flight_data" + assert result["target_table"] == "dot_model__airlines_data" + + def test_expect_similar_means_form_name_alias(self): + result = prepare_test_parameters( + "expect_similar_means_across_reporters", + {"form_name": "iccmview_assessment", "key": "reported_by"}, + ) + assert "form_name" not in result + assert result["data_table"] == "dot_model__iccmview_assessment" + + def test_other_test_type_unchanged(self): + params = {"values": ["a", "b"]} + assert prepare_test_parameters("accepted_values", params) == params + + def test_non_dict_passthrough(self): + assert prepare_test_parameters("relationships", None) is None + assert prepare_test_parameters("relationships", "") == "" diff --git a/dot/utils/configuration_management.py b/dot/utils/configuration_management.py old mode 100644 new mode 100755 index 5e8b18a..ab8b2ad --- a/dot/utils/configuration_management.py +++ b/dot/utils/configuration_management.py @@ -21,7 +21,8 @@ GE_GREAT_EXPECTATIONS_FINAL_FILENAME, GE_CONFIG_VARIABLES_FINAL_FILENAME, load_config_file, - DBT_MODELNAME_PREFIX + DBT_MODELNAME_PREFIX, + prepare_test_parameters, ) from utils.dbt import create_core_entities @@ -330,7 +331,9 @@ def generate_tests_from_db(project_id, logger=logging.Logger): column_name = row["column_name"] description = row["description"] test_type = row["test_type"] - test_parameters = row["test_parameters"] + test_parameters = prepare_test_parameters( + test_type, row["test_parameters"] + ) # if test_parameters != None: # test_parameters = "| ".join([f"{k}={test_parameters[k]}" for k in test_parameters]) # else: @@ -433,7 +436,10 @@ def generate_tests_from_db(project_id, logger=logging.Logger): # "kwargs": add_ge_schema_parameters( # json.loads(row["test_parameters"]), project_id # ), - "kwargs": add_ge_schema_parameters(row["test_parameters"], project_id), + "kwargs": add_ge_schema_parameters( + prepare_test_parameters(row["test_type"], row["test_parameters"]), + project_id, + ), "meta": {}, } diff --git a/dot/utils/configuration_utils.py b/dot/utils/configuration_utils.py old mode 100644 new mode 100755 index 18d506f..a2a6679 --- a/dot/utils/configuration_utils.py +++ b/dot/utils/configuration_utils.py @@ -20,9 +20,81 @@ ) DBT_MODELNAME_PREFIX = "dot_model__" +# Matches ref('dot_model__entity') or ref("dot_model__entity") +_DBT_REF_PATTERN = re.compile( + r"""^ref\(\s*['"]""" + re.escape(DBT_MODELNAME_PREFIX) + r"""([^'"]+)['"]\s*\)$""" +) + DBT_PROJECT_SEPARATOR = "/" +def to_bare_entity_id(value) -> Optional[str]: + """ + Normalize a stored entity reference to a bare entity_id. + + Accepts bare ids, `dot_model__{id}`, or `ref('dot_model__{id}')`. + Returns None for empty/non-string values. + """ + if value is None or not isinstance(value, str): + return None + value = value.strip() + if not value: + return None + + ref_match = _DBT_REF_PATTERN.match(value) + if ref_match: + return ref_match.group(1) + + if value.startswith(DBT_MODELNAME_PREFIX): + return value[len(DBT_MODELNAME_PREFIX) :] + + return value + + +def to_dbt_ref(value) -> Optional[str]: + """Convert an entity reference to `ref('dot_model__{entity_id}')`.""" + entity_id = to_bare_entity_id(value) + if entity_id is None: + return value if isinstance(value, str) else value + return f"ref('{DBT_MODELNAME_PREFIX}{entity_id}')" + + +def to_dbt_table(value) -> Optional[str]: + """Convert an entity reference to a physical `dot_model__{entity_id}` table name.""" + entity_id = to_bare_entity_id(value) + if entity_id is None: + return value if isinstance(value, str) else value + return f"{DBT_MODELNAME_PREFIX}{entity_id}" + + +def prepare_test_parameters(test_type: str, params) -> dict: + """ + Adapt stored test_parameters for DBT/GE artifact generation. + + Stores may use bare entity_ids; legacy rows may still use ref()/dot_model__ + prefixes. Also aliases legacy keys (reference→to, form_name→data_table). + """ + if params in ("", None, "null") or not isinstance(params, dict): + return params + + prepared = dict(params) + + if test_type == "relationships": + if "to" not in prepared and "reference" in prepared: + prepared["to"] = prepared.pop("reference") + if "to" in prepared and prepared["to"] not in ("", None): + prepared["to"] = to_dbt_ref(prepared["to"]) + + elif test_type == "expect_similar_means_across_reporters": + if "data_table" not in prepared and "form_name" in prepared: + prepared["data_table"] = prepared.pop("form_name") + for key in ("data_table", "target_table"): + if key in prepared and prepared[key] not in ("", None): + prepared[key] = to_dbt_table(prepared[key]) + + return prepared + + def _get_filename_safely(path: str) -> str: """ Internal function - checks if the path exists From 8d16ad99159045592f0a0724a2fe80c6c1f82d83 Mon Sep 17 00:00:00 2001 From: Oluwajuwon Date: Wed, 5 Aug 2026 13:09:26 +0100 Subject: [PATCH 2/2] fix: silence missing-docstring pylint on bare entity ID tests CI lints only changed Python files; the new unit test module fell below the pylint threshold because pytest-style methods lack method docstrings. --- dot/self_tests/unit/test_entity_id_normalize.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dot/self_tests/unit/test_entity_id_normalize.py b/dot/self_tests/unit/test_entity_id_normalize.py index 47576fa..a61af1f 100644 --- a/dot/self_tests/unit/test_entity_id_normalize.py +++ b/dot/self_tests/unit/test_entity_id_normalize.py @@ -1,5 +1,8 @@ """Tests for bare entity_id normalization in test_parameters.""" +# Test method names already describe behavior; keep assertions uncluttered. +# pylint: disable=missing-function-docstring + from utils.configuration_utils import ( prepare_test_parameters, to_bare_entity_id, @@ -39,10 +42,7 @@ class TestToDbtFormats: """Wrap bare/legacy values into DBT artifact forms.""" def test_to_dbt_ref_from_bare(self): - assert ( - to_dbt_ref("all_airports_data") - == "ref('dot_model__all_airports_data')" - ) + assert to_dbt_ref("all_airports_data") == "ref('dot_model__all_airports_data')" def test_to_dbt_ref_from_legacy_ref(self): assert (