diff --git a/docs/concepts/tests.md b/docs/concepts/tests.md index a9bd45a25a..8bf0fe0cbf 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. + You can pass `--local` to run tests without loading state from the configured state connection: ``` bash diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 1ed03535b9..f7943e2f71 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 c2c81ada9c..fc6cad8011 100644 --- a/sqlmesh/cli/main.py +++ b/sqlmesh/cli/main.py @@ -835,7 +835,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 ) @@ -845,6 +850,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 b52df5394a..5b74c892e5 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -2666,6 +2666,27 @@ def test_format_does_not_open_state_connection( 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 + + def test_test_local_runs_project_unit_tests(runner: CliRunner, tmp_path: Path, mocker) -> None: """A real unit test from the project's YAML runs under `--local` without touching state.""" create_example_project(tmp_path) @@ -2776,3 +2797,26 @@ def test_test_local_multi_repo_partial(runner: CliRunner, copy_to_temp_path, moc ) assert "Successfully Ran 1 tests" in output, "the repo_2 test should still run" mock.assert_not_called() + + +def test_test_local_with_model_paths(runner: CliRunner, tmp_path: Path, mocker) -> None: + """`--local` and model path selectors compose, which is the pre-commit hook case in #6020.""" + create_example_project(tmp_path) + mock = _patch_state_access(mocker) + + result = runner.invoke( + cli, + ["--paths", str(tmp_path), "test", "--local", str(tmp_path / "models" / "full_model.sql")], + ) + + 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() + + # An unresolvable path still fails loudly, without reaching state. + result = runner.invoke( + cli, ["--paths", str(tmp_path), "test", "--local", str(tmp_path / "models" / "nope.sql")] + ) + 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 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()