Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support disabling unit tests #10831

Merged
merged 6 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20241007-121402.yaml
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions core/dbt/artifacts/resources/v1/unit_test_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class UnitTestConfig(BaseConfig):
default_factory=dict,
metadata=MergeBehavior.Update.meta(),
)
enabled: bool = True


class UnitTestFormat(StrEnum):
Expand Down
2 changes: 2 additions & 0 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion core/dbt/graph/selector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 16 additions & 1 deletion core/dbt/parser/unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
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

Check warning on line 56 in core/dbt/parser/unit_tests.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/unit_tests.py#L56

Added line #L56 was not covered by tests
self.parse_unit_test_case(unit_test_case)
return self.unit_test_manifest

Expand Down Expand Up @@ -295,7 +297,10 @@
# 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()

Expand Down Expand Up @@ -505,6 +510,16 @@
# 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:

Check warning on line 514 in core/dbt/parser/unit_tests.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/unit_tests.py#L514

Added line #L514 was not covered by tests
# The model is disabled, so we don't need to do anything (#10540)
return

Check warning on line 516 in core/dbt/parser/unit_tests.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/unit_tests.py#L516

Added line #L516 was not covered by tests
else:
# If we've reached here and the model is not disabled, throw an error
raise ParsingError(

Check warning on line 519 in core/dbt/parser/unit_tests.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/unit_tests.py#L519

Added line #L519 was not covered by tests
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)

Expand Down
8 changes: 8 additions & 0 deletions schemas/dbt/manifest/v12.json
Original file line number Diff line number Diff line change
Expand Up @@ -20935,6 +20935,10 @@
"propertyNames": {
"type": "string"
}
},
"enabled": {
"type": "boolean",
"default": true
}
},
"additionalProperties": true
Expand Down Expand Up @@ -22500,6 +22504,10 @@
"propertyNames": {
"type": "string"
}
},
"enabled": {
"type": "boolean",
"default": true
}
},
"additionalProperties": true
Expand Down
18 changes: 18 additions & 0 deletions tests/functional/unit_testing/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion tests/functional/unit_testing/test_ut_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

Expand All @@ -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")
Expand Down Expand Up @@ -59,7 +61,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)
Expand Down
29 changes: 29 additions & 0 deletions tests/unit/parser/test_unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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(
Expand Down
Loading