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

Call adapter.pre_model_hook + adapter.post_model_hook within tests #10258

Merged
merged 12 commits into from
Jun 14, 2024
6 changes: 6 additions & 0 deletions .changes/unreleased/Features-20240606-112334.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: add pre_model and post_model hook calls to data and unit tests to be able to provide extra config options
time: 2024-06-06T11:23:34.758675-05:00
custom:
Author: McKnight-42
Issue: "10198"
16 changes: 13 additions & 3 deletions core/dbt/task/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@
def execute_data_test(self, data_test: TestNode, manifest: Manifest) -> TestResultData:
context = generate_runtime_model_context(data_test, self.config, manifest)

hook_ctx = self.adapter.pre_model_hook(context)

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

View check run for this annotation

Codecov / codecov/patch

core/dbt/task/test.py#L129

Added line #L129 was not covered by tests
McKnight-42 marked this conversation as resolved.
Show resolved Hide resolved

materialization_macro = manifest.find_materialization_macro_by_name(
self.config.project_name, data_test.get_materialization(), self.adapter.type()
)
Expand All @@ -142,8 +144,12 @@

# generate materialization macro
macro_func = MacroGenerator(materialization_macro, context)
# execute materialization macro
macro_func()
try:

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

View check run for this annotation

Codecov / codecov/patch

core/dbt/task/test.py#L147

Added line #L147 was not covered by tests
# execute materialization macro
macro_func()

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

View check run for this annotation

Codecov / codecov/patch

core/dbt/task/test.py#L149

Added line #L149 was not covered by tests
finally:
self.adapter.post_model_hook(context, hook_ctx)

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

View check run for this annotation

Codecov / codecov/patch

core/dbt/task/test.py#L151

Added line #L151 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To confirm, this is not a behaviour change because:

cc @jtcohen6

Copy link
Contributor Author

@McKnight-42 McKnight-42 Jun 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think i can help supply a answer here. with this pr and

as tested in dbt-labs/dbt-snowflake#1070

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah! gotcha -- so configs were not possible to provide for generic tests in the past, which means there should not be any currently set on them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from my understanding yes, we could use a old pattern to set a set number of accepted configs linked in the generic_config issue, making this update to the newer syntax while maintaining the old will allow us to do this now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @MichelleArk @McKnight-42 for thinking of this!

As I see it, the only way in which this could be a "breaking" change is if someone had configured a singular test, or all tests (from dbt_project.yml), with a snowflake_warehouse that they didn't actually want to be used.

Something like:

# dbt_project.yml
tests:
  +snowflake_warehouse: does_not_exist
-- tests/my_singular_test.sql
{{ config(snowflake_warehouse = 'wrong_warehouse') }}

select ...

I do not see that as a breaking change; I see it as this feature in dbt (specifically dbt-snowflake) finally working as intended. As such, I do not believe we need to put it behind a legacy behavior migration flag.


# load results from context
# could eventually be returned directly by materialization
result = context["load_result"]("main")
Expand Down Expand Up @@ -198,6 +204,8 @@
# materialization, not compile the node.compiled_code
context = generate_runtime_model_context(unit_test_node, self.config, unit_test_manifest)

hook_ctx = self.adapter.pre_model_hook(context)

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

View check run for this annotation

Codecov / codecov/patch

core/dbt/task/test.py#L207

Added line #L207 was not covered by tests

materialization_macro = unit_test_manifest.find_materialization_macro_by_name(
self.config.project_name, unit_test_node.get_materialization(), self.adapter.type()
)
Expand All @@ -215,14 +223,16 @@

# generate materialization macro
macro_func = MacroGenerator(materialization_macro, context)
# execute materialization macro
try:
# execute materialization macro
macro_func()
except DbtBaseException as e:
raise DbtRuntimeError(
f"An error occurred during execution of unit test '{unit_test_def.name}'. "
f"There may be an error in the unit test definition: check the data types.\n {e}"
)
finally:
self.adapter.post_model_hook(context, hook_ctx)

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

View check run for this annotation

Codecov / codecov/patch

core/dbt/task/test.py#L235

Added line #L235 was not covered by tests

# load results from context
# could eventually be returned directly by materialization
Expand Down
111 changes: 111 additions & 0 deletions tests/functional/data_tests/test_hooks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
from unittest import mock

import pytest

from dbt.tests.util import run_dbt, run_dbt_and_capture
from dbt_common.exceptions import CompilationError

orders_csv = """order_id,order_date,customer_id
1,2024-06-01,1001
2,2024-06-02,1002
3,2024-06-03,1003
4,2024-06-04,1004
"""


orders_model_sql = """
with source as (
select
order_id,
order_date,
customer_id
from {{ ref('seed_orders') }}
),
final as (
select
order_id,
order_date,
customer_id
from source
)
select * from final
"""


orders_test_sql = """
select *
from {{ ref('orders') }}
where order_id is null
"""


class BaseSingularTestHooks:
@pytest.fixture(scope="class")
def seeds(self):
return {"seed_orders.csv": orders_csv}

@pytest.fixture(scope="class")
def models(self):
return {"orders.sql": orders_model_sql}

@pytest.fixture(scope="class")
def tests(self):
return {"orders_test.sql": orders_test_sql}


class TestSingularTestPreHook(BaseSingularTestHooks):
def test_data_test_runs_adapter_pre_hook_pass(self, project):
results = run_dbt(["seed"])
assert len(results) == 1

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

mock_pre_model_hook = mock.Mock()
with mock.patch.object(type(project.adapter), "pre_model_hook", mock_pre_model_hook):
results = run_dbt(["test"], expect_pass=True)
assert len(results) == 1
mock_pre_model_hook.assert_called_once()

def test_data_test_runs_adapter_pre_hook_fails(self, project):
results = run_dbt(["seed"])
assert len(results) == 1

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

mock_pre_model_hook = mock.Mock()
mock_pre_model_hook.side_effect = CompilationError("exception from adapter.pre_model_hook")
with mock.patch.object(type(project.adapter), "pre_model_hook", mock_pre_model_hook):
(_, log_output) = run_dbt_and_capture(["test"], expect_pass=False)
assert "exception from adapter.pre_model_hook" in log_output


class TestSingularTestPostHook(BaseSingularTestHooks):
def test_data_test_runs_adapter_post_hook_pass(self, project):
results = run_dbt(["seed"])
assert len(results) == 1

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

mock_post_model_hook = mock.Mock()
with mock.patch.object(type(project.adapter), "post_model_hook", mock_post_model_hook):
results = run_dbt(["test"], expect_pass=True)
assert len(results) == 1
mock_post_model_hook.assert_called_once()

def test_data_test_runs_adapter_post_hook_fails(self, project):
results = run_dbt(["seed"])
assert len(results) == 1

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

mock_post_model_hook = mock.Mock()
mock_post_model_hook.side_effect = CompilationError(
"exception from adapter.post_model_hook"
)
with mock.patch.object(type(project.adapter), "post_model_hook", mock_post_model_hook):
(_, log_output) = run_dbt_and_capture(["test"], expect_pass=False)
assert "exception from adapter.post_model_hook" in log_output
17 changes: 17 additions & 0 deletions tests/functional/unit_testing/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,23 @@
tags: test_this
"""

test_my_model_pass_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: 3}
"""


test_my_model_simple_fixture_yml = """
unit_tests:
Expand Down
75 changes: 75 additions & 0 deletions tests/functional/unit_testing/test_ut_adapter_hooks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
from unittest import mock

import pytest

from dbt.tests.util import run_dbt, run_dbt_and_capture
from dbt_common.exceptions import CompilationError
from tests.functional.unit_testing.fixtures import (
my_model_a_sql,
my_model_b_sql,
my_model_sql,
test_my_model_pass_yml,
)


class BaseUnitTestAdapterHook:
@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_pass_yml,
}


class TestUnitTestAdapterPreHook(BaseUnitTestAdapterHook):
def test_unit_test_runs_adapter_pre_hook_passes(self, project):
results = run_dbt(["run"])
assert len(results) == 3

mock_pre_model_hook = mock.Mock()
with mock.patch.object(type(project.adapter), "pre_model_hook", mock_pre_model_hook):
results = run_dbt(["test", "--select", "test_name:test_my_model"], expect_pass=True)

assert len(results) == 1
mock_pre_model_hook.assert_called_once()

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

mock_pre_model_hook = mock.Mock()
mock_pre_model_hook.side_effect = CompilationError("exception from adapter.pre_model_hook")
with mock.patch.object(type(project.adapter), "pre_model_hook", mock_pre_model_hook):
(_, log_output) = run_dbt_and_capture(
["test", "--select", "test_name:test_my_model"], expect_pass=False
)
assert "exception from adapter.pre_model_hook" in log_output


class TestUnitTestAdapterPostHook(BaseUnitTestAdapterHook):
def test_unit_test_runs_adapter_post_hook_pass(self, project):
results = run_dbt(["run"])
assert len(results) == 3

mock_post_model_hook = mock.Mock()
with mock.patch.object(type(project.adapter), "post_model_hook", mock_post_model_hook):
results = run_dbt(["test", "--select", "test_name:test_my_model"], expect_pass=True)

assert len(results) == 1
mock_post_model_hook.assert_called_once()

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

mock_post_model_hook = mock.Mock()
mock_post_model_hook.side_effect = CompilationError(
"exception from adapter.post_model_hook"
)
with mock.patch.object(type(project.adapter), "post_model_hook", mock_post_model_hook):
(_, log_output) = run_dbt_and_capture(
["test", "--select", "test_name:test_my_model"], expect_pass=False
)
assert "exception from adapter.post_model_hook" in log_output
Loading