From 2d81994ebf514bfd9a137aacf930584b6f557a31 Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Tue, 7 Nov 2023 18:38:30 -0500 Subject: [PATCH 1/7] Add unit tests to state:modified selection --- core/dbt/contracts/graph/nodes.py | 11 +++ core/dbt/graph/selector_methods.py | 12 ++- core/dbt/parser/unit_tests.py | 2 + .../unit_testing/test_unit_testing.py | 75 ++++++++++++++++++- tests/unit/test_unit_test_parser.py | 1 + 5 files changed, 98 insertions(+), 3 deletions(-) diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index 3c56d784ed8..a041838dcea 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -1080,6 +1080,7 @@ class UnitTestDefinition(GraphNode): overrides: Optional[UnitTestOverrides] = None depends_on: DependsOn = field(default_factory=DependsOn) config: UnitTestConfig = field(default_factory=UnitTestConfig) + checksum: Optional[str] = None @property def depends_on_nodes(self): @@ -1090,6 +1091,16 @@ def tags(self) -> List[str]: tags = self.config.tags return [tags] if isinstance(tags, str) else tags + def build_unit_test_checksum(self): + # everything except 'description' + data = f"{self.model}-{self.given}-{self.expect}-{self.overrides}".encode("utf-8") + self.checksum = hashlib.new("sha256", data).hexdigest() + + def same_contents(self, other: Optional["UnitTestDefinition"]) -> bool: + if other is None: + return False + return self.checksum == other.checksum + # ==================================== # Snapshot node diff --git a/core/dbt/graph/selector_methods.py b/core/dbt/graph/selector_methods.py index bf9820b13ed..c8007cdab4f 100644 --- a/core/dbt/graph/selector_methods.py +++ b/core/dbt/graph/selector_methods.py @@ -102,7 +102,9 @@ def is_selected_node(fqn: List[str], node_selector: str, is_versioned: bool) -> return True -SelectorTarget = Union[SourceDefinition, ManifestNode, Exposure, Metric] +SelectorTarget = Union[ + SourceDefinition, ManifestNode, Exposure, Metric, SemanticModel, UnitTestDefinition +] class SelectorMethod(metaclass=abc.ABCMeta): @@ -639,7 +641,9 @@ def check_macros_modified(self, node): def check_modified_content( self, old: Optional[SelectorTarget], new: SelectorTarget, adapter_type: str ) -> bool: - if isinstance(new, (SourceDefinition, Exposure, Metric, SemanticModel)): + if isinstance( + new, (SourceDefinition, Exposure, Metric, SemanticModel, UnitTestDefinition) + ): # these all overwrite `same_contents` different_contents = not new.same_contents(old) # type: ignore else: @@ -730,6 +734,10 @@ def search(self, included_nodes: Set[UniqueId], selector: str) -> Iterator[Uniqu previous_node = manifest.exposures[node] elif node in manifest.metrics: previous_node = manifest.metrics[node] + elif node in manifest.semantic_models: + previous_node = manifest.semantic_models[node] + elif node in manifest.unit_tests: + previous_node = manifest.unit_tests[node] keyword_args = {} if checker.__name__ in [ diff --git a/core/dbt/parser/unit_tests.py b/core/dbt/parser/unit_tests.py index a2bdb8852d8..8898fdb8358 100644 --- a/core/dbt/parser/unit_tests.py +++ b/core/dbt/parser/unit_tests.py @@ -229,6 +229,8 @@ def parse(self) -> ParseResult: fqn=unit_test_fqn, config=unit_test_config, ) + # for calculating state:modified + unit_test_definition.build_unit_test_checksum() self.manifest.add_unit_test(self.yaml.file, unit_test_definition) return ParseResult() diff --git a/tests/functional/unit_testing/test_unit_testing.py b/tests/functional/unit_testing/test_unit_testing.py index 5c3055525d0..8a7c3d993d0 100644 --- a/tests/functional/unit_testing/test_unit_testing.py +++ b/tests/functional/unit_testing/test_unit_testing.py @@ -1,5 +1,13 @@ import pytest -from dbt.tests.util import run_dbt, write_file, get_manifest, get_artifact +import os +import shutil +from dbt.tests.util import ( + run_dbt, + write_file, + get_manifest, + get_artifact, + write_config_file, +) from dbt.exceptions import DuplicateResourceNameError, ParsingError, YamlParseDictError my_model_sql = """ @@ -416,3 +424,68 @@ def test_basic(self, project): # Select by model name results = run_dbt(["unit-test", "--select", "my_incremental_model"], expect_pass=True) assert len(results) == 2 + + +class TestUnitTestStateModified: + @pytest.fixture(scope="class") + def models(self): + return { + "my_model.sql": my_model_sql, + "my_model_a.sql": my_model_a_sql, + "my_model_b.sql": my_model_b_sql, + "test_my_model.yml": test_my_model_yml, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return {"vars": {"my_test": "my_test_var"}} + + def copy_state(self, project_root): + state_path = os.path.join(project_root, "state") + if not os.path.exists(state_path): + os.makedirs(state_path) + shutil.copyfile( + f"{project_root}/target/manifest.json", f"{project_root}/state/manifest.json" + ) + shutil.copyfile( + f"{project_root}/target/run_results.json", f"{project_root}/state/run_results.json" + ) + + def test_state_modified(self, project): + run_dbt(["run"]) + run_dbt(["unit-test"], expect_pass=False) + self.copy_state(project.project_root) + + # no changes + results = run_dbt(["unit-test", "--select", "state:modified", "--state", "state"]) + assert len(results) == 0 + + # change unit test definition + with_changes = test_my_model_yml.replace("{string_c: ab}", "{string_c: bc}") + write_config_file(with_changes, project.project_root, "models", "test_my_model.yml") + results = run_dbt( + ["unit-test", "--select", "state:modified", "--state", "state"], expect_pass=False + ) + assert len(results) == 1 + + # change underlying model logic + write_config_file(test_my_model_yml, project.project_root, "models", "test_my_model.yml") + write_file( + my_model_sql.replace("a+b as c,", "a + b as c,"), + project.project_root, + "models", + "my_model.sql", + ) + results = run_dbt( + ["unit-test", "--select", "state:modified", "--state", "state"], expect_pass=False + ) + assert len(results) == 4 + + def test_retry(self, project): + run_dbt(["run"]) + run_dbt(["unit-test"], expect_pass=False) + self.copy_state(project.project_root) + + # no changes + results = run_dbt(["retry"], expect_pass=False) + assert len(results) == 1 diff --git a/tests/unit/test_unit_test_parser.py b/tests/unit/test_unit_test_parser.py index a49b3b69eb2..32211885486 100644 --- a/tests/unit/test_unit_test_parser.py +++ b/tests/unit/test_unit_test_parser.py @@ -132,6 +132,7 @@ def test_basic(self): fqn=["snowplow", "my_model", "test_my_model"], config=UnitTestConfig(), ) + expected.build_unit_test_checksum() assertEqualNodes(unit_test, expected) def test_unit_test_config(self): From f70ff8cfd8417eacb16bbaca8d01bf678b99e35f Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Tue, 7 Nov 2023 20:33:51 -0500 Subject: [PATCH 2/7] Get defer working too yolo --- core/dbt/parser/unit_tests.py | 15 ++++++++-- core/dbt/task/unit_test.py | 11 ++++++- .../unit_testing/test_unit_testing.py | 30 +++++++++++++++++-- 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/core/dbt/parser/unit_tests.py b/core/dbt/parser/unit_tests.py index 8898fdb8358..3fa6e8a6a58 100644 --- a/core/dbt/parser/unit_tests.py +++ b/core/dbt/parser/unit_tests.py @@ -4,7 +4,7 @@ from dbt.context.context_config import ContextConfig from dbt.context.providers import generate_parse_exposure, get_rendered from dbt.contracts.files import FileHash -from dbt.contracts.graph.manifest import Manifest +from dbt.contracts.graph.manifest import Manifest, WritableManifest from dbt.contracts.graph.model_config import UnitTestNodeConfig, ModelConfig from dbt.contracts.graph.nodes import ( ModelNode, @@ -31,14 +31,25 @@ class UnitTestManifestLoader: - def __init__(self, manifest, root_project, selected) -> None: + def __init__( + self, manifest, root_project, adapter, selected, defer_manifest, favor_state + ) -> None: self.manifest: Manifest = manifest self.root_project: RuntimeConfig = root_project # selected comes from the initial selection against a "regular" manifest + self.adapter = adapter self.selected: Set[UniqueId] = selected + self.defer_manifest: WritableManifest = defer_manifest + self.favor_state = favor_state self.unit_test_manifest = Manifest(macros=manifest.macros) def load(self) -> Manifest: + if self.defer_manifest: + # This is a little bit gross! + adapter = self.adapter + with adapter.connection_named("master"): + self.manifest.merge_from_artifact(adapter, self.defer_manifest, self.selected) + for unique_id in self.selected: unit_test_case = self.manifest.unit_tests[unique_id] self.parse_unit_test_case(unit_test_case) diff --git a/core/dbt/task/unit_test.py b/core/dbt/task/unit_test.py index 9697e2e873f..519f60115c6 100644 --- a/core/dbt/task/unit_test.py +++ b/core/dbt/task/unit_test.py @@ -7,6 +7,7 @@ from .compile import CompileRunner from .run import RunTask +from dbt.adapters.factory import get_adapter from dbt.contracts.graph.nodes import UnitTestNode from dbt.contracts.graph.manifest import Manifest from dbt.contracts.results import TestStatus, RunResult @@ -202,7 +203,15 @@ def exclusion_arg(self): return () def build_unit_test_manifest(self): - loader = UnitTestManifestLoader(self.manifest, self.config, self.job_queue._selected) + adapter = get_adapter(self.config) + loader = UnitTestManifestLoader( + self.manifest, + self.config, + adapter, + self.job_queue._selected, + self._get_deferred_manifest(), + favor_state=bool(self.args.favor_state), + ) return loader.load() def reset_job_queue_and_manifest(self): diff --git a/tests/functional/unit_testing/test_unit_testing.py b/tests/functional/unit_testing/test_unit_testing.py index 8a7c3d993d0..27b0a1fc6a3 100644 --- a/tests/functional/unit_testing/test_unit_testing.py +++ b/tests/functional/unit_testing/test_unit_testing.py @@ -1,6 +1,7 @@ import pytest import os import shutil +from copy import deepcopy from dbt.tests.util import ( run_dbt, write_file, @@ -426,7 +427,7 @@ def test_basic(self, project): assert len(results) == 2 -class TestUnitTestStateModified: +class UnitTestState: @pytest.fixture(scope="class") def models(self): return { @@ -451,6 +452,8 @@ def copy_state(self, project_root): f"{project_root}/target/run_results.json", f"{project_root}/state/run_results.json" ) + +class TestUnitTestStateModified(UnitTestState): def test_state_modified(self, project): run_dbt(["run"]) run_dbt(["unit-test"], expect_pass=False) @@ -481,11 +484,32 @@ def test_state_modified(self, project): ) assert len(results) == 4 - def test_retry(self, project): + +class TestUnitTestRetry(UnitTestState): + def test_unit_test_retry(self, project): run_dbt(["run"]) run_dbt(["unit-test"], expect_pass=False) self.copy_state(project.project_root) - # no changes results = run_dbt(["retry"], expect_pass=False) assert len(results) == 1 + + +class TestUnitTestDeferState(UnitTestState): + @pytest.fixture(scope="class") + def other_schema(self, unique_schema): + return unique_schema + "_other" + + @pytest.fixture(scope="class") + def profiles_config_update(self, dbt_profile_target, unique_schema, other_schema): + outputs = {"default": dbt_profile_target, "otherschema": deepcopy(dbt_profile_target)} + outputs["default"]["schema"] = unique_schema + outputs["otherschema"]["schema"] = other_schema + return {"test": {"outputs": outputs, "target": "default"}} + + def test_unit_test_defer_state(self, project): + run_dbt(["run", "--target", "otherschema"]) + self.copy_state(project.project_root) + results = run_dbt(["unit-test", "--defer", "--state", "state"], expect_pass=False) + assert len(results) == 4 + assert sorted([r.status for r in results]) == ["fail", "pass", "pass", "pass"] From 3d2f98863a067eac5b0fe2818d228f35fac5148e Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Tue, 7 Nov 2023 23:09:17 -0500 Subject: [PATCH 3/7] Refactor per marky suggestion --- core/dbt/parser/unit_tests.py | 15 ++------------- core/dbt/task/unit_test.py | 24 ++++++++++++++---------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/core/dbt/parser/unit_tests.py b/core/dbt/parser/unit_tests.py index 3fa6e8a6a58..8898fdb8358 100644 --- a/core/dbt/parser/unit_tests.py +++ b/core/dbt/parser/unit_tests.py @@ -4,7 +4,7 @@ from dbt.context.context_config import ContextConfig from dbt.context.providers import generate_parse_exposure, get_rendered from dbt.contracts.files import FileHash -from dbt.contracts.graph.manifest import Manifest, WritableManifest +from dbt.contracts.graph.manifest import Manifest from dbt.contracts.graph.model_config import UnitTestNodeConfig, ModelConfig from dbt.contracts.graph.nodes import ( ModelNode, @@ -31,25 +31,14 @@ class UnitTestManifestLoader: - def __init__( - self, manifest, root_project, adapter, selected, defer_manifest, favor_state - ) -> None: + def __init__(self, manifest, root_project, selected) -> None: self.manifest: Manifest = manifest self.root_project: RuntimeConfig = root_project # selected comes from the initial selection against a "regular" manifest - self.adapter = adapter self.selected: Set[UniqueId] = selected - self.defer_manifest: WritableManifest = defer_manifest - self.favor_state = favor_state self.unit_test_manifest = Manifest(macros=manifest.macros) def load(self) -> Manifest: - if self.defer_manifest: - # This is a little bit gross! - adapter = self.adapter - with adapter.connection_named("master"): - self.manifest.merge_from_artifact(adapter, self.defer_manifest, self.selected) - for unique_id in self.selected: unit_test_case = self.manifest.unit_tests[unique_id] self.parse_unit_test_case(unit_test_case) diff --git a/core/dbt/task/unit_test.py b/core/dbt/task/unit_test.py index 519f60115c6..b8e66223707 100644 --- a/core/dbt/task/unit_test.py +++ b/core/dbt/task/unit_test.py @@ -1,7 +1,7 @@ from dataclasses import dataclass from dbt.dataclass_schema import dbtClassMixin import threading -from typing import Dict, Any, Optional +from typing import Dict, Any, Optional, AbstractSet import io from .compile import CompileRunner @@ -203,18 +203,17 @@ def exclusion_arg(self): return () def build_unit_test_manifest(self): - adapter = get_adapter(self.config) - loader = UnitTestManifestLoader( - self.manifest, - self.config, - adapter, - self.job_queue._selected, - self._get_deferred_manifest(), - favor_state=bool(self.args.favor_state), - ) + loader = UnitTestManifestLoader(self.manifest, self.config, self.job_queue._selected) return loader.load() def reset_job_queue_and_manifest(self): + # We want deferral to happen here (earlier than normal) before we turn + # the normal manifest into the unit testing manifest + adapter = get_adapter(self.config) + with adapter.connection_named("master"): + self.populate_adapter_cache(adapter) + self.defer_to_manifest(adapter, self.job_queue._selected) + # We have the selected models from the "regular" manifest, now we switch # to using the unit_test_manifest to run the unit tests. self.using_unit_test_manifest = True @@ -222,6 +221,11 @@ def reset_job_queue_and_manifest(self): self.compile_manifest() # create the networkx graph self.job_queue = self.get_graph_queue() + def before_run(self, adapter, selected_uids: AbstractSet[str]) -> None: + # We already did cache population + deferral earlier (above) + # and we don't need to create any schemas + pass + def get_node_selector(self) -> ResourceTypeSelector: if self.manifest is None or self.graph is None: raise DbtInternalError("manifest and graph must be set to get perform node selection") From 0c790a4a1e8b0a5b2885df06d5f8a6a3985d6f87 Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Tue, 7 Nov 2023 23:10:13 -0500 Subject: [PATCH 4/7] Add changelog --- .changes/unreleased/Features-20231107-231006.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Features-20231107-231006.yaml diff --git a/.changes/unreleased/Features-20231107-231006.yaml b/.changes/unreleased/Features-20231107-231006.yaml new file mode 100644 index 00000000000..0865c72cc58 --- /dev/null +++ b/.changes/unreleased/Features-20231107-231006.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Unit tests support --defer and state:modified +time: 2023-11-07T23:10:06.376588-05:00 +custom: + Author: jtcohen6 + Issue: "8517" From 475e1ebd4469a653a4bcd1114fb55c18462732e9 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Sun, 12 Nov 2023 18:39:56 -0500 Subject: [PATCH 5/7] separate out unit test state tests + fix csv fixture tests --- tests/functional/unit_testing/fixtures.py | 86 +++++++++++-- tests/functional/unit_testing/test_state.py | 114 ++++++++++++++++++ .../unit_testing/test_unit_testing.py | 92 -------------- 3 files changed, 188 insertions(+), 104 deletions(-) create mode 100644 tests/functional/unit_testing/test_state.py diff --git a/tests/functional/unit_testing/fixtures.py b/tests/functional/unit_testing/fixtures.py index 9018407baa9..cd4d85d699e 100644 --- a/tests/functional/unit_testing/fixtures.py +++ b/tests/functional/unit_testing/fixtures.py @@ -105,6 +105,73 @@ tags: test_this """ + +test_my_model_simple_fixture_yml = """ +unit_tests: + - name: test_my_model + model: my_model + given: + - input: ref('my_model_a') + rows: + - {id: 1, a: 1} + - input: ref('my_model_b') + rows: + - {id: 1, b: 2} + - {id: 2, b: 2} + expect: + rows: + - {c: 2} + + - name: test_my_model_empty + model: my_model + given: + - input: ref('my_model_a') + rows: [] + - input: ref('my_model_b') + format: csv + fixture: test_my_model_fixture + expect: + rows: [] + + - name: test_my_model_overrides + model: my_model + given: + - input: ref('my_model_a') + rows: + - {id: 1, a: 1} + - input: ref('my_model_b') + rows: + - {id: 1, b: 2} + - {id: 2, b: 2} + overrides: + macros: + type_numeric: override + invocation_id: 123 + vars: + my_test: var_override + env_vars: + MY_TEST: env_var_override + expect: + rows: + - {macro_call: override, var_call: var_override, env_var_call: env_var_override, invocation_id: 123} + + - name: test_my_model_string_concat + model: my_model + given: + - input: ref('my_model_a') + rows: + - {id: 1, string_a: a} + - input: ref('my_model_b') + rows: + - {id: 1, string_b: b} + expect: + rows: + - {string_c: ab} + config: + tags: test_this +""" + + datetime_test = """ - name: test_my_model_datetime model: my_model @@ -381,36 +448,31 @@ tags: test_this """ -test_my_model_fixture_csv = """ -id,b +test_my_model_fixture_csv = """id,b 1,2 2,2 """ -test_my_model_a_fixture_csv = """ -id,string_a +test_my_model_a_fixture_csv = """id,string_a 1,a """ test_my_model_a_empty_fixture_csv = """ """ -test_my_model_a_numeric_fixture_csv = """ -id,a +test_my_model_a_numeric_fixture_csv = """id,a 1,1 """ -test_my_model_b_fixture_csv = """ -id,string_b +test_my_model_b_fixture_csv = """id,string_b 1,b """ -test_my_model_basic_fixture_csv = """ -c + +test_my_model_basic_fixture_csv = """c 2 """ -test_my_model_concat_fixture_csv = """ -string_c +test_my_model_concat_fixture_csv = """string_c ab """ diff --git a/tests/functional/unit_testing/test_state.py b/tests/functional/unit_testing/test_state.py new file mode 100644 index 00000000000..b24f249fcfa --- /dev/null +++ b/tests/functional/unit_testing/test_state.py @@ -0,0 +1,114 @@ +import os +import pytest +import shutil +from copy import deepcopy + +from dbt.tests.util import ( + run_dbt, + write_file, + write_config_file, +) +from fixtures import ( + my_model_vars_sql, + my_model_a_sql, + my_model_b_sql, + test_my_model_simple_fixture_yml, + test_my_model_fixture_csv, + test_my_model_b_fixture_csv, +) + + +class UnitTestState: + @pytest.fixture(scope="class") + def models(self): + return { + "my_model.sql": my_model_vars_sql, + "my_model_a.sql": my_model_a_sql, + "my_model_b.sql": my_model_b_sql, + "test_my_model.yml": test_my_model_simple_fixture_yml, + } + + @pytest.fixture(scope="class") + def tests(self): + return { + "fixtures": { + "test_my_model_fixture.csv": test_my_model_fixture_csv, + } + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return {"vars": {"my_test": "my_test_var"}} + + def copy_state(self, project_root): + state_path = os.path.join(project_root, "state") + if not os.path.exists(state_path): + os.makedirs(state_path) + shutil.copyfile( + f"{project_root}/target/manifest.json", f"{project_root}/state/manifest.json" + ) + shutil.copyfile( + f"{project_root}/target/run_results.json", f"{project_root}/state/run_results.json" + ) + + +class TestUnitTestStateModified(UnitTestState): + def test_state_modified(self, project): + run_dbt(["run"]) + run_dbt(["unit-test"], expect_pass=False) + self.copy_state(project.project_root) + + # no changes + results = run_dbt(["unit-test", "--select", "state:modified", "--state", "state"]) + assert len(results) == 0 + + # change unit test definition of a single unit test + with_changes = test_my_model_simple_fixture_yml.replace("{string_c: ab}", "{string_c: bc}") + write_config_file(with_changes, project.project_root, "models", "test_my_model.yml") + results = run_dbt( + ["unit-test", "--select", "state:modified", "--state", "state"], expect_pass=False + ) + assert len(results) == 1 + + # change underlying model logic + write_config_file(test_my_model_simple_fixture_yml, project.project_root, "models", "test_my_model.yml") + write_file( + my_model_vars_sql.replace("a+b as c,", "a + b as c,"), + project.project_root, + "models", + "my_model.sql", + ) + results = run_dbt( + ["unit-test", "--select", "state:modified", "--state", "state"], expect_pass=False + ) + assert len(results) == 4 + + +class TestUnitTestRetry(UnitTestState): + def test_unit_test_retry(self, project): + run_dbt(["run"]) + run_dbt(["unit-test"], expect_pass=False) + self.copy_state(project.project_root) + + results = run_dbt(["retry"], expect_pass=False) + assert len(results) == 1 + + +class TestUnitTestDeferState(UnitTestState): + @pytest.fixture(scope="class") + def other_schema(self, unique_schema): + return unique_schema + "_other" + + @pytest.fixture(scope="class") + def profiles_config_update(self, dbt_profile_target, unique_schema, other_schema): + outputs = {"default": dbt_profile_target, "otherschema": deepcopy(dbt_profile_target)} + outputs["default"]["schema"] = unique_schema + outputs["otherschema"]["schema"] = other_schema + return {"test": {"outputs": outputs, "target": "default"}} + + def test_unit_test_defer_state(self, project): + run_dbt(["run", "--target", "otherschema"]) + self.copy_state(project.project_root) + results = run_dbt(["unit-test", "--defer", "--state", "state"], expect_pass=False) + assert len(results) == 4 + assert sorted([r.status for r in results]) == ["fail", "pass", "pass", "pass"] diff --git a/tests/functional/unit_testing/test_unit_testing.py b/tests/functional/unit_testing/test_unit_testing.py index c78616ab430..03074c3b543 100644 --- a/tests/functional/unit_testing/test_unit_testing.py +++ b/tests/functional/unit_testing/test_unit_testing.py @@ -1,13 +1,9 @@ import pytest -import os -import shutil -from copy import deepcopy from dbt.tests.util import ( run_dbt, write_file, get_manifest, get_artifact, - write_config_file, ) from dbt.exceptions import DuplicateResourceNameError from fixtures import ( @@ -109,91 +105,3 @@ def test_basic(self, project): # Select by model name results = run_dbt(["unit-test", "--select", "my_incremental_model"], expect_pass=True) assert len(results) == 2 - - -class UnitTestState: - @pytest.fixture(scope="class") - def models(self): - return { - "my_model.sql": my_model_vars_sql, - "my_model_a.sql": my_model_a_sql, - "my_model_b.sql": my_model_b_sql, - "test_my_model.yml": test_my_model_yml, - } - - @pytest.fixture(scope="class") - def project_config_update(self): - return {"vars": {"my_test": "my_test_var"}} - - def copy_state(self, project_root): - state_path = os.path.join(project_root, "state") - if not os.path.exists(state_path): - os.makedirs(state_path) - shutil.copyfile( - f"{project_root}/target/manifest.json", f"{project_root}/state/manifest.json" - ) - shutil.copyfile( - f"{project_root}/target/run_results.json", f"{project_root}/state/run_results.json" - ) - - -class TestUnitTestStateModified(UnitTestState): - def test_state_modified(self, project): - run_dbt(["run"]) - run_dbt(["unit-test"], expect_pass=False) - self.copy_state(project.project_root) - - # no changes - results = run_dbt(["unit-test", "--select", "state:modified", "--state", "state"]) - assert len(results) == 0 - - # change unit test definition - with_changes = test_my_model_yml.replace("{string_c: ab}", "{string_c: bc}") - write_config_file(with_changes, project.project_root, "models", "test_my_model.yml") - results = run_dbt( - ["unit-test", "--select", "state:modified", "--state", "state"], expect_pass=False - ) - assert len(results) == 1 - - # change underlying model logic - write_config_file(test_my_model_yml, project.project_root, "models", "test_my_model.yml") - write_file( - my_model_vars_sql.replace("a+b as c,", "a + b as c,"), - project.project_root, - "models", - "my_model.sql", - ) - results = run_dbt( - ["unit-test", "--select", "state:modified", "--state", "state"], expect_pass=False - ) - assert len(results) == 4 - - -class TestUnitTestRetry(UnitTestState): - def test_unit_test_retry(self, project): - run_dbt(["run"]) - run_dbt(["unit-test"], expect_pass=False) - self.copy_state(project.project_root) - - results = run_dbt(["retry"], expect_pass=False) - assert len(results) == 1 - - -class TestUnitTestDeferState(UnitTestState): - @pytest.fixture(scope="class") - def other_schema(self, unique_schema): - return unique_schema + "_other" - - @pytest.fixture(scope="class") - def profiles_config_update(self, dbt_profile_target, unique_schema, other_schema): - outputs = {"default": dbt_profile_target, "otherschema": deepcopy(dbt_profile_target)} - outputs["default"]["schema"] = unique_schema - outputs["otherschema"]["schema"] = other_schema - return {"test": {"outputs": outputs, "target": "default"}} - - def test_unit_test_defer_state(self, project): - run_dbt(["run", "--target", "otherschema"]) - self.copy_state(project.project_root) - results = run_dbt(["unit-test", "--defer", "--state", "state"], expect_pass=False) - assert len(results) == 4 - assert sorted([r.status for r in results]) == ["fail", "pass", "pass", "pass"] From 7e49d3bde373c8c3ec0c8603f4099af14a735364 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 13 Nov 2023 15:41:47 -0500 Subject: [PATCH 6/7] formatting --- tests/functional/unit_testing/test_state.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/functional/unit_testing/test_state.py b/tests/functional/unit_testing/test_state.py index b24f249fcfa..fe6528ae128 100644 --- a/tests/functional/unit_testing/test_state.py +++ b/tests/functional/unit_testing/test_state.py @@ -14,10 +14,9 @@ my_model_b_sql, test_my_model_simple_fixture_yml, test_my_model_fixture_csv, - test_my_model_b_fixture_csv, ) - + class UnitTestState: @pytest.fixture(scope="class") def models(self): @@ -71,7 +70,9 @@ def test_state_modified(self, project): assert len(results) == 1 # change underlying model logic - write_config_file(test_my_model_simple_fixture_yml, project.project_root, "models", "test_my_model.yml") + write_config_file( + test_my_model_simple_fixture_yml, project.project_root, "models", "test_my_model.yml" + ) write_file( my_model_vars_sql.replace("a+b as c,", "a + b as c,"), project.project_root, From 8c9eb48553948a228ac0be327d864593a92580c6 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 13 Nov 2023 18:32:37 -0500 Subject: [PATCH 7/7] detect changes to fixture files with state:modified --- core/dbt/contracts/graph/nodes.py | 13 ++++++++++--- core/dbt/parser/unit_tests.py | 4 +++- tests/functional/unit_testing/fixtures.py | 4 ++-- tests/functional/unit_testing/test_state.py | 20 ++++++++++++++++++++ tests/unit/test_unit_test_parser.py | 2 +- 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index a041838dcea..eb124cf5dde 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -1091,14 +1091,21 @@ def tags(self) -> List[str]: tags = self.config.tags return [tags] if isinstance(tags, str) else tags - def build_unit_test_checksum(self): + def build_unit_test_checksum(self, project_root: str, fixture_paths: List[str]): # everything except 'description' - data = f"{self.model}-{self.given}-{self.expect}-{self.overrides}".encode("utf-8") - self.checksum = hashlib.new("sha256", data).hexdigest() + data = f"{self.model}-{self.given}-{self.expect}-{self.overrides}" + + # include underlying fixture data + for input in self.given: + if input.fixture: + data += f"-{input.get_rows(project_root, fixture_paths)}" + + self.checksum = hashlib.new("sha256", data.encode("utf-8")).hexdigest() def same_contents(self, other: Optional["UnitTestDefinition"]) -> bool: if other is None: return False + return self.checksum == other.checksum diff --git a/core/dbt/parser/unit_tests.py b/core/dbt/parser/unit_tests.py index e6b4e2cfe88..b625a38a4de 100644 --- a/core/dbt/parser/unit_tests.py +++ b/core/dbt/parser/unit_tests.py @@ -236,7 +236,9 @@ def parse(self) -> ParseResult: config=unit_test_config, ) # for calculating state:modified - unit_test_definition.build_unit_test_checksum() + unit_test_definition.build_unit_test_checksum( + self.schema_parser.project.project_root, self.schema_parser.project.fixture_paths + ) self.manifest.add_unit_test(self.yaml.file, unit_test_definition) return ParseResult() diff --git a/tests/functional/unit_testing/fixtures.py b/tests/functional/unit_testing/fixtures.py index cd4d85d699e..b4a147b63c4 100644 --- a/tests/functional/unit_testing/fixtures.py +++ b/tests/functional/unit_testing/fixtures.py @@ -122,7 +122,7 @@ rows: - {c: 2} - - name: test_my_model_empty + - name: test_depends_on_fixture model: my_model given: - input: ref('my_model_a') @@ -155,7 +155,7 @@ rows: - {macro_call: override, var_call: var_override, env_var_call: env_var_override, invocation_id: 123} - - name: test_my_model_string_concat + - name: test_has_string_c_ab model: my_model given: - input: ref('my_model_a') diff --git a/tests/functional/unit_testing/test_state.py b/tests/functional/unit_testing/test_state.py index fe6528ae128..71a3992a407 100644 --- a/tests/functional/unit_testing/test_state.py +++ b/tests/functional/unit_testing/test_state.py @@ -14,6 +14,7 @@ my_model_b_sql, test_my_model_simple_fixture_yml, test_my_model_fixture_csv, + test_my_model_b_fixture_csv as test_my_model_fixture_csv_modified, ) @@ -61,6 +62,24 @@ def test_state_modified(self, project): results = run_dbt(["unit-test", "--select", "state:modified", "--state", "state"]) assert len(results) == 0 + # change underlying fixture file + write_file( + test_my_model_fixture_csv_modified, + project.project_root, + "tests", + "fixtures", + "test_my_model_fixture.csv", + ) + # TODO: remove --no-partial-parse as part of https://github.com/dbt-labs/dbt-core/issues/9067 + results = run_dbt( + ["--no-partial-parse", "unit-test", "--select", "state:modified", "--state", "state"], + expect_pass=True, + ) + assert len(results) == 1 + assert results[0].node.name.endswith("test_depends_on_fixture") + # reset changes + self.copy_state(project.project_root) + # change unit test definition of a single unit test with_changes = test_my_model_simple_fixture_yml.replace("{string_c: ab}", "{string_c: bc}") write_config_file(with_changes, project.project_root, "models", "test_my_model.yml") @@ -68,6 +87,7 @@ def test_state_modified(self, project): ["unit-test", "--select", "state:modified", "--state", "state"], expect_pass=False ) assert len(results) == 1 + assert results[0].node.name.endswith("test_has_string_c_ab") # change underlying model logic write_config_file( diff --git a/tests/unit/test_unit_test_parser.py b/tests/unit/test_unit_test_parser.py index 32211885486..d87002e85da 100644 --- a/tests/unit/test_unit_test_parser.py +++ b/tests/unit/test_unit_test_parser.py @@ -132,7 +132,7 @@ def test_basic(self): fqn=["snowplow", "my_model", "test_my_model"], config=UnitTestConfig(), ) - expected.build_unit_test_checksum() + expected.build_unit_test_checksum("anything", "anything") assertEqualNodes(unit_test, expected) def test_unit_test_config(self):