From e7e97fce86b10a242c0b099fc7941bb7b1fa6061 Mon Sep 17 00:00:00 2001 From: Tim Sturge Date: Mon, 7 Oct 2024 12:01:51 -0700 Subject: [PATCH 1/6] Support config: disabled for unit tests. (#9109) Also, when a model is disabled, disable the corresponding unit tests automatically. (#10540) --- .../resources/v1/unit_test_definition.py | 1 + core/dbt/contracts/graph/manifest.py | 2 ++ core/dbt/graph/selector.py | 3 +- core/dbt/parser/unit_tests.py | 17 ++++++++++- tests/unit/parser/test_unit_tests.py | 29 +++++++++++++++++++ 5 files changed, 50 insertions(+), 2 deletions(-) diff --git a/core/dbt/artifacts/resources/v1/unit_test_definition.py b/core/dbt/artifacts/resources/v1/unit_test_definition.py index 5c18538a733..7e4114953d7 100644 --- a/core/dbt/artifacts/resources/v1/unit_test_definition.py +++ b/core/dbt/artifacts/resources/v1/unit_test_definition.py @@ -20,6 +20,7 @@ class UnitTestConfig(BaseConfig): default_factory=dict, metadata=MergeBehavior.Update.meta(), ) + enabled: bool = True class UnitTestFormat(StrEnum): diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index d387616a8ea..77a5708326e 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -1655,6 +1655,8 @@ def add_disabled(self, source_file: AnySourceFile, node: GraphMemberNode, test_f source_file.semantic_models.append(node.unique_id) if isinstance(node, Exposure): source_file.exposures.append(node.unique_id) + if isinstance(node, UnitTestDefinition): + source_file.unit_tests.append(node.unique_id) elif isinstance(source_file, FixtureSourceFile): pass else: diff --git a/core/dbt/graph/selector.py b/core/dbt/graph/selector.py index cc0b4ebe9fc..bf586851fb8 100644 --- a/core/dbt/graph/selector.py +++ b/core/dbt/graph/selector.py @@ -173,7 +173,8 @@ def _is_graph_member(self, unique_id: UniqueId) -> bool: semantic_model = self.manifest.semantic_models[unique_id] return semantic_model.config.enabled elif unique_id in self.manifest.unit_tests: - return True + unit_test = self.manifest.unit_tests[unique_id] + return unit_test.config.enabled elif unique_id in self.manifest.saved_queries: saved_query = self.manifest.saved_queries[unique_id] return saved_query.config.enabled diff --git a/core/dbt/parser/unit_tests.py b/core/dbt/parser/unit_tests.py index 0e60c90274b..c227319f7e9 100644 --- a/core/dbt/parser/unit_tests.py +++ b/core/dbt/parser/unit_tests.py @@ -52,6 +52,8 @@ def load(self) -> Manifest: for unique_id in self.selected: if unique_id in self.manifest.unit_tests: unit_test_case: UnitTestDefinition = self.manifest.unit_tests[unique_id] + if not unit_test_case.config.enabled: + continue self.parse_unit_test_case(unit_test_case) return self.unit_test_manifest @@ -295,7 +297,10 @@ def parse(self) -> ParseResult: # for calculating state:modified unit_test_definition.build_unit_test_checksum() assert isinstance(self.yaml.file, SchemaSourceFile) - self.manifest.add_unit_test(self.yaml.file, unit_test_definition) + if unit_test_config.enabled: + self.manifest.add_unit_test(self.yaml.file, unit_test_definition) + else: + self.manifest.add_disabled(self.yaml.file, unit_test_definition) return ParseResult() @@ -505,6 +510,16 @@ def process_models_for_unit_test( # The UnitTestDefinition should only have one "depends_on" at this point, # the one that's found by the "model" field. target_model_id = unit_test_def.depends_on.nodes[0] + if target_model_id not in manifest.nodes: + if target_model_id in manifest.disabled: + # The model is disabled, so we don't need to do anything (#10540) + return + else: + # If we've reached here and the model is not disabled, throw an error + raise ParsingError( + f"Unit test '{unit_test_def.name}' references a model that does not exist: {target_model_id}" + ) + target_model = manifest.nodes[target_model_id] assert isinstance(target_model, ModelNode) diff --git a/tests/unit/parser/test_unit_tests.py b/tests/unit/parser/test_unit_tests.py index 4e6554aeca7..e1c4d8327bd 100644 --- a/tests/unit/parser/test_unit_tests.py +++ b/tests/unit/parser/test_unit_tests.py @@ -135,6 +135,19 @@ - {"id": , "col1": "e"} """ +UNIT_TEST_DISABLED = """ +unit_tests: + - name: test_my_model_disabled + model: my_model + description: "this unit test is disabled" + config: + enabled: false + given: [] + expect: + rows: + - {a: 1} +""" + class UnitTestParserTest(SchemaParserTest): def setUp(self): @@ -198,6 +211,22 @@ def test_unit_test_config(self): self.assertEqual(sorted(unit_test.config.tags), sorted(["schema_tag", "project_tag"])) self.assertEqual(unit_test.config.meta, {"meta_key": "meta_value", "meta_jinja_key": "2"}) + def test_unit_test_disabled(self): + block = self.yaml_block_for(UNIT_TEST_DISABLED, "test_my_model.yml") + self.root_project_config.unit_tests = { + "snowplow": {"my_model": {"+tags": ["project_tag"]}} + } + + UnitTestParser(self.parser, block).parse() + + self.assert_has_manifest_lengths(self.parser.manifest, nodes=1, unit_tests=0, disabled=1) + unit_test_disabled_list = self.parser.manifest.disabled[ + "unit_test.snowplow.my_model.test_my_model_disabled" + ] + self.assertEqual(len(unit_test_disabled_list), 1) + unit_test_disabled = unit_test_disabled_list[0] + self.assertEqual(unit_test_disabled.config.enabled, False) + def test_unit_test_versioned_model(self): block = self.yaml_block_for(UNIT_TEST_VERSIONED_MODEL_SOURCE, "test_my_model.yml") my_model_versioned_node = MockNode( From 8afe6844b4e413d97b81ba020da323d78846f934 Mon Sep 17 00:00:00 2001 From: Tim Sturge Date: Mon, 7 Oct 2024 12:16:04 -0700 Subject: [PATCH 2/6] add changelog entry --- .changes/unreleased/Fixes-20241007-121402.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20241007-121402.yaml diff --git a/.changes/unreleased/Fixes-20241007-121402.yaml b/.changes/unreleased/Fixes-20241007-121402.yaml new file mode 100644 index 00000000000..5c858f3a9e9 --- /dev/null +++ b/.changes/unreleased/Fixes-20241007-121402.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Support disabling unit tests via config. +time: 2024-10-07T12:14:02.252744-07:00 +custom: + Author: tsturge + Issue: 9109 10540 From b500141d6c5b94336ea18edb416e608798dff865 Mon Sep 17 00:00:00 2001 From: Tim Sturge Date: Tue, 29 Oct 2024 11:11:44 -0700 Subject: [PATCH 3/6] add enabled to config block --- tests/functional/unit_testing/test_ut_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/unit_testing/test_ut_list.py b/tests/functional/unit_testing/test_ut_list.py index 0b4f263909b..a348203ec64 100644 --- a/tests/functional/unit_testing/test_ut_list.py +++ b/tests/functional/unit_testing/test_ut_list.py @@ -59,7 +59,7 @@ def test_unit_test_list(self, project): "original_file_path": os.path.join("models", "test_my_model.yml"), "unique_id": "unit_test.test.my_model.test_my_model", "depends_on": {"macros": [], "nodes": ["model.test.my_model"]}, - "config": {"tags": [], "meta": {}}, + "config": {"tags": [], "meta": {}, "enabled": True}, } for result in results: json_result = json.loads(result) From 7e7d36f299c95adfc8ee155d75f0121382ac36dc Mon Sep 17 00:00:00 2001 From: Tim Sturge Date: Tue, 29 Oct 2024 11:23:56 -0700 Subject: [PATCH 4/6] add a disabled test and make sure it does not list with the other unit tests --- tests/functional/unit_testing/fixtures.py | 18 ++++++++++++++++++ tests/functional/unit_testing/test_ut_list.py | 2 ++ 2 files changed, 20 insertions(+) diff --git a/tests/functional/unit_testing/fixtures.py b/tests/functional/unit_testing/fixtures.py index 1da67f7e762..7445f8b3493 100644 --- a/tests/functional/unit_testing/fixtures.py +++ b/tests/functional/unit_testing/fixtures.py @@ -145,6 +145,24 @@ - {c: 3} """ +test_disabled_my_model_yml = """ +unit_tests: + - name: test_disabled_my_model + model: my_model + config: + enabled: false + 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: 3} +""" test_my_model_simple_fixture_yml = """ unit_tests: diff --git a/tests/functional/unit_testing/test_ut_list.py b/tests/functional/unit_testing/test_ut_list.py index a348203ec64..ccb9f887bae 100644 --- a/tests/functional/unit_testing/test_ut_list.py +++ b/tests/functional/unit_testing/test_ut_list.py @@ -7,6 +7,7 @@ my_model_a_sql, my_model_b_sql, my_model_vars_sql, + test_disabled_my_model_yml, test_my_model_yml, ) @@ -21,6 +22,7 @@ def models(self): "my_model_a.sql": my_model_a_sql, "my_model_b.sql": my_model_b_sql, "test_my_model.yml": test_my_model_yml + datetime_test, + "test_disabled_my_model.yml": test_disabled_my_model_yml } @pytest.fixture(scope="class") From b5d8424ea19b7bf372438fbe0cce228142b94c9c Mon Sep 17 00:00:00 2001 From: Tim Sturge Date: Tue, 29 Oct 2024 11:29:05 -0700 Subject: [PATCH 5/6] formatting --- tests/functional/unit_testing/test_ut_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/unit_testing/test_ut_list.py b/tests/functional/unit_testing/test_ut_list.py index ccb9f887bae..e137d7aa252 100644 --- a/tests/functional/unit_testing/test_ut_list.py +++ b/tests/functional/unit_testing/test_ut_list.py @@ -22,7 +22,7 @@ def models(self): "my_model_a.sql": my_model_a_sql, "my_model_b.sql": my_model_b_sql, "test_my_model.yml": test_my_model_yml + datetime_test, - "test_disabled_my_model.yml": test_disabled_my_model_yml + "test_disabled_my_model.yml": test_disabled_my_model_yml, } @pytest.fixture(scope="class") From 56f2300dc44d835d52c2ed6820efe866dc4473b8 Mon Sep 17 00:00:00 2001 From: Tim Sturge Date: Wed, 30 Oct 2024 12:10:25 -0700 Subject: [PATCH 6/6] add enabled flag to manifest schema --- schemas/dbt/manifest/v12.json | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/schemas/dbt/manifest/v12.json b/schemas/dbt/manifest/v12.json index 0a8322611e4..4d66f65f234 100644 --- a/schemas/dbt/manifest/v12.json +++ b/schemas/dbt/manifest/v12.json @@ -20935,6 +20935,10 @@ "propertyNames": { "type": "string" } + }, + "enabled": { + "type": "boolean", + "default": true } }, "additionalProperties": true @@ -22500,6 +22504,10 @@ "propertyNames": { "type": "string" } + }, + "enabled": { + "type": "boolean", + "default": true } }, "additionalProperties": true