From 8f616e0bd4ae8851c1b8b16e1c06cf3f372bef18 Mon Sep 17 00:00:00 2001 From: Stephen Thibeault Date: Wed, 24 Jun 2026 16:06:33 -0400 Subject: [PATCH] New validation for to/from column length and PK/UK/Join field references along with basic testing. --- validation/tests/conftest.py | 6 + validation/tests/test_validate_references.py | 141 ++++++++++++++++++ .../tests/test_validate_unique_names.py | 58 +++++++ validation/validate.py | 46 +++++- 4 files changed, 248 insertions(+), 3 deletions(-) create mode 100644 validation/tests/conftest.py create mode 100644 validation/tests/test_validate_references.py create mode 100644 validation/tests/test_validate_unique_names.py diff --git a/validation/tests/conftest.py b/validation/tests/conftest.py new file mode 100644 index 0000000..4c1ad31 --- /dev/null +++ b/validation/tests/conftest.py @@ -0,0 +1,6 @@ +"""Shared test setup: make validation/validate.py importable as `validate`""" + +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) diff --git a/validation/tests/test_validate_references.py b/validation/tests/test_validate_references.py new file mode 100644 index 0000000..3a5c34f --- /dev/null +++ b/validation/tests/test_validate_references.py @@ -0,0 +1,141 @@ +"""Tests for relationship and key column checks in validate_references""" + +from validate import validate_references + + +def test_valid_columns_produce_no_findings(): + data = { + "semantic_model": [ + { + "name": "m", + "datasets": [ + {"name": "store_sales", "fields": [{"name": "ss_customer_sk"}]}, + {"name": "customer", "fields": [{"name": "c_customer_sk"}]}, + ], + "relationships": [ + { + "name": "r", + "from": "store_sales", + "to": "customer", + "from_columns": ["ss_customer_sk"], + "to_columns": ["c_customer_sk"], + } + ], + } + ] + } + assert validate_references(data) == [] + + +def test_column_count_mismatch_is_reported(): + data = { + "semantic_model": [ + { + "name": "m", + "datasets": [ + { + "name": "store_sales", + "fields": [{"name": "ss_customer_sk"}, {"name": "ss_item_sk"}], + }, + {"name": "customer", "fields": [{"name": "c_customer_sk"}]}, + ], + "relationships": [ + { + "name": "r", + "from": "store_sales", + "to": "customer", + "from_columns": ["ss_customer_sk", "ss_item_sk"], + "to_columns": ["c_customer_sk"], + } + ], + } + ] + } + findings = validate_references(data) + assert any("counts must match" in f for f in findings) + # The mismatch is the only problem, columns themselves are declared fields. + assert all("not a declared field" not in f for f in findings) + + +def test_unknown_column_is_warned(): + data = { + "semantic_model": [ + { + "name": "m", + "datasets": [ + {"name": "store_sales", "fields": [{"name": "ss_customer_sk"}]}, + {"name": "customer", "fields": [{"name": "c_customer_sk"}]}, + ], + "relationships": [ + { + "name": "r", + "from": "store_sales", + "to": "customer", + "from_columns": ["typo"], + "to_columns": ["c_customer_sk"], + } + ], + } + ] + } + findings = validate_references(data) + assert any( + "Warning" in f and "'typo'" in f and "not a declared field of 'store_sales'" in f + for f in findings + ) + # Counts match here, so no count error. + assert all("counts must match" not in f for f in findings) + + +def test_unknown_key_columns_are_warned(): + data = { + "semantic_model": [ + { + "name": "m", + "datasets": [ + { + "name": "store_sales", + "primary_key": ["ss_item_sk", "ss_ticket_number"], + "unique_keys": [["ss_item_sk", "ss_ticket_number"]], + "fields": [{"name": "ss_item_sk"}], + } + ], + } + ] + } + findings = validate_references(data) + # ss_ticket_number is in primary_key and unique_keys but is not a declared field. + assert any( + "primary_key references 'ss_ticket_number', not a declared field" in f + for f in findings + ) + assert any( + "unique_keys[0] references 'ss_ticket_number', not a declared field" in f + for f in findings + ) + # ss_item_sk is declared, so it must not be flagged. + assert all("'ss_item_sk'" not in f for f in findings) + + +def test_unknown_dataset_skips_column_check(): + data = { + "semantic_model": [ + { + "name": "m", + "datasets": [{"name": "b", "fields": [{"name": "c"}]}], + "relationships": [ + { + "name": "r", + "from": "missing", + "to": "b", + "from_columns": ["x"], + "to_columns": ["c"], + } + ], + } + ] + } + findings = validate_references(data) + # Unknown dataset is reported, but its columns are not flagged as undeclared. + assert any("unknown dataset 'missing'" in f for f in findings) + assert all("'x'" not in f for f in findings) diff --git a/validation/tests/test_validate_unique_names.py b/validation/tests/test_validate_unique_names.py new file mode 100644 index 0000000..72feb30 --- /dev/null +++ b/validation/tests/test_validate_unique_names.py @@ -0,0 +1,58 @@ +"""Tests for validate_unique_names.""" + +from validate import validate_unique_names + + +def test_all_unique_names_produce_no_errors(): + data = { + "semantic_model": [ + { + "name": "m", + "datasets": [ + {"name": "a", "fields": [{"name": "x"}, {"name": "y"}]}, + {"name": "b", "fields": [{"name": "z"}]}, + ], + "metrics": [{"name": "m1"}, {"name": "m2"}], + "relationships": [{"name": "r1"}, {"name": "r2"}], + } + ] + } + assert validate_unique_names(data) == [] + + +def test_duplicate_dataset_name(): + data = {"semantic_model": [{"name": "m", "datasets": [{"name": "a"}, {"name": "a"}]}]} + errors = validate_unique_names(data) + assert errors == ["[Unique] Duplicate dataset name 'a' in model 'm'"] + + +def test_duplicate_field_name_is_scoped_to_dataset(): + # Same field name in two different datasets is ok, only an in dataset repeat fails. + data = { + "semantic_model": [ + { + "name": "m", + "datasets": [ + {"name": "a", "fields": [{"name": "x"}, {"name": "x"}]}, + {"name": "b", "fields": [{"name": "x"}]}, + ], + } + ] + } + errors = validate_unique_names(data) + assert errors == ["[Unique] Duplicate field name 'x' in dataset 'a'"] + + +def test_duplicate_metric_and_relationship_names(): + data = { + "semantic_model": [ + { + "name": "m", + "metrics": [{"name": "dup"}, {"name": "dup"}], + "relationships": [{"name": "rel"}, {"name": "rel"}], + } + ] + } + errors = validate_unique_names(data) + assert "[Unique] Duplicate metric name 'dup' in model 'm'" in errors + assert "[Unique] Duplicate relationship name 'rel' in model 'm'" in errors diff --git a/validation/validate.py b/validation/validate.py index a33cf76..0bae299 100644 --- a/validation/validate.py +++ b/validation/validate.py @@ -106,18 +106,58 @@ def validate_references(data: dict) -> list[str]: for model in data.get("semantic_model", []): model_name = model.get("name", "") - dataset_names = {d.get("name") for d in model.get("datasets", []) if d.get("name")} + datasets = {d.get("name"): d for d in model.get("datasets", []) if d.get("name")} + fields_by_ds = { + name: {f.get("name") for f in d.get("fields", []) if f.get("name")} + for name, d in datasets.items() + } + + # Key columns should be declared fields of their dataset (WARNING) + for ds_name, dataset in datasets.items(): + known = fields_by_ds[ds_name] + keys = [("primary_key", dataset.get("primary_key", []) or [])] + for i, uk in enumerate(dataset.get("unique_keys", []) or []): + keys.append((f"unique_keys[{i}]", uk or [])) + for key_name, cols in keys: + for col in cols: + if col not in known: + errors.append( + f"[Reference] Warning: dataset '{ds_name}' {key_name} references " + f"'{col}', not a declared field" + ) for rel in model.get("relationships", []): rel_name = rel.get("name", "") from_ds = rel.get("from") to_ds = rel.get("to") + from_cols = rel.get("from_columns", []) or [] + to_cols = rel.get("to_columns", []) or [] - if from_ds and from_ds not in dataset_names: + # Dataset existence + if from_ds and from_ds not in datasets: errors.append(f"[Reference] Relationship '{rel_name}' references unknown dataset '{from_ds}'") - if to_ds and to_ds not in dataset_names: + if to_ds and to_ds not in datasets: errors.append(f"[Reference] Relationship '{rel_name}' references unknown dataset '{to_ds}'") + # Column counts must correspond + if len(from_cols) != len(to_cols): + errors.append( + f"[Reference] Relationship '{rel_name}' has {len(from_cols)} from_columns " + f"but {len(to_cols)} to_columns; counts must match" + ) + + # Columns should be declared fields of their dataset (WARNING) + for side, ds_name, cols in (("from", from_ds, from_cols), ("to", to_ds, to_cols)): + known = fields_by_ds.get(ds_name) + if known is None: + continue + for col in cols: + if col not in known: + errors.append( + f"[Reference] Warning: relationship '{rel_name}' {side}_columns references " + f"'{col}', not a declared field of '{ds_name}'" + ) + return errors