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 longer unit test names + improve error handling in unit test construction #9396

Merged
merged 6 commits into from
Jan 18, 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-20240115-165310.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Support reasonably long unit test names
time: 2024-01-15T16:53:10.42761-05:00
custom:
Author: michelleark
Issue: "9015"
1 change: 0 additions & 1 deletion core/dbt/context/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,6 @@ def resolve(
target_package: Optional[str] = None,
target_version: Optional[NodeVersion] = None,
) -> RelationProxy:
target_name = f"{self.model.name}__{target_name}"
return super().resolve(target_name, target_package, target_version)


Expand Down
6 changes: 3 additions & 3 deletions core/dbt/parser/unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
# already been done, we don't have to care about fields that are necessary
# for selection.
# Note: no depends_on, that's added later using input nodes
name = f"{test_case.model}__{test_case.name}"
name = test_case.name
unit_test_node = UnitTestNode(
name=name,
resource_type=NodeType.Unit,
Expand Down Expand Up @@ -134,7 +134,7 @@
NodeType.Seed,
NodeType.Snapshot,
):
input_name = f"{unit_test_node.name}__{original_input_node.name}"
input_name = original_input_node.name
input_node = ModelNode(
**common_fields,
unique_id=f"model.{test_case.package_name}.{input_name}",
Expand All @@ -145,7 +145,7 @@
# We are reusing the database/schema/identifier from the original source,
# but that shouldn't matter since this acts as an ephemeral model which just
# wraps a CTE around the unit test node.
input_name = f"{unit_test_node.name}__{original_input_node.search_name}__{original_input_node.name}"
input_name = original_input_node.name

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

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/unit_tests.py#L148

Added line #L148 was not covered by tests
input_node = UnitTestSourceDefinition(
**common_fields,
unique_id=f"model.{test_case.package_name}.{input_name}",
Expand Down
26 changes: 18 additions & 8 deletions core/dbt/task/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,9 @@
LogTestResult,
LogStartLine,
)
from dbt.exceptions import (
DbtInternalError,
BooleanError,
)
from ..adapters.exceptions import MissingMaterializationError
from dbt.exceptions import DbtInternalError, BooleanError
from dbt_common.exceptions import DbtBaseException, DbtRuntimeError
from dbt.adapters.exceptions import MissingMaterializationError
from dbt.graph import (
ResourceTypeSelector,
)
Expand Down Expand Up @@ -83,15 +81,21 @@
class TestRunner(CompileRunner):
_ANSI_ESCAPE = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])")

def describe_node_name(self):
if self.node.resource_type == NodeType.Unit:
return f"{self.node.model}::{self.node.name}"
else:
return self.node.name

def describe_node(self):
return f"{self.node.resource_type} {self.node.name}"
return f"{self.node.resource_type} {self.describe_node_name()}"

def print_result_line(self, result):
model = result.node

fire_event(
LogTestResult(
name=model.name,
name=self.describe_node_name(),
status=str(result.status),
index=self.node_index,
num_models=self.num_nodes,
Expand Down Expand Up @@ -207,7 +211,13 @@
# generate materialization macro
macro_func = MacroGenerator(materialization_macro, context)
# execute materialization macro
macro_func()
try:
macro_func()
except DbtBaseException as e:
raise DbtRuntimeError(

Check warning on line 217 in core/dbt/task/test.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/task/test.py#L216-L217

Added lines #L216 - L217 were not covered by tests
f"During unit test execution of {self.describe_node_name()}, dbt could not build the 'actual' result for comparison against 'expected' given the unit test definition:\n {e}"
)

# load results from context
# could eventually be returned directly by materialization
result = context["load_result"]("main")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import pytest

from dbt.adapters.postgres.relation_configs import MAX_CHARACTERS_IN_IDENTIFIER
from dbt.tests.util import run_dbt, write_file

my_model_a_sql = """
SELECT
1 as a,
1 as id,
2 as not_testing,
'a' as string_a,
DATE '2020-01-02' as date_a
"""

test_model_a_long_test_name_yml = """
unit_tests:
- name: {test_name}
model: my_model_a
given: []
expect:
rows:
- {{a: 1, id: 1, not_testing: 2, string_a: "a", date_a: "2020-01-02"}}
"""


class BaseUnitTestLongTestName:
@pytest.fixture(scope="class")
def models(self):
return {
"my_model_a.sql": my_model_a_sql,
"test_model_a.yml": test_model_a_long_test_name_yml,
}

@pytest.fixture
def max_unit_test_name_length(self) -> int:
return -1

def test_long_unit_test_name(self, project, max_unit_test_name_length):
# max test name == passing unit test
write_file(
test_model_a_long_test_name_yml.format(test_name="a" * max_unit_test_name_length),
"models",
"test_model_a.yml",
)
results = run_dbt(["run"])
assert len(results) == 1

results = run_dbt(["test"], expect_pass=True)
assert len(results) == 1

# max test name == failing command
write_file(
test_model_a_long_test_name_yml.format(
test_name="a" * (max_unit_test_name_length + 1)
),
"models",
"test_model_a.yml",
)

results = run_dbt(["run"])
assert len(results) == 1

run_dbt(["test"], expect_pass=False)


class TestPostgresUnitTestLongTestNames(BaseUnitTestLongTestName):
@pytest.fixture
def max_unit_test_name_length(self) -> int:
return MAX_CHARACTERS_IN_IDENTIFIER
51 changes: 51 additions & 0 deletions tests/functional/unit_testing/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -598,3 +598,54 @@
format: csv
fixture: test_my_model_basic_fixture
"""

test_model_a_b_yml = """
unit_tests:
- name: my_test_name
model: my_model_a
given: []
expect:
rows:
- {a: 1, id: 1, not_testing: 2, string_a: "a", date_a: "2020-01-02"}

- name: my_test_name
model: my_model_b
given: []
expect:
rows:
- {b: 2, id: 1, c: 2, string_b: "b"}
"""

test_model_a_with_duplicate_test_name_yml = """
unit_tests:
- name: my_test_name
model: my_model_a
given: []
expect:
rows:
- {a: 1, id: 1, not_testing: 2, string_a: "a", date_a: "2020-01-02"}

- name: my_test_name
model: my_model_a
given: []
expect:
rows:
- {a: 1, id: 1, not_testing: 2, string_a: "a", date_a: "2020-01-02"}
"""

test_my_model_yml_invalid = """
unit_tests:
- name: test_my_model
model: my_model
given:
- input: ref('my_model_a')
rows:
- {id: 1, a: "a"}
- input: ref('my_model_b')
rows:
- {id: 1, b: 2}
- {id: 2, b: 2}
expect:
rows:
- {c: 3}
"""
19 changes: 19 additions & 0 deletions tests/functional/unit_testing/test_unit_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from dbt.contracts.results import NodeStatus
from dbt.exceptions import DuplicateResourceNameError, ParsingError
from fixtures import (
my_model_sql,
my_model_vars_sql,
my_model_a_sql,
my_model_b_sql,
Expand All @@ -15,6 +16,7 @@
my_incremental_model_sql,
event_sql,
test_my_model_incremental_yml,
test_my_model_yml_invalid,
)


Expand Down Expand Up @@ -237,3 +239,20 @@ def test_nonexistent_seed(self, project):
ParsingError, match="Unable to find seed 'test.my_second_favorite_seed' for unit tests"
):
run_dbt(["test", "--select", "my_new_model"], expect_pass=False)


class TestUnitTestInvalidInputConfiguration:
@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_invalid,
}

def test_invalid_input_configuration(self, project):
results = run_dbt(["run"])
assert len(results) == 3

run_dbt(["test"], expect_pass=False)
69 changes: 69 additions & 0 deletions tests/functional/unit_testing/test_ut_names.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import pytest

from dbt.tests.util import run_dbt, run_dbt_and_capture
from dbt.exceptions import DuplicateResourceNameError

from fixtures import (
my_model_a_sql,
my_model_b_sql,
test_model_a_b_yml,
test_model_a_with_duplicate_test_name_yml,
)


class TestUnitTestDuplicateTestNamesAcrossModels:
@pytest.fixture(scope="class")
def models(self):
return {
"my_model_a.sql": my_model_a_sql,
"my_model_b.sql": my_model_b_sql,
"test_model_a_b.yml": test_model_a_b_yml,
}

def test_duplicate_test_names_across_models(self, project):
results = run_dbt(["run"])
assert len(results) == 2

# Select duplicate tests
results, log_output = run_dbt_and_capture(["test"], expect_pass=True)
assert len(results) == 2
assert ["my_model_a", "my_model_b"] == sorted([result.node.model for result in results])
assert "my_model_a::my_test_name" in log_output
assert "my_model_b::my_test_name" in log_output

# Test select duplicates by by test name
results = run_dbt(["test", "--select", "test_name:my_test_name"])
assert len(results) == 2
assert ["my_model_a", "my_model_b"] == sorted([result.node.model for result in results])
assert "my_model_a::my_test_name" in log_output
assert "my_model_b::my_test_name" in log_output

results = run_dbt(["test", "--select", "my_model_a,test_name:my_test_name"])
assert len(results) == 1
assert results[0].node.model == "my_model_a"

results = run_dbt(["test", "--select", "my_model_b,test_name:my_test_name"])
assert len(results) == 1
assert results[0].node.model == "my_model_b"

# Test select by model name
results = run_dbt(["test", "--select", "my_model_a"])
assert len(results) == 1
assert results[0].node.model == "my_model_a"

results = run_dbt(["test", "--select", "my_model_b"])
assert len(results) == 1
assert results[0].node.model == "my_model_b"


class TestUnitTestDuplicateTestNamesWithinModel:
@pytest.fixture(scope="class")
def models(self):
return {
"my_model_a.sql": my_model_a_sql,
"test_model_a.yml": test_model_a_with_duplicate_test_name_yml,
}

def test_duplicate_test_names_within_model(self, project):
with pytest.raises(DuplicateResourceNameError):
run_dbt(["run"])