From 94f5f3382f7e7d608e9b008bd5ca34bac9873094 Mon Sep 17 00:00:00 2001 From: Adegbite Ayoade Date: Sat, 12 Sep 2026 22:58:44 +0100 Subject: [PATCH 1/2] feat(test): treat model file paths as test selectors `sqlmesh test` already accepted multiple positional arguments, but they were only ever looked up in the test-file index. Passing a model file matched nothing and exited 0, so a commit hook handing over staged model files silently ran no tests at all. Each positional argument now resolves to a test file, a `file.yaml::test_name`, or a model file, in which case the tests targeting that model are selected. The results are unioned and deduplicated on the fully qualified test name, so a model file and the test file holding its tests select the same test once rather than running it twice. An argument that resolves to neither a known test nor a known model file is now an error instead of a silent skip. That is opt-in via raise_on_unknown_paths and only the CLI turns it on, because LSPContext.get_document_tests selects against arbitrary open documents and relies on an empty result. A known model that simply has no tests still selects nothing, which is not an error. Selectors are matched as given before being normalized, so relative paths work from the project root without any resolve() calls per model. The model path index is built at most once per call, and only when a selector is not a test file. Signed-off-by: Adegbite Ayoade --- docs/concepts/tests.md | 14 ++++++ docs/reference/cli.md | 4 ++ sqlmesh/cli/main.py | 8 ++- sqlmesh/core/context.py | 106 ++++++++++++++++++++++++++++++++++------ tests/cli/test_cli.py | 21 ++++++++ tests/core/test_test.py | 100 +++++++++++++++++++++++++++++++++++++ 6 files changed, 237 insertions(+), 16 deletions(-) diff --git a/docs/concepts/tests.md b/docs/concepts/tests.md index a293237687..261ffb8694 100644 --- a/docs/concepts/tests.md +++ b/docs/concepts/tests.md @@ -463,6 +463,20 @@ You can also run tests that match a pattern or substring using a glob pathname e $ sqlmesh test tests/test_* ``` +Passing the path of a model file runs the tests for that model, which is useful for commit hooks and other tools that work with changed files rather than test names: + +``` +$ sqlmesh test models/full_model.sql +``` + +Model files and test files can be mixed, and the results are unioned. A test selected by more than one argument still runs only once, so the following runs each of `full_model`'s tests a single time even though both arguments cover them: + +``` +$ sqlmesh test models/full_model.sql tests/test_full_model.yaml +``` + +An argument that is neither a known model file nor a known test file is an error, so a mistyped or stale path fails instead of quietly running no tests. A model that simply has no tests is not an error. + ### Testing using notebooks You can execute tests on demand using the `%run_test` notebook magic as follows: diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 1367f8551b..2112d8b050 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -623,6 +623,10 @@ Usage: sqlmesh test [OPTIONS] [TESTS]... Run model unit tests. + TESTS are test files, `file.yaml::test_name` selectors, or model files, in + which case the tests for those models are run. They are unioned, and a test + selected more than once still only runs once. + Options: -k TEXT Only run tests that match the pattern of substring. -v, --verbose Verbose output. diff --git a/sqlmesh/cli/main.py b/sqlmesh/cli/main.py index b6678136f0..85c2186a3f 100644 --- a/sqlmesh/cli/main.py +++ b/sqlmesh/cli/main.py @@ -823,7 +823,12 @@ def test( select_model: t.List[str], tests: t.List[str], ) -> None: - """Run model unit tests.""" + """Run model unit tests. + + TESTS are test files, `file.yaml::test_name` selectors, or model files, in which case the + tests for those models are run. They are unioned, and a test selected more than once still + only runs once. + """ model_names = ( obj._new_selector().expand_model_selections(select_model) if select_model else None ) @@ -833,6 +838,7 @@ def test( verbosity=Verbosity(verbose), preserve_fixtures=preserve_fixtures, model_names=model_names, + raise_on_unknown_paths=True, ) if not result.wasSuccessful(): exit(1) diff --git a/sqlmesh/core/context.py b/sqlmesh/core/context.py index b70ccae141..9d4650d2de 100644 --- a/sqlmesh/core/context.py +++ b/sqlmesh/core/context.py @@ -36,6 +36,7 @@ import abc import collections import logging +import os.path import sys import time import traceback @@ -119,7 +120,7 @@ filter_tests_by_patterns, ) from sqlmesh.core.user import User -from sqlmesh.utils import CorrelationId, UniqueKeyDict, Verbosity +from sqlmesh.utils import CorrelationId, UniqueKeyDict, Verbosity, unique from sqlmesh.utils.concurrency import concurrent_apply_to_values from sqlmesh.utils.dag import DAG from sqlmesh.utils.date import ( @@ -2407,6 +2408,7 @@ def test( preserve_fixtures: bool = False, stream: t.Optional[t.TextIO] = None, model_names: t.Optional[t.Collection[str]] = None, + raise_on_unknown_paths: bool = False, ) -> ModelTextTestResult: """Discover and run model tests""" if verbosity >= Verbosity.VERBOSE: @@ -2414,10 +2416,18 @@ def test( pd.set_option("display.max_columns", None) - baseline_meta = self.select_tests(tests=tests, patterns=match_patterns, model_names=None) + baseline_meta = self.select_tests( + tests=tests, + patterns=match_patterns, + model_names=None, + raise_on_unknown_paths=raise_on_unknown_paths, + ) if model_names is not None: test_meta = self.select_tests( - tests=tests, patterns=match_patterns, model_names=model_names + tests=tests, + patterns=match_patterns, + model_names=model_names, + raise_on_unknown_paths=raise_on_unknown_paths, ) tests_skipped = len(baseline_meta) - len(test_meta) else: @@ -3611,30 +3621,96 @@ def lint_models( return all_violations + def _tests_by_absolute_model_path(self) -> t.Dict[str, t.List[ModelTestMetadata]]: + """Map each model file to the tests that target the model(s) defined in it.""" + tests_by_model_name: t.Dict[str, t.List[ModelTestMetadata]] = collections.defaultdict(list) + for metadata in self._model_test_metadata: + if metadata.model_name: + tests_by_model_name[ + normalize_model_name( + metadata.model_name, + default_catalog=self.default_catalog, + dialect=self.default_dialect, + ) + ].append(metadata) + + # A path is made absolute rather than resolved, so this costs no syscalls per model. + tests_by_path: t.Dict[str, t.List[ModelTestMetadata]] = {} + for fqn, model in self._models.items(): + if model._path is not None: + tests_by_path.setdefault(os.path.abspath(model._path), []).extend( + tests_by_model_name.get(fqn, []) + ) + + return tests_by_path + + def _select_tests_by_test_path(self, selector: str) -> t.Optional[t.List[ModelTestMetadata]]: + """Resolve a selector against the test files, or return None if it matches none of them. + + The selector is a test file path or a `path::test_name`. Paths are matched as given + first, so an unchanged selector never pays for normalization. + """ + if "::" in selector: + metadata = self._model_test_metadata_fully_qualified_name_index.get(selector) + if metadata is None: + path, _, test_name = selector.rpartition("::") + metadata = self._model_test_metadata_fully_qualified_name_index.get( + f"{os.path.abspath(path)}::{test_name}" + ) + return [metadata] if metadata is not None else None + + for path in (Path(selector), Path(os.path.abspath(selector))): + matched = self._model_test_metadata_path_index.get(path) + if matched is not None: + return list(matched) + + return None + def select_tests( self, tests: t.Optional[t.List[str]] = None, patterns: t.Optional[t.List[str]] = None, model_names: t.Optional[t.Collection[str]] = None, + raise_on_unknown_paths: bool = False, ) -> t.List[ModelTestMetadata]: - """Filter pre-loaded test metadata based on tests and patterns.""" + """Filter pre-loaded test metadata based on tests and patterns. + + Args: + tests: Test selectors. Each one is a test file path, a `path::test_name`, or the path + of a model file, in which case that model's tests are selected. Selectors are + unioned and the result is deduplicated, so a model file and a test file that + resolve to the same test run it once rather than twice. + patterns: Patterns matched against fully qualified test names. + model_names: If given, narrows the selection to tests targeting these models. + raise_on_unknown_paths: Whether to raise when a selector matches neither a known test + nor a known model file. Off by default so that callers which probe arbitrary + documents, such as the LSP, keep getting an empty result instead of an error. + """ test_meta = self._model_test_metadata if tests: - filtered_tests = [] + filtered_tests: t.List[ModelTestMetadata] = [] + # Built at most once, and only if a selector turns out not to be a test file. + tests_by_model_path: t.Optional[t.Dict[str, t.List[ModelTestMetadata]]] = None + for test in tests: - if "::" in test: - if test in self._model_test_metadata_fully_qualified_name_index: - filtered_tests.append( - self._model_test_metadata_fully_qualified_name_index[test] - ) - else: - test_path = Path(test) - if test_path in self._model_test_metadata_path_index: - filtered_tests.extend(self._model_test_metadata_path_index[test_path]) + matched = self._select_tests_by_test_path(test) + if matched is None and "::" not in test: + if tests_by_model_path is None: + tests_by_model_path = self._tests_by_absolute_model_path() + # A known model with no tests matches an empty list, which is not the same + # as a selector that resolves to nothing at all. + matched = tests_by_model_path.get(os.path.abspath(test)) + if matched is None: + if raise_on_unknown_paths: + raise SQLMeshError(f"'{test}' is not a known model or test file.") + continue + filtered_tests.extend(matched) - test_meta = filtered_tests + # Selectors can overlap, e.g. a model file and the test file holding its tests, so + # the union is deduplicated to avoid running the same test more than once. + test_meta = unique(filtered_tests) if patterns: test_meta = filter_tests_by_patterns(test_meta, patterns) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index da73952991..e23194a1b4 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -2611,3 +2611,24 @@ def test_format_does_not_open_state_connection( result = runner.invoke(cli, ["--paths", str(tmp_path), "format"]) assert result.exit_code == 0, f"Format failed: {result.output}\nException: {result.exception}" mock.assert_not_called() + + +def test_test_accepts_model_paths(runner: CliRunner, tmp_path: Path) -> None: + create_example_project(tmp_path) + + result = runner.invoke( + cli, ["--paths", str(tmp_path), "test", str(tmp_path / "models" / "full_model.sql")] + ) + assert result.exit_code == 0, f"Test failed: {result.output}\nException: {result.exception}" + assert "Ran 1 test" in result.output + + +def test_test_unknown_path_fails(runner: CliRunner, tmp_path: Path) -> None: + """A staged file that resolves to nothing must fail rather than silently run no tests.""" + create_example_project(tmp_path) + + result = runner.invoke( + cli, ["--paths", str(tmp_path), "test", str(tmp_path / "models" / "nope.sql")] + ) + assert result.exit_code != 0 + assert "is not a known model or test file" in result.output diff --git a/tests/core/test_test.py b/tests/core/test_test.py index 7c67192a14..3e3ceb6d14 100644 --- a/tests/core/test_test.py +++ b/tests/core/test_test.py @@ -2687,6 +2687,106 @@ def test_number_of_tests_found(tmp_path: Path) -> None: assert len(results.successes) == 1 +def test_model_path_selects_its_tests(tmp_path: Path) -> None: + """A model file path selects that model's tests, even though the YAML path wasn't given.""" + init_example_project(tmp_path, engine_type="duckdb") + context = Context(paths=tmp_path) + + results = context.test(tests=[str(tmp_path / "models" / "full_model.sql")]) + assert len(results.successes) == 1 + assert results.testsRun == 1 + + +def test_model_path_without_tests_selects_nothing(tmp_path: Path) -> None: + """A known model that simply has no tests is not an error.""" + init_example_project(tmp_path, engine_type="duckdb") + context = Context(paths=tmp_path) + + results = context.test(tests=[str(tmp_path / "models" / "incremental_model.sql")]) + assert results.testsRun == 0 + assert results.wasSuccessful() + + +def test_model_and_test_paths_are_unioned_without_duplicates(tmp_path: Path) -> None: + """Overlapping selectors must not run the same test twice.""" + init_example_project(tmp_path, engine_type="duckdb") + context = Context(paths=tmp_path) + + model_path = str(tmp_path / "models" / "full_model.sql") + test_path = str(tmp_path / "tests" / "test_full_model.yaml") + + # The YAML holds full_model's only test, so both selectors resolve to the same test. + assert context.test(tests=[model_path]).testsRun == 1 + assert context.test(tests=[test_path]).testsRun == 1 + assert context.test(tests=[model_path, test_path]).testsRun == 1 + + +def test_overlapping_yaml_and_named_test_are_deduplicated(tmp_path: Path) -> None: + """`file.yaml::name` is a subset of `file.yaml`, so together they're still one run.""" + init_example_project(tmp_path, engine_type="duckdb") + context = Context(paths=tmp_path) + + test_path = str(tmp_path / "tests" / "test_full_model.yaml") + results = context.test(tests=[f"{test_path}::test_example_full_model", test_path]) + assert results.testsRun == 1 + + +def test_relative_paths_select_tests(tmp_path: Path, monkeypatch) -> None: + """Pre-commit passes paths relative to the repo root, not absolute ones.""" + init_example_project(tmp_path, engine_type="duckdb") + context = Context(paths=tmp_path) + + monkeypatch.chdir(tmp_path) + assert context.test(tests=["models/full_model.sql"]).testsRun == 1 + assert context.test(tests=["tests/test_full_model.yaml"]).testsRun == 1 + + +def test_unknown_path_is_ignored_by_default(tmp_path: Path) -> None: + """Default behavior is unchanged, so the LSP can keep probing arbitrary documents.""" + init_example_project(tmp_path, engine_type="duckdb") + context = Context(paths=tmp_path) + + assert context.select_tests(tests=[str(tmp_path / "models" / "nope.sql")]) == [] + + +def test_unknown_path_errors_when_requested(tmp_path: Path) -> None: + """A path that is neither a known model nor a known test file must not pass silently.""" + init_example_project(tmp_path, engine_type="duckdb") + context = Context(paths=tmp_path) + + missing = tmp_path / "models" / "nope.sql" + with pytest.raises(SQLMeshError, match="is not a known model or test file"): + context.select_tests(tests=[str(missing)], raise_on_unknown_paths=True) + + with pytest.raises(SQLMeshError, match="is not a known model or test file"): + context.test(tests=[str(missing)], raise_on_unknown_paths=True) + + +def test_unknown_test_name_errors_when_requested(tmp_path: Path) -> None: + """A known YAML file with an unknown `::test_name` is just as wrong as a bad path.""" + init_example_project(tmp_path, engine_type="duckdb") + context = Context(paths=tmp_path) + + test_path = tmp_path / "tests" / "test_full_model.yaml" + with pytest.raises(SQLMeshError, match="is not a known model or test file"): + context.select_tests(tests=[f"{test_path}::nope"], raise_on_unknown_paths=True) + + +def test_select_model_still_filters_path_selection(tmp_path: Path) -> None: + """`--select-model` keeps narrowing the selection rather than adding to it.""" + init_example_project(tmp_path, engine_type="duckdb") + context = Context(paths=tmp_path) + + model_path = str(tmp_path / "models" / "full_model.sql") + assert ( + context.test(tests=[model_path], model_names=["sqlmesh_example.full_model"]).testsRun == 1 + ) + assert ( + context.test(tests=[model_path], model_names=["sqlmesh_example.incremental_model"]).testsRun + == 0 + ) + + def test_freeze_time_concurrent(tmp_path: Path) -> None: tests_dir = tmp_path / "tests" tests_dir.mkdir() From 14f1567ec646dbf358ed743b055eddb86372c6b1 Mon Sep 17 00:00:00 2001 From: Adegbite Ayoade Date: Fri, 18 Sep 2026 14:55:04 +0100 Subject: [PATCH 2/2] fix(test): distinguish an unknown test name from an unknown file Review feedback on #6061. A `path::test_name` selector that fails because the file is not a test file and one that fails because the file has no such test both reported "is not a known model or test file", which points at the wrong thing in the second case. The two are now told apart: if the path resolves to a known test file, the error names the test and the file it looked in. Otherwise the message is unchanged. Also adds Python model coverage for path selection, both in isolation and combined with --local, since selection is by file path and there was nothing pinning that .py behaves the same as .sql. Separately, renames a loop variable in _select_tests_by_test_path. It shadowed a str binding from the branch above with a Path, which mypy rejects; it was pre-existing on this branch rather than introduced here. Signed-off-by: Adegbite Ayoade --- sqlmesh/core/context.py | 22 ++++++++++++++--- tests/cli/test_cli.py | 51 ++++++++++++++++++++++++++++++++++++++ tests/core/test_test.py | 55 +++++++++++++++++++++++++++++++++++++++-- 3 files changed, 123 insertions(+), 5 deletions(-) diff --git a/sqlmesh/core/context.py b/sqlmesh/core/context.py index 9d4650d2de..64dd6d67da 100644 --- a/sqlmesh/core/context.py +++ b/sqlmesh/core/context.py @@ -3659,13 +3659,29 @@ def _select_tests_by_test_path(self, selector: str) -> t.Optional[t.List[ModelTe ) return [metadata] if metadata is not None else None - for path in (Path(selector), Path(os.path.abspath(selector))): - matched = self._model_test_metadata_path_index.get(path) + for candidate in (Path(selector), Path(os.path.abspath(selector))): + matched = self._model_test_metadata_path_index.get(candidate) if matched is not None: return list(matched) return None + def _unknown_test_selector_error(self, selector: str) -> str: + """Explains why a selector matched nothing. + + A `path::test_name` whose file is a known test file failed on the test name, not the + path, so the message says so rather than claiming the file is unknown. + """ + if "::" in selector: + path, _, _ = selector.rpartition("::") + if any( + candidate in self._model_test_metadata_path_index + for candidate in (Path(path), Path(os.path.abspath(path))) + ): + return f"'{selector}' is not a known test in '{path}'." + + return f"'{selector}' is not a known model or test file." + def select_tests( self, tests: t.Optional[t.List[str]] = None, @@ -3704,7 +3720,7 @@ def select_tests( matched = tests_by_model_path.get(os.path.abspath(test)) if matched is None: if raise_on_unknown_paths: - raise SQLMeshError(f"'{test}' is not a known model or test file.") + raise SQLMeshError(self._unknown_test_selector_error(test)) continue filtered_tests.extend(matched) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 5b74c892e5..251ea54fa3 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -2820,3 +2820,54 @@ def test_test_local_with_model_paths(runner: CliRunner, tmp_path: Path, mocker) assert result.exit_code != 0 assert "is not a known model or test file" in result.output mock.assert_not_called() + + +def test_test_local_with_python_model_paths(runner: CliRunner, tmp_path: Path, mocker) -> None: + """The `--local` + path-selector combination works for Python models too.""" + create_example_project(tmp_path) + + (tmp_path / "models" / "py_model.py").write_text( + """ +import pandas as pd # noqa: TID253 +from sqlmesh import model, ExecutionContext +import typing as t + +@model( + name="sqlmesh_example.py_model", + columns={"id": "int"}, +) +def execute(context: ExecutionContext, **kwargs: t.Any) -> pd.DataFrame: + return pd.DataFrame([{"id": 1}]) +""", + encoding="utf-8", + ) + (tmp_path / "tests" / "test_py_model.yaml").write_text( + """ +test_py_model: + model: sqlmesh_example.py_model + outputs: + query: + rows: + - id: 1 +""", + encoding="utf-8", + ) + + mock = _patch_state_access(mocker) + + result = runner.invoke( + cli, + ["--paths", str(tmp_path), "test", "--local", str(tmp_path / "models" / "py_model.py")], + ) + + assert result.exit_code == 0, f"Test failed: {result.output}\nException: {result.exception}" + assert "Successfully Ran 1 tests" in " ".join(result.output.split()) + mock.assert_not_called() + + # A Python file that is not a model is still an error rather than a silent no-op. + result = runner.invoke( + cli, ["--paths", str(tmp_path), "test", "--local", str(tmp_path / "models" / "nope.py")] + ) + assert result.exit_code != 0 + assert "is not a known model or test file" in result.output + mock.assert_not_called() diff --git a/tests/core/test_test.py b/tests/core/test_test.py index 3e3ceb6d14..0fbe8e926c 100644 --- a/tests/core/test_test.py +++ b/tests/core/test_test.py @@ -1,6 +1,7 @@ from __future__ import annotations import datetime +import re import typing as t import io from pathlib import Path @@ -2697,6 +2698,46 @@ def test_model_path_selects_its_tests(tmp_path: Path) -> None: assert results.testsRun == 1 +def test_python_model_path_selects_its_tests(tmp_path: Path) -> None: + """Selection is by file path, so a Python model works the same way a SQL one does.""" + init_example_project(tmp_path, engine_type="duckdb") + + py_model = tmp_path / "models" / "py_model.py" + py_model.write_text( + """ +import pandas as pd # noqa: TID253 +from sqlmesh import model, ExecutionContext +import typing as t + +@model( + name="sqlmesh_example.py_model", + columns={"id": "int"}, +) +def execute(context: ExecutionContext, **kwargs: t.Any) -> pd.DataFrame: + return pd.DataFrame([{"id": 1}]) +""" + ) + (tmp_path / "tests" / "test_py_model.yaml").write_text( + """ +test_py_model: + model: sqlmesh_example.py_model + outputs: + query: + rows: + - id: 1 +""" + ) + + context = Context(paths=tmp_path) + + results = context.test(tests=[str(py_model)]) + assert results.testsRun == 1 + assert len(results.successes) == 1 + + # The SQL model's own test is not pulled in by selecting the Python model. + assert context.test(tests=[str(tmp_path / "models" / "full_model.sql")]).testsRun == 1 + + def test_model_path_without_tests_selects_nothing(tmp_path: Path) -> None: """A known model that simply has no tests is not an error.""" init_example_project(tmp_path, engine_type="duckdb") @@ -2763,15 +2804,25 @@ def test_unknown_path_errors_when_requested(tmp_path: Path) -> None: def test_unknown_test_name_errors_when_requested(tmp_path: Path) -> None: - """A known YAML file with an unknown `::test_name` is just as wrong as a bad path.""" + """A known YAML file with an unknown `::test_name` reports the test, not the file.""" init_example_project(tmp_path, engine_type="duckdb") context = Context(paths=tmp_path) test_path = tmp_path / "tests" / "test_full_model.yaml" - with pytest.raises(SQLMeshError, match="is not a known model or test file"): + with pytest.raises(SQLMeshError, match=f"is not a known test in '{re.escape(str(test_path))}'"): context.select_tests(tests=[f"{test_path}::nope"], raise_on_unknown_paths=True) +def test_unknown_test_name_in_unknown_file_reports_the_file(tmp_path: Path) -> None: + """A `::test_name` on a file that isn't a test file is a path problem, not a name one.""" + init_example_project(tmp_path, engine_type="duckdb") + context = Context(paths=tmp_path) + + missing = tmp_path / "tests" / "test_nope.yaml" + with pytest.raises(SQLMeshError, match="is not a known model or test file"): + context.select_tests(tests=[f"{missing}::nope"], raise_on_unknown_paths=True) + + def test_select_model_still_filters_path_selection(tmp_path: Path) -> None: """`--select-model` keeps narrowing the selection rather than adding to it.""" init_example_project(tmp_path, engine_type="duckdb")