From eb0c3c709b4d914f13b7969e490aaebc79da55e1 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Fri, 12 Jan 2024 15:22:03 -0500 Subject: [PATCH 1/6] support longer unit test names --- core/dbt/context/providers.py | 1 - core/dbt/parser/unit_tests.py | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/core/dbt/context/providers.py b/core/dbt/context/providers.py index cd51cd8d1ec..aafbbfa5f67 100644 --- a/core/dbt/context/providers.py +++ b/core/dbt/context/providers.py @@ -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) diff --git a/core/dbt/parser/unit_tests.py b/core/dbt/parser/unit_tests.py index c2c744b8dd5..acfaf32b852 100644 --- a/core/dbt/parser/unit_tests.py +++ b/core/dbt/parser/unit_tests.py @@ -64,7 +64,7 @@ def parse_unit_test_case(self, test_case: UnitTestDefinition): # 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, @@ -134,7 +134,7 @@ def parse_unit_test_case(self, test_case: UnitTestDefinition): 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}", @@ -145,7 +145,7 @@ def parse_unit_test_case(self, test_case: UnitTestDefinition): # 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 input_node = UnitTestSourceDefinition( **common_fields, unique_id=f"model.{test_case.package_name}.{input_name}", From 3ed4bc880e588911a295038aa50510ffad489a84 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 15 Jan 2024 16:52:39 -0500 Subject: [PATCH 2/6] add tests --- core/dbt/task/test.py | 10 +- tests/functional/unit_testing/fixtures.py | 34 +++++++ .../functional/unit_testing/test_ut_names.py | 97 +++++++++++++++++++ 3 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 tests/functional/unit_testing/test_ut_names.py diff --git a/core/dbt/task/test.py b/core/dbt/task/test.py index c0295a7e705..bc2baf84cfa 100644 --- a/core/dbt/task/test.py +++ b/core/dbt/task/test.py @@ -83,15 +83,21 @@ class UnitTestResultData(dbtClassMixin): 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, diff --git a/tests/functional/unit_testing/fixtures.py b/tests/functional/unit_testing/fixtures.py index b4a147b63c4..07874c0e384 100644 --- a/tests/functional/unit_testing/fixtures.py +++ b/tests/functional/unit_testing/fixtures.py @@ -598,3 +598,37 @@ 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"} +""" diff --git a/tests/functional/unit_testing/test_ut_names.py b/tests/functional/unit_testing/test_ut_names.py new file mode 100644 index 00000000000..bf3577e0f78 --- /dev/null +++ b/tests/functional/unit_testing/test_ut_names.py @@ -0,0 +1,97 @@ +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"]) + + +test_model_a_long_test_name_yml = """ +unit_tests: + - name: my_very_reasonable_but_kind_of_long_test_name_for_model_a + model: my_model_a + given: [] + expect: + rows: + - {a: 1, id: 1, not_testing: 2, string_a: "a", date_a: "2020-01-02"} +""" + + +class TestUnitTestLongTestNames: + @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, + } + + def test_long_unit_test_name(self, project): + results = run_dbt(["run"]) + assert len(results) == 1 + + results = run_dbt(["test"], expect_pass=True) + assert len(results) == 1 + assert len(results[0].node.name) >= 50 From cb629f5eeb7a861be08cf1ff77d52e4e6a8f2b03 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 15 Jan 2024 16:53:16 -0500 Subject: [PATCH 3/6] changelog entry --- .changes/unreleased/Fixes-20240115-165310.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20240115-165310.yaml diff --git a/.changes/unreleased/Fixes-20240115-165310.yaml b/.changes/unreleased/Fixes-20240115-165310.yaml new file mode 100644 index 00000000000..d05064bba61 --- /dev/null +++ b/.changes/unreleased/Fixes-20240115-165310.yaml @@ -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" From 703880e36ee7980d03e7b2a2361666f7b9a50276 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Tue, 16 Jan 2024 16:21:47 -0500 Subject: [PATCH 4/6] reorganize tests --- core/dbt/task/test.py | 16 +++-- .../adapter/unit_testing/test_unit_testing.py | 69 +++++++++++++++++++ .../functional/unit_testing/test_ut_names.py | 28 -------- 3 files changed, 79 insertions(+), 34 deletions(-) create mode 100644 tests/adapter/dbt/tests/adapter/unit_testing/test_unit_testing.py diff --git a/core/dbt/task/test.py b/core/dbt/task/test.py index bc2baf84cfa..9aba5e291e5 100644 --- a/core/dbt/task/test.py +++ b/core/dbt/task/test.py @@ -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, ) @@ -213,7 +211,13 @@ def execute_unit_test( # generate materialization macro macro_func = MacroGenerator(materialization_macro, context) # execute materialization macro - macro_func() + try: + macro_func() + except DbtBaseException as e: + raise DbtRuntimeError( + 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") diff --git a/tests/adapter/dbt/tests/adapter/unit_testing/test_unit_testing.py b/tests/adapter/dbt/tests/adapter/unit_testing/test_unit_testing.py new file mode 100644 index 00000000000..c571b74480a --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/unit_testing/test_unit_testing.py @@ -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 diff --git a/tests/functional/unit_testing/test_ut_names.py b/tests/functional/unit_testing/test_ut_names.py index bf3577e0f78..27c0a56201c 100644 --- a/tests/functional/unit_testing/test_ut_names.py +++ b/tests/functional/unit_testing/test_ut_names.py @@ -67,31 +67,3 @@ def models(self): def test_duplicate_test_names_within_model(self, project): with pytest.raises(DuplicateResourceNameError): run_dbt(["run"]) - - -test_model_a_long_test_name_yml = """ -unit_tests: - - name: my_very_reasonable_but_kind_of_long_test_name_for_model_a - model: my_model_a - given: [] - expect: - rows: - - {a: 1, id: 1, not_testing: 2, string_a: "a", date_a: "2020-01-02"} -""" - - -class TestUnitTestLongTestNames: - @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, - } - - def test_long_unit_test_name(self, project): - results = run_dbt(["run"]) - assert len(results) == 1 - - results = run_dbt(["test"], expect_pass=True) - assert len(results) == 1 - assert len(results[0].node.name) >= 50 From 618ff36ee6c3546a2a1e2d3c76dac600b0c29164 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 17 Jan 2024 09:29:38 -0500 Subject: [PATCH 5/6] TestUnitTestBadTestName --- core/dbt/task/test.py | 2 +- tests/functional/unit_testing/fixtures.py | 11 +++++++++++ tests/functional/unit_testing/test_ut_names.py | 16 +++++++++++++++- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/core/dbt/task/test.py b/core/dbt/task/test.py index 9aba5e291e5..83f9aea351e 100644 --- a/core/dbt/task/test.py +++ b/core/dbt/task/test.py @@ -28,7 +28,7 @@ LogStartLine, ) from dbt.exceptions import DbtInternalError, BooleanError -from dbt.common.exceptions import DbtBaseException, DbtRuntimeError +from dbt_common.exceptions import DbtBaseException, DbtRuntimeError from dbt.adapters.exceptions import MissingMaterializationError from dbt.graph import ( ResourceTypeSelector, diff --git a/tests/functional/unit_testing/fixtures.py b/tests/functional/unit_testing/fixtures.py index 07874c0e384..4a326d8ced9 100644 --- a/tests/functional/unit_testing/fixtures.py +++ b/tests/functional/unit_testing/fixtures.py @@ -632,3 +632,14 @@ rows: - {a: 1, id: 1, not_testing: 2, string_a: "a", date_a: "2020-01-02"} """ + + +test_model_a_with_bad_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"} +""" diff --git a/tests/functional/unit_testing/test_ut_names.py b/tests/functional/unit_testing/test_ut_names.py index 27c0a56201c..316cef9fdf3 100644 --- a/tests/functional/unit_testing/test_ut_names.py +++ b/tests/functional/unit_testing/test_ut_names.py @@ -1,13 +1,14 @@ import pytest from dbt.tests.util import run_dbt, run_dbt_and_capture -from dbt.exceptions import DuplicateResourceNameError +from dbt.exceptions import DuplicateResourceNameError, DbtRuntimeError from fixtures import ( my_model_a_sql, my_model_b_sql, test_model_a_b_yml, test_model_a_with_duplicate_test_name_yml, + test_model_a_with_bad_test_name_yml, ) @@ -67,3 +68,16 @@ def models(self): def test_duplicate_test_names_within_model(self, project): with pytest.raises(DuplicateResourceNameError): run_dbt(["run"]) + + +class TestUnitTestBadTestName: + @pytest.fixture(scope="class") + def models(self): + return { + "my_model_a.sql": my_model_a_sql, + "test_model_a.yml": test_model_a_with_bad_test_name_yml, + } + + def test_duplicate_test_names_bad_name(self, project): + with pytest.raises(DbtRuntimeError): + run_dbt(["test"]) From 0ca82701d80e1a80be268afd27836af2dd86ec31 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 17 Jan 2024 12:02:57 -0500 Subject: [PATCH 6/6] improve invalid config test --- tests/functional/unit_testing/fixtures.py | 18 ++++++++++++------ .../unit_testing/test_unit_testing.py | 19 +++++++++++++++++++ .../functional/unit_testing/test_ut_names.py | 16 +--------------- 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/tests/functional/unit_testing/fixtures.py b/tests/functional/unit_testing/fixtures.py index 4a326d8ced9..5c439b92cef 100644 --- a/tests/functional/unit_testing/fixtures.py +++ b/tests/functional/unit_testing/fixtures.py @@ -633,13 +633,19 @@ - {a: 1, id: 1, not_testing: 2, string_a: "a", date_a: "2020-01-02"} """ - -test_model_a_with_bad_test_name_yml = """ +test_my_model_yml_invalid = """ unit_tests: - - name: !!my_test_name - model: my_model_a - given: [] + - 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: - - {a: 1, id: 1, not_testing: 2, string_a: "a", date_a: "2020-01-02"} + - {c: 3} """ diff --git a/tests/functional/unit_testing/test_unit_testing.py b/tests/functional/unit_testing/test_unit_testing.py index 7eea3627a32..2c683d99598 100644 --- a/tests/functional/unit_testing/test_unit_testing.py +++ b/tests/functional/unit_testing/test_unit_testing.py @@ -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, @@ -15,6 +16,7 @@ my_incremental_model_sql, event_sql, test_my_model_incremental_yml, + test_my_model_yml_invalid, ) @@ -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) diff --git a/tests/functional/unit_testing/test_ut_names.py b/tests/functional/unit_testing/test_ut_names.py index 316cef9fdf3..27c0a56201c 100644 --- a/tests/functional/unit_testing/test_ut_names.py +++ b/tests/functional/unit_testing/test_ut_names.py @@ -1,14 +1,13 @@ import pytest from dbt.tests.util import run_dbt, run_dbt_and_capture -from dbt.exceptions import DuplicateResourceNameError, DbtRuntimeError +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, - test_model_a_with_bad_test_name_yml, ) @@ -68,16 +67,3 @@ def models(self): def test_duplicate_test_names_within_model(self, project): with pytest.raises(DuplicateResourceNameError): run_dbt(["run"]) - - -class TestUnitTestBadTestName: - @pytest.fixture(scope="class") - def models(self): - return { - "my_model_a.sql": my_model_a_sql, - "test_model_a.yml": test_model_a_with_bad_test_name_yml, - } - - def test_duplicate_test_names_bad_name(self, project): - with pytest.raises(DbtRuntimeError): - run_dbt(["test"])