From 66ae51ff46d8c5b09f7a364fdb323db173fcc6cc Mon Sep 17 00:00:00 2001 From: Yufei Date: Wed, 9 Sep 2026 16:40:02 -0700 Subject: [PATCH 1/2] fix(validation): classify warnings independently of message text --- .github/workflows/validation-ci.yml | 2 +- validation/tests/test_validate.py | 102 ++++++++++++++++++++++++++++ validation/validate.py | 18 +++-- 3 files changed, 116 insertions(+), 6 deletions(-) diff --git a/.github/workflows/validation-ci.yml b/.github/workflows/validation-ci.yml index a5c670a3..ab530bec 100644 --- a/.github/workflows/validation-ci.yml +++ b/.github/workflows/validation-ci.yml @@ -60,7 +60,7 @@ jobs: run: uv run validation/test_validate.py - name: Run validation pytest tests - run: uv run --with pytest --with pyyaml --with jsonschema -m pytest validation/tests/ + run: uv run --with pytest --with pyyaml --with jsonschema --with sqlglot -m pytest validation/tests/ - name: Validate canonical example run: uv run validation/validate.py examples/tpcds_semantic_model.yaml diff --git a/validation/tests/test_validate.py b/validation/tests/test_validate.py index 30806f18..4c03f2b6 100644 --- a/validation/tests/test_validate.py +++ b/validation/tests/test_validate.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. +import json from importlib.util import module_from_spec, spec_from_file_location from pathlib import Path @@ -161,3 +162,104 @@ def test_skips_malformed_flat_unique_keys() -> None: ) assert errors == [] + + +@pytest.fixture +def run_validator(tmp_path, monkeypatch, capsys): + def run(document): + model_path = tmp_path / "model.yaml" + model_path.write_text(json.dumps(document)) + monkeypatch.setattr(_VALIDATE.sys, "argv", [str(_VALIDATE_PATH), str(model_path)]) + with pytest.raises(SystemExit) as caught: + _VALIDATE.main() + return caught.value.code, capsys.readouterr().out + + return run + + +@pytest.mark.parametrize("target", [ + "missing_customers", + "Warning: missing_customers", + "[SQL] Warning: missing_customers", + "[Reference] Warning: missing_customers", +]) +def test_unknown_dataset_is_an_error_regardless_of_its_name(run_validator, target): + document = _document([_ORDERS], [_relationship(to_columns=["id"], to=target)]) + + exit_code, output = run_validator(document) + + assert exit_code == 1 + assert "Validation FAILED with 1 error(s)" in output + assert f"references unknown dataset '{target}'" in output + assert "Validation PASSED" not in output + + +def test_duplicate_dataset_with_warning_in_name_is_an_error(run_validator): + dataset = {"name": "Warning: orders", "source": "db.s.orders"} + + exit_code, output = run_validator(_document([dataset, dataset], [])) + + assert exit_code == 1 + assert "Validation FAILED with 1 error(s)" in output + assert "Duplicate dataset name 'Warning: orders'" in output + + +def test_schema_error_containing_warning_text_is_an_error(run_validator): + document = _document([_ORDERS], []) + document["Warning: unexpected"] = True + + exit_code, output = run_validator(document) + + assert exit_code == 1 + assert "Validation FAILED with 1 error(s)" in output + assert "[Schema]" in output + assert "Warning: unexpected" in output + + +@pytest.mark.skipif(not _VALIDATE.SQLGLOT_AVAILABLE, reason="sqlglot is not installed") +def test_sql_error_in_metric_with_warning_in_name_is_an_error(run_validator): + document = _document([_ORDERS], []) + document["semantic_model"][0]["metrics"] = [{ + "name": "Warning: broken_metric", + "expression": {"dialects": [{"dialect": "ANSI_SQL", "expression": "SUM("}]}, + }] + + exit_code, output = run_validator(document) + + assert exit_code == 1 + assert "Validation FAILED with 1 error(s)" in output + assert "[SQL] Metric 'Warning: broken_metric'" in output + + +def test_key_coverage_warning_remains_nonfatal(run_validator): + document = _document([_ORDERS, _CUSTOMERS], [_relationship(to_columns=["region"])]) + + exit_code, output = run_validator(document) + + assert exit_code == 0 + assert "[Reference] Warning:" in output + assert "Validation PASSED" in output + + +def test_missing_sqlglot_warning_remains_nonfatal(run_validator, monkeypatch): + monkeypatch.setattr(_VALIDATE, "SQLGLOT_AVAILABLE", False) + + exit_code, output = run_validator(_document([_ORDERS], [])) + + assert exit_code == 0 + assert "[SQL] Warning: sqlglot not installed" in output + assert "Validation PASSED" in output + + +def test_genuine_warning_does_not_hide_reference_error(run_validator): + document = _document([_ORDERS, _CUSTOMERS], [ + _relationship(to_columns=["region"]), + {**_relationship(to_columns=["id"], to="Warning: missing"), "name": "broken"}, + ]) + + exit_code, output = run_validator(document) + + assert exit_code == 1 + assert "[Reference] Warning:" in output + assert "references unknown dataset 'Warning: missing'" in output + assert "Validation FAILED with 1 error(s)" in output diff --git a/validation/validate.py b/validation/validate.py index e11d49f4..9060bfcf 100644 --- a/validation/validate.py +++ b/validation/validate.py @@ -79,6 +79,10 @@ SKIP_SQL_VALIDATION = {"MDX", "TABLEAU", "MAQL", "SIGMA", "THOUGHTSPOT"} +class ValidationWarning(str): + """An explicitly nonfatal diagnostic, compatible with existing string callers.""" + + class UniqueKeyLoader(yaml.SafeLoader): """Safe YAML loader that rejects duplicate explicit mapping keys.""" @@ -226,7 +230,9 @@ def validate_references(data: dict) -> list[str]: declared_keys = [k for k in candidate_keys if isinstance(k, list) and k] to_column_set = set(to_columns) if declared_keys and not any(set(key) <= to_column_set for key in declared_keys): - errors.append(f"[Reference] Warning: Relationship '{rel_name}' in model '{model_name}': to_columns {to_columns} does not cover the primary key or a unique key of dataset '{to_ds}'") + errors.append(ValidationWarning( + f"[Reference] Warning: Relationship '{rel_name}' in model '{model_name}': to_columns {to_columns} does not cover the primary key or a unique key of dataset '{to_ds}'" + )) return errors @@ -263,7 +269,9 @@ def validate_sql(data: dict) -> list[str]: return [] if not SQLGLOT_AVAILABLE: - return ["[SQL] Warning: sqlglot not installed, skipping SQL validation. Install with: pip install sqlglot"] + return [ValidationWarning( + "[SQL] Warning: sqlglot not installed, skipping SQL validation. Install with: pip install sqlglot" + )] errors = [] @@ -348,9 +356,9 @@ def main(): # Report results if errors: - # Separate warnings from errors - warnings = [e for e in errors if "Warning:" in e] - actual_errors = [e for e in errors if "Warning:" not in e] + # Severity must not depend on user-controlled text in a diagnostic. + warnings = [e for e in errors if isinstance(e, ValidationWarning)] + actual_errors = [e for e in errors if not isinstance(e, ValidationWarning)] for warning in warnings: print(f" {warning}") From 12766108645ba50c9d2b7c6113bde4c1b578e2eb Mon Sep 17 00:00:00 2001 From: Yufei Date: Wed, 9 Sep 2026 17:02:59 -0700 Subject: [PATCH 2/2] test(validation): match fixture extension to JSON contents --- validation/tests/test_validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validation/tests/test_validate.py b/validation/tests/test_validate.py index 4c03f2b6..56f02f67 100644 --- a/validation/tests/test_validate.py +++ b/validation/tests/test_validate.py @@ -167,7 +167,7 @@ def test_skips_malformed_flat_unique_keys() -> None: @pytest.fixture def run_validator(tmp_path, monkeypatch, capsys): def run(document): - model_path = tmp_path / "model.yaml" + model_path = tmp_path / "model.json" model_path.write_text(json.dumps(document)) monkeypatch.setattr(_VALIDATE.sys, "argv", [str(_VALIDATE_PATH), str(model_path)]) with pytest.raises(SystemExit) as caught: