From 718b8921a4b88526f727d717afab2e8c3ca2169f Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 15 Sep 2023 17:43:03 -0400 Subject: [PATCH 01/37] add strategy parameter to TestConfig, default to ephemeral, catch strategy parameter in test materialization --- core/dbt/contracts/graph/model_config.py | 3 +- .../macros/materializations/tests/test.sql | 79 ++++++++++++------- .../macros/relations/create.sql | 6 ++ 3 files changed, 58 insertions(+), 30 deletions(-) diff --git a/core/dbt/contracts/graph/model_config.py b/core/dbt/contracts/graph/model_config.py index b28091e68c1..296b9ce3810 100644 --- a/core/dbt/contracts/graph/model_config.py +++ b/core/dbt/contracts/graph/model_config.py @@ -216,7 +216,6 @@ class Hook(dbtClassMixin, Replaceable): @dataclass class BaseConfig(AdditionalPropertiesAllowed, Replaceable): - # enable syntax like: config['key'] def __getitem__(self, key): return self.get(key) @@ -560,6 +559,8 @@ class TestConfig(NodeAndTestConfig): fail_calc: str = "count(*)" warn_if: str = "!= 0" error_if: str = "!= 0" + # this is typically a RelationType, but is generalized to StrEnum for adapters that implement custom relations + strategy: Union[StrEnum, str] = "ephemeral" @classmethod def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> bool: diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index fb6755058fd..8a1886859b6 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -1,48 +1,69 @@ {%- materialization test, default -%} - {% set relations = [] %} + {% set relations = [] %} + {% set relation_type = model.config.get('strategy') %} - {% if should_store_failures() %} + {% if relation_type in ['view', 'table'] %} - {% set identifier = model['alias'] %} - {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} - {% set target_relation = api.Relation.create( - identifier=identifier, schema=schema, database=database, type='table') -%} %} + {% set identifier = model['alias'] %} + {% set existing_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} + {% set target_relation = api.Relation.create(database, schema, identifier, relation_type) %} - {% if old_relation %} - {% do adapter.drop_relation(old_relation) %} - {% endif %} + {% call statement(auto_begin=True) %} + {% if existing_relation %} + {{ get_create_sql(False, target_relation, sql) }} + {% else %} + {{ get_create_sql(existing_relation, target_relation, sql) }} + {% endif %} + {% endcall %} + + {% do relations.append(target_relation) %} + + {% set main_sql %} + select * from {{ target_relation }} + {% endset %} - {% call statement(auto_begin=True) %} - {{ create_table_as(False, target_relation, sql) }} - {% endcall %} + {{ adapter.commit() }} - {% do relations.append(target_relation) %} + {% elif should_store_failures() %} - {% set main_sql %} - select * - from {{ target_relation }} - {% endset %} + {% set identifier = model['alias'] %} + {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} + {% set target_relation = api.Relation.create( + identifier=identifier, schema=schema, database=database, type='table' + ) %} - {{ adapter.commit() }} + {% if old_relation %} + {% do adapter.drop_relation(old_relation) %} + {% endif %} - {% else %} + {% call statement(auto_begin=True) %} + {{ create_table_as(False, target_relation, sql) }} + {% endcall %} - {% set main_sql = sql %} + {% do relations.append(target_relation) %} - {% endif %} + {% set main_sql %} + select * from {{ target_relation }} + {% endset %} - {% set limit = config.get('limit') %} - {% set fail_calc = config.get('fail_calc') %} - {% set warn_if = config.get('warn_if') %} - {% set error_if = config.get('error_if') %} + {{ adapter.commit() }} - {% call statement('main', fetch_result=True) -%} + {% else %} + + {% set main_sql = sql %} + + {% endif %} - {{ get_test_sql(main_sql, fail_calc, warn_if, error_if, limit)}} + {% set limit = config.get('limit') %} + {% set fail_calc = config.get('fail_calc') %} + {% set warn_if = config.get('warn_if') %} + {% set error_if = config.get('error_if') %} - {%- endcall %} + {% call statement('main', fetch_result=True) -%} + {{ get_test_sql(main_sql, fail_calc, warn_if, error_if, limit)}} + {%- endcall %} - {{ return({'relations': relations}) }} + {{ return({'relations': relations}) }} {%- endmaterialization -%} diff --git a/core/dbt/include/global_project/macros/relations/create.sql b/core/dbt/include/global_project/macros/relations/create.sql index 0cd1e0a70a3..b9b0f087ca1 100644 --- a/core/dbt/include/global_project/macros/relations/create.sql +++ b/core/dbt/include/global_project/macros/relations/create.sql @@ -6,6 +6,12 @@ {%- macro default__get_create_sql(relation, sql) -%} + {%- if relation.is_view -%} + {{ get_create_view_as_sql(relation, sql) }} + + {%- if relation.is_table -%} + {{ get_create_table_as_sql(false, relation, sql) }} + {%- if relation.is_materialized_view -%} {{ get_create_materialized_view_as_sql(relation, sql) }} From d4dfec8b53dbb7aefee7d6e09697d30da46b99fc Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 15 Sep 2023 17:44:39 -0400 Subject: [PATCH 02/37] changie --- .changes/unreleased/Features-20230915-174428.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Features-20230915-174428.yaml diff --git a/.changes/unreleased/Features-20230915-174428.yaml b/.changes/unreleased/Features-20230915-174428.yaml new file mode 100644 index 00000000000..40e97f7e941 --- /dev/null +++ b/.changes/unreleased/Features-20230915-174428.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Support persisting test results as views +time: 2023-09-15T17:44:28.833877-04:00 +custom: + Author: mikealfare + Issue: "6914" From e999ed2551b07e9a33cbfda86824eace5f0cb004 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 15 Sep 2023 23:04:55 -0400 Subject: [PATCH 03/37] create test results as views --- core/dbt/contracts/graph/model_config.py | 4 +- core/dbt/contracts/graph/nodes.py | 9 +-- .../macros/materializations/configs.sql | 7 +- .../macros/materializations/tests/test.sql | 15 ++-- .../macros/relations/create.sql | 6 +- core/dbt/parser/generic_test_builders.py | 8 +- .../test_persist_results.py | 77 +++++++++++++++++++ 7 files changed, 107 insertions(+), 19 deletions(-) create mode 100644 tests/adapter/dbt/tests/adapter/store_test_failures_tests/test_persist_results.py diff --git a/core/dbt/contracts/graph/model_config.py b/core/dbt/contracts/graph/model_config.py index 296b9ce3810..d03d49e8186 100644 --- a/core/dbt/contracts/graph/model_config.py +++ b/core/dbt/contracts/graph/model_config.py @@ -559,8 +559,7 @@ class TestConfig(NodeAndTestConfig): fail_calc: str = "count(*)" warn_if: str = "!= 0" error_if: str = "!= 0" - # this is typically a RelationType, but is generalized to StrEnum for adapters that implement custom relations - strategy: Union[StrEnum, str] = "ephemeral" + strategy: Optional[str] = None @classmethod def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> bool: @@ -573,6 +572,7 @@ def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> boo "warn_if", "error_if", "store_failures", + "strategy", ] seen = set() diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index 35d2ee4d810..84cf96d96b7 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -779,7 +779,6 @@ def same_contract(self, old, adapter_type=None) -> bool: or enforced_column_constraint_removed or materialization_changed ): - breaking_changes = [] if contract_enforced_disabled: breaking_changes.append( @@ -984,15 +983,15 @@ def language(self): class TestShouldStoreFailures: @property def should_store_failures(self): - if self.config.store_failures: + if self.config.strategy in ["view", "table"]: + return True + elif self.config.store_failures: return self.config.store_failures return get_flags().STORE_FAILURES @property def is_relational(self): - if self.should_store_failures: - return True - return False + return self.should_store_failures @dataclass diff --git a/core/dbt/include/global_project/macros/materializations/configs.sql b/core/dbt/include/global_project/macros/materializations/configs.sql index d15ccb8e603..c9a8d3efe6b 100644 --- a/core/dbt/include/global_project/macros/materializations/configs.sql +++ b/core/dbt/include/global_project/macros/materializations/configs.sql @@ -13,9 +13,14 @@ {% macro should_store_failures() %} + {% set config_store_failures_strategy = config.get('strategy') %} {% set config_store_failures = config.get('store_failures') %} - {% if config_store_failures is none %} + + {% if config_store_failures_strategy in ['view', 'table'] %} + {% set config_store_failures = True %} + {% elif config_store_failures is none %} {% set config_store_failures = flags.STORE_FAILURES %} {% endif %} + {% do return(config_store_failures) %} {% endmacro %} diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index 8a1886859b6..ea79ce9f99e 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -1,19 +1,21 @@ {%- materialization test, default -%} {% set relations = [] %} - {% set relation_type = model.config.get('strategy') %} + {% set relation_type = config.get('strategy') %} {% if relation_type in ['view', 'table'] %} {% set identifier = model['alias'] %} {% set existing_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} - {% set target_relation = api.Relation.create(database, schema, identifier, relation_type) %} + {% set target_relation = api.Relation.create( + database=database, schema=schema, identifier=identifier, type=relation_type + ) %} {% call statement(auto_begin=True) %} {% if existing_relation %} - {{ get_create_sql(False, target_relation, sql) }} + {{ get_replace_sql(existing_relation, target_relation, sql) }} {% else %} - {{ get_create_sql(existing_relation, target_relation, sql) }} + {{ get_create_sql(target_relation, sql) }} {% endif %} {% endcall %} @@ -30,8 +32,7 @@ {% set identifier = model['alias'] %} {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} {% set target_relation = api.Relation.create( - identifier=identifier, schema=schema, database=database, type='table' - ) %} + identifier=identifier, schema=schema, database=database, type='table') %} {% if old_relation %} {% do adapter.drop_relation(old_relation) %} @@ -61,7 +62,7 @@ {% set error_if = config.get('error_if') %} {% call statement('main', fetch_result=True) -%} - {{ get_test_sql(main_sql, fail_calc, warn_if, error_if, limit)}} + {{ get_test_sql(main_sql, fail_calc, warn_if, error_if, limit) }} {%- endcall %} {{ return({'relations': relations}) }} diff --git a/core/dbt/include/global_project/macros/relations/create.sql b/core/dbt/include/global_project/macros/relations/create.sql index b9b0f087ca1..3522392d2cb 100644 --- a/core/dbt/include/global_project/macros/relations/create.sql +++ b/core/dbt/include/global_project/macros/relations/create.sql @@ -9,10 +9,10 @@ {%- if relation.is_view -%} {{ get_create_view_as_sql(relation, sql) }} - {%- if relation.is_table -%} - {{ get_create_table_as_sql(false, relation, sql) }} + {%- elif relation.is_table -%} + {{ get_create_table_as_sql(False, relation, sql) }} - {%- if relation.is_materialized_view -%} + {%- elif relation.is_materialized_view -%} {{ get_create_materialized_view_as_sql(relation, sql) }} {%- else -%} diff --git a/core/dbt/parser/generic_test_builders.py b/core/dbt/parser/generic_test_builders.py index 69c86853162..9d4bd618433 100644 --- a/core/dbt/parser/generic_test_builders.py +++ b/core/dbt/parser/generic_test_builders.py @@ -101,6 +101,7 @@ class TestBuilder(Generic[Testable]): "error_if", "fail_calc", "store_failures", + "strategy", "meta", "database", "schema", @@ -149,7 +150,6 @@ def __init__( if not value and "config" in self.args: value = self.args["config"].pop(key, None) if isinstance(value, str): - try: value = get_rendered(value, render_ctx, native=True) except UndefinedMacroError as e: @@ -242,6 +242,10 @@ def severity(self) -> Optional[str]: def store_failures(self) -> Optional[bool]: return self.config.get("store_failures") + @property + def strategy(self) -> Optional[str]: + return self.config.get("strategy") + @property def where(self) -> Optional[str]: return self.config.get("where") @@ -294,6 +298,8 @@ def get_static_config(self): config["fail_calc"] = self.fail_calc if self.store_failures is not None: config["store_failures"] = self.store_failures + if self.strategy is not None: + config["strategy"] = self.strategy if self.meta is not None: config["meta"] = self.meta if self.database is not None: diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/test_persist_results.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/test_persist_results.py new file mode 100644 index 00000000000..d4b0e27be11 --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/test_persist_results.py @@ -0,0 +1,77 @@ +import pytest + +from dbt.contracts.results import TestStatus +from dbt.tests.util import run_dbt + + +seeds__chipmunks_stage = """ +name,shirt +alvin,red +simon,blue +theodore,green +""".strip() + + +models__chipmunks = """ +{{ config(materialized='view') }} +select * +from {{ ref('chipmunks_stage') }} +""" + +tests__fail_with_view_strategy = """ +{{ config(strategy="view") }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +tests__pass_with_view_strategy = """ +{{ config(strategy="view") }} +select * +from {{ ref('chipmunks') }} +where shirt = 'purple' +""" + + +class TestPersistResults: + @pytest.fixture(scope="function", autouse=True) + def setup_teardown(self, project): + run_dbt(["seed"]) + run_dbt(["run"]) + + yield + + with project.adapter.connection_named("__test"): + test_results_schema = project.adapter.Relation.create( + database=project.database, schema=f"{project.test_schema}_dbt_test__audit" + ) + relations_schema = project.adapter.Relation.create( + database=project.database, schema=project.test_schema + ) + project.adapter.drop_schema(test_results_schema) + project.adapter.drop_schema(relations_schema) + + @pytest.fixture(scope="class") + def seeds(self): + return {"chipmunks_stage.csv": seeds__chipmunks_stage} + + @pytest.fixture(scope="class") + def models(self): + return {"chipmunks.sql": models__chipmunks} + + @pytest.fixture(scope="class") + def tests(self): + return { + "fail_with_view_strategy.sql": tests__fail_with_view_strategy, + "pass_with_view_strategy.sql": tests__pass_with_view_strategy, + } + + def test_tests_run_successfully_and_are_persisted_as_views(self, project): + results = run_dbt(["test"], expect_pass=False) + actual_results = {(result.node.name, result.status) for result in results} + expected_results = { + ("pass_with_view_strategy", TestStatus.Pass), + ("fail_with_view_strategy", TestStatus.Fail), + } + assert actual_results == expected_results From f5e07971493e0c0e872fcf7b259290d386ef6c92 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 15 Sep 2023 23:09:34 -0400 Subject: [PATCH 04/37] removed unnecessary formatting changes --- .../global_project/macros/materializations/configs.sql | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/dbt/include/global_project/macros/materializations/configs.sql b/core/dbt/include/global_project/macros/materializations/configs.sql index c9a8d3efe6b..a071b807098 100644 --- a/core/dbt/include/global_project/macros/materializations/configs.sql +++ b/core/dbt/include/global_project/macros/materializations/configs.sql @@ -13,14 +13,12 @@ {% macro should_store_failures() %} - {% set config_store_failures_strategy = config.get('strategy') %} {% set config_store_failures = config.get('store_failures') %} - + {% set config_store_failures_strategy = config.get('strategy') %} {% if config_store_failures_strategy in ['view', 'table'] %} {% set config_store_failures = True %} {% elif config_store_failures is none %} {% set config_store_failures = flags.STORE_FAILURES %} {% endif %} - {% do return(config_store_failures) %} {% endmacro %} From 8e366bbc72b6b16591d30acd384ff9be07b97bb3 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 15 Sep 2023 23:13:02 -0400 Subject: [PATCH 05/37] removed unnecessary formatting changes --- .../macros/materializations/tests/test.sql | 56 ++++++++++--------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index ea79ce9f99e..2c906a40ee0 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -27,44 +27,46 @@ {{ adapter.commit() }} - {% elif should_store_failures() %} + {% elif should_store_failures() %} - {% set identifier = model['alias'] %} - {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} - {% set target_relation = api.Relation.create( - identifier=identifier, schema=schema, database=database, type='table') %} + {% set identifier = model['alias'] %} + {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} + {% set target_relation = api.Relation.create( + identifier=identifier, schema=schema, database=database, type='table') %} - {% if old_relation %} - {% do adapter.drop_relation(old_relation) %} - {% endif %} + {% if old_relation %} + {% do adapter.drop_relation(old_relation) %} + {% endif %} - {% call statement(auto_begin=True) %} - {{ create_table_as(False, target_relation, sql) }} - {% endcall %} + {% call statement(auto_begin=True) %} + {{ create_table_as(False, target_relation, sql) }} + {% endcall %} - {% do relations.append(target_relation) %} + {% do relations.append(target_relation) %} - {% set main_sql %} - select * from {{ target_relation }} - {% endset %} + {% set main_sql %} + select * from {{ target_relation }} + {% endset %} - {{ adapter.commit() }} + {{ adapter.commit() }} + + {% else %} + + {% set main_sql = sql %} - {% else %} + {% endif %} - {% set main_sql = sql %} + {% set limit = config.get('limit') %} + {% set fail_calc = config.get('fail_calc') %} + {% set warn_if = config.get('warn_if') %} + {% set error_if = config.get('error_if') %} - {% endif %} + {% call statement('main', fetch_result=True) -%} - {% set limit = config.get('limit') %} - {% set fail_calc = config.get('fail_calc') %} - {% set warn_if = config.get('warn_if') %} - {% set error_if = config.get('error_if') %} + {{ get_test_sql(main_sql, fail_calc, warn_if, error_if, limit) }} - {% call statement('main', fetch_result=True) -%} - {{ get_test_sql(main_sql, fail_calc, warn_if, error_if, limit) }} - {%- endcall %} + {%- endcall %} - {{ return({'relations': relations}) }} + {{ return({'relations': relations}) }} {%- endmaterialization -%} From 1a3bd81edc6d1d32606bfe2722ab2c8fb1521ba2 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 15 Sep 2023 23:14:27 -0400 Subject: [PATCH 06/37] removed unnecessary formatting changes --- .../global_project/macros/materializations/tests/test.sql | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index 2c906a40ee0..8465ca4af27 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -45,7 +45,8 @@ {% do relations.append(target_relation) %} {% set main_sql %} - select * from {{ target_relation }} + select * + from {{ target_relation }} {% endset %} {{ adapter.commit() }} @@ -63,7 +64,7 @@ {% call statement('main', fetch_result=True) -%} - {{ get_test_sql(main_sql, fail_calc, warn_if, error_if, limit) }} + {{ get_test_sql(main_sql, fail_calc, warn_if, error_if, limit)}} {%- endcall %} From 7b4be33a1b40fb913bc73df9f0dcc52d4cfa8d93 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 15 Sep 2023 23:15:59 -0400 Subject: [PATCH 07/37] removed unnecessary formatting changes --- .../macros/materializations/tests/test.sql | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index 8465ca4af27..66ad5c93312 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -29,27 +29,27 @@ {% elif should_store_failures() %} - {% set identifier = model['alias'] %} - {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} - {% set target_relation = api.Relation.create( - identifier=identifier, schema=schema, database=database, type='table') %} + {% set identifier = model['alias'] %} + {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} + {% set target_relation = api.Relation.create( + identifier=identifier, schema=schema, database=database, type='table') -%} %} - {% if old_relation %} - {% do adapter.drop_relation(old_relation) %} - {% endif %} + {% if old_relation %} + {% do adapter.drop_relation(old_relation) %} + {% endif %} - {% call statement(auto_begin=True) %} - {{ create_table_as(False, target_relation, sql) }} - {% endcall %} + {% call statement(auto_begin=True) %} + {{ create_table_as(False, target_relation, sql) }} + {% endcall %} - {% do relations.append(target_relation) %} + {% do relations.append(target_relation) %} - {% set main_sql %} - select * - from {{ target_relation }} - {% endset %} + {% set main_sql %} + select * + from {{ target_relation }} + {% endset %} - {{ adapter.commit() }} + {{ adapter.commit() }} {% else %} From a149d669100315c2b663c7e4da0561d415b21edc Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 15 Sep 2023 23:16:37 -0400 Subject: [PATCH 08/37] removed unnecessary formatting changes --- .../global_project/macros/materializations/tests/test.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index 66ad5c93312..a135e88f38d 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -1,6 +1,6 @@ {%- materialization test, default -%} - {% set relations = [] %} + {% set relations = [] %} {% set relation_type = config.get('strategy') %} {% if relation_type in ['view', 'table'] %} From 0313904bc32a6e3a9fe4973f320b0bec23fac25a Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 15 Sep 2023 23:22:15 -0400 Subject: [PATCH 09/37] removed unnecessary formatting changes --- core/dbt/contracts/graph/nodes.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index 84cf96d96b7..2cf580682a1 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -991,7 +991,9 @@ def should_store_failures(self): @property def is_relational(self): - return self.should_store_failures + if self.should_store_failures: + return True + return False @dataclass From ccf199ccc970c62ac3ed15a620ccf760c5a1e684 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 15 Sep 2023 23:35:29 -0400 Subject: [PATCH 10/37] removed unnecessary formatting changes --- core/dbt/parser/generic_test_builders.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/core/dbt/parser/generic_test_builders.py b/core/dbt/parser/generic_test_builders.py index 9d4bd618433..dfdcaa4822a 100644 --- a/core/dbt/parser/generic_test_builders.py +++ b/core/dbt/parser/generic_test_builders.py @@ -101,7 +101,6 @@ class TestBuilder(Generic[Testable]): "error_if", "fail_calc", "store_failures", - "strategy", "meta", "database", "schema", @@ -242,10 +241,6 @@ def severity(self) -> Optional[str]: def store_failures(self) -> Optional[bool]: return self.config.get("store_failures") - @property - def strategy(self) -> Optional[str]: - return self.config.get("strategy") - @property def where(self) -> Optional[str]: return self.config.get("where") @@ -298,8 +293,6 @@ def get_static_config(self): config["fail_calc"] = self.fail_calc if self.store_failures is not None: config["store_failures"] = self.store_failures - if self.strategy is not None: - config["strategy"] = self.strategy if self.meta is not None: config["meta"] = self.meta if self.database is not None: From f76fc74eecd4e264e74de747dd53cdaaf0c20d81 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 15 Sep 2023 23:36:53 -0400 Subject: [PATCH 11/37] removed unnecessary formatting changes --- core/dbt/parser/generic_test_builders.py | 1 + 1 file changed, 1 insertion(+) diff --git a/core/dbt/parser/generic_test_builders.py b/core/dbt/parser/generic_test_builders.py index dfdcaa4822a..69c86853162 100644 --- a/core/dbt/parser/generic_test_builders.py +++ b/core/dbt/parser/generic_test_builders.py @@ -149,6 +149,7 @@ def __init__( if not value and "config" in self.args: value = self.args["config"].pop(key, None) if isinstance(value, str): + try: value = get_rendered(value, render_ctx, native=True) except UndefinedMacroError as e: From 9a0656f08c6145f809263767deb123e9babc39c9 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Sat, 16 Sep 2023 00:05:28 -0400 Subject: [PATCH 12/37] updated test expected values for new config option --- tests/functional/artifacts/expected_manifest.py | 2 +- tests/functional/list/test_list.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index 6082ae4b8d4..6ea3f6cf2b1 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -132,6 +132,7 @@ def get_rendered_tst_config(**updates): "tags": [], "severity": "ERROR", "store_failures": None, + "strategy": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", @@ -201,7 +202,6 @@ def __str__(self): def expected_seeded_manifest(project, model_database=None, quote_model=False): - model_sql_path = os.path.join("models", "model.sql") second_model_sql_path = os.path.join("models", "second_model.sql") model_schema_yml_path = os.path.join("models", "schema.yml") diff --git a/tests/functional/list/test_list.py b/tests/functional/list/test_list.py index f6db7461274..f3793cb859a 100644 --- a/tests/functional/list/test_list.py +++ b/tests/functional/list/test_list.py @@ -443,6 +443,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, + "strategy": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", @@ -470,6 +471,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, + "strategy": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", @@ -500,6 +502,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, + "strategy": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", From 40b646b7de563e3522d488ede9f6c0b0b133fbe3 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Sat, 16 Sep 2023 15:28:45 -0400 Subject: [PATCH 13/37] break up tests into reusable tests and adapter specific configuration, update test to check for relation type and confirm views update --- .../adapter/persist_test_results/_files.py | 44 ++++++ .../adapter/persist_test_results/basic.py | 128 ++++++++++++++++++ .../test_persist_results.py | 77 ----------- .../test_persist_test_results.py | 20 +++ .../functional/persist_test_results/utils.py | 74 ++++++++++ 5 files changed, 266 insertions(+), 77 deletions(-) create mode 100644 tests/adapter/dbt/tests/adapter/persist_test_results/_files.py create mode 100644 tests/adapter/dbt/tests/adapter/persist_test_results/basic.py delete mode 100644 tests/adapter/dbt/tests/adapter/store_test_failures_tests/test_persist_results.py create mode 100644 tests/functional/persist_test_results/test_persist_test_results.py create mode 100644 tests/functional/persist_test_results/utils.py diff --git a/tests/adapter/dbt/tests/adapter/persist_test_results/_files.py b/tests/adapter/dbt/tests/adapter/persist_test_results/_files.py new file mode 100644 index 00000000000..5c2739374ef --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/persist_test_results/_files.py @@ -0,0 +1,44 @@ +SEED__CHIPMUNKS = """ +name,shirt +alvin,red +simon,blue +theodore,green +""".strip() + + +MODEL__CHIPMUNKS = """ +{{ config(materialized='table') }} +select * +from {{ ref('chipmunks_stage') }} +""" + +TEST__FAIL_WITH_VIEW_STRATEGY = """ +{{ config(strategy="view") }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__PASS_WITH_VIEW_STRATEGY = """ +{{ config(strategy="view") }} +select * +from {{ ref('chipmunks') }} +where shirt = 'purple' +""" + + +TEST__FAIL_WITH_TABLE_STRATEGY = """ +{{ config(strategy="table") }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__PASS_WITH_TABLE_STRATEGY = """ +{{ config(strategy="table") }} +select * +from {{ ref('chipmunks') }} +where shirt = 'purple' +""" diff --git a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py new file mode 100644 index 00000000000..b17a0534f20 --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py @@ -0,0 +1,128 @@ +from typing import Dict, Set, Tuple + +import pytest + +from dbt.contracts.results import TestStatus +from dbt.tests.util import run_dbt + +from dbt.tests.adapter.persist_test_results._files import ( + SEED__CHIPMUNKS, + MODEL__CHIPMUNKS, + TEST__FAIL_WITH_VIEW_STRATEGY, + TEST__PASS_WITH_VIEW_STRATEGY, + TEST__FAIL_WITH_TABLE_STRATEGY, + TEST__PASS_WITH_TABLE_STRATEGY, +) + + +class PersistTestResults: + seed_table: str = "chipmunks_stage" + model_table: str = "chipmunks" + + audit_schema: str + + @pytest.fixture(scope="class", autouse=True) + def setup_teardown_class(self, project): + # the seed doesn't get touched, load it once + run_dbt(["seed"]) + yield + + @pytest.fixture(scope="function", autouse=True) + def setup_teardown_method(self, project, setup_teardown_class): + # make sure the model is always right + run_dbt(["run"]) + + # the name of the audit schema doesn't change in a class, but the fixtures run out of order for some reason + self.audit_schema = f"{project.test_schema}_dbt_test__audit" + + yield + + # clear out the audit schema after each test case + with project.adapter.connection_named("__test"): + audit_schema = project.adapter.Relation.create( + database=project.database, schema=self.audit_schema + ) + project.adapter.drop_schema(audit_schema) + + @pytest.fixture(scope="class") + def seeds(self): + return {f"{self.seed_table}.csv": SEED__CHIPMUNKS} + + @pytest.fixture(scope="class") + def models(self): + return {f"{self.model_table}.sql": MODEL__CHIPMUNKS} + + @pytest.fixture(scope="class") + def tests(self): + return { + "fail_with_view_strategy.sql": TEST__FAIL_WITH_VIEW_STRATEGY, + "pass_with_view_strategy.sql": TEST__PASS_WITH_VIEW_STRATEGY, + "fail_with_table_strategy.sql": TEST__FAIL_WITH_TABLE_STRATEGY, + "pass_with_table_strategy.sql": TEST__PASS_WITH_TABLE_STRATEGY, + } + + def get_audit_relation_summary(self, project) -> Set[Tuple]: + """ + Return summary stats about each relation in the audit schema to be verified in a test. + + Args: + project: the project fixture + + Returns: + Relation stats, e.g.: + { + ("my_table", "table", 0) + ("my_view", "view", 1) + } + """ + raise NotImplementedError( + "To use this test, please implement `get_audit_relation_summary`, inherited from `PersistTestResults`." + ) + + def insert_record(self, project, record: Dict[str, str]): + raise NotImplementedError( + "To use this test, please implement `insert_record`, inherited from `PersistTestResults`." + ) + + def delete_record(self, project, record: Dict[str, str]): + raise NotImplementedError( + "To use this test, please implement `delete_record`, inherited from `PersistTestResults`." + ) + + def test_tests_run_successfully_and_are_persisted_correctly(self, project): + # run the tests once + results = run_dbt(["test"], expect_pass=False) + + # make sure the results are what we expect + actual_results = {(result.node.name, result.status) for result in results} + expected_results = { + ("pass_with_view_strategy", TestStatus.Pass), + ("fail_with_view_strategy", TestStatus.Fail), + ("pass_with_table_strategy", TestStatus.Pass), + ("fail_with_table_strategy", TestStatus.Fail), + } + assert actual_results == expected_results + + # show that the results are persisted in the correct database objects + persisted_objects = self.get_audit_relation_summary(project) + assert persisted_objects == { + ("pass_with_view_strategy", "view", 0), + ("fail_with_view_strategy", "view", 1), + ("pass_with_table_strategy", "table", 0), + ("fail_with_table_strategy", "table", 1), + } + + # insert a new record in the model that fails the "pass" tests + self.insert_record(project, {"name": "dave", "shirt": "purple"}) + + # delete the original record from the model that failed the "fail" tests + self.delete_record(project, {"name": "theodore", "shirt": "green"}) + + # show that the views update and the tables do not + persisted_objects = self.get_audit_relation_summary(project) + assert persisted_objects == { + ("pass_with_view_strategy", "view", 1), # the views update + ("fail_with_view_strategy", "view", 0), # the views update + ("pass_with_table_strategy", "table", 0), # the tables do not update + ("fail_with_table_strategy", "table", 1), # the tables do not update + } diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/test_persist_results.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/test_persist_results.py deleted file mode 100644 index d4b0e27be11..00000000000 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/test_persist_results.py +++ /dev/null @@ -1,77 +0,0 @@ -import pytest - -from dbt.contracts.results import TestStatus -from dbt.tests.util import run_dbt - - -seeds__chipmunks_stage = """ -name,shirt -alvin,red -simon,blue -theodore,green -""".strip() - - -models__chipmunks = """ -{{ config(materialized='view') }} -select * -from {{ ref('chipmunks_stage') }} -""" - -tests__fail_with_view_strategy = """ -{{ config(strategy="view") }} -select * -from {{ ref('chipmunks') }} -where shirt = 'green' -""" - - -tests__pass_with_view_strategy = """ -{{ config(strategy="view") }} -select * -from {{ ref('chipmunks') }} -where shirt = 'purple' -""" - - -class TestPersistResults: - @pytest.fixture(scope="function", autouse=True) - def setup_teardown(self, project): - run_dbt(["seed"]) - run_dbt(["run"]) - - yield - - with project.adapter.connection_named("__test"): - test_results_schema = project.adapter.Relation.create( - database=project.database, schema=f"{project.test_schema}_dbt_test__audit" - ) - relations_schema = project.adapter.Relation.create( - database=project.database, schema=project.test_schema - ) - project.adapter.drop_schema(test_results_schema) - project.adapter.drop_schema(relations_schema) - - @pytest.fixture(scope="class") - def seeds(self): - return {"chipmunks_stage.csv": seeds__chipmunks_stage} - - @pytest.fixture(scope="class") - def models(self): - return {"chipmunks.sql": models__chipmunks} - - @pytest.fixture(scope="class") - def tests(self): - return { - "fail_with_view_strategy.sql": tests__fail_with_view_strategy, - "pass_with_view_strategy.sql": tests__pass_with_view_strategy, - } - - def test_tests_run_successfully_and_are_persisted_as_views(self, project): - results = run_dbt(["test"], expect_pass=False) - actual_results = {(result.node.name, result.status) for result in results} - expected_results = { - ("pass_with_view_strategy", TestStatus.Pass), - ("fail_with_view_strategy", TestStatus.Fail), - } - assert actual_results == expected_results diff --git a/tests/functional/persist_test_results/test_persist_test_results.py b/tests/functional/persist_test_results/test_persist_test_results.py new file mode 100644 index 00000000000..82b9d5b1c4b --- /dev/null +++ b/tests/functional/persist_test_results/test_persist_test_results.py @@ -0,0 +1,20 @@ +from typing import Dict, Set, Tuple + +from dbt.tests.adapter.persist_test_results.basic import PersistTestResults + +from tests.functional.persist_test_results.utils import ( + delete_record, + get_relation_summary_in_schema, + insert_record, +) + + +class TestPersistTestResults(PersistTestResults): + def get_audit_relation_summary(self, project) -> Set[Tuple]: + return get_relation_summary_in_schema(project, self.audit_schema) + + def insert_record(self, project, record: Dict[str, str]): + insert_record(project, project.test_schema, self.model_table, record) + + def delete_record(self, project, record: Dict[str, str]): + delete_record(project, project.test_schema, self.model_table, record) diff --git a/tests/functional/persist_test_results/utils.py b/tests/functional/persist_test_results/utils.py new file mode 100644 index 00000000000..056c6bfbe01 --- /dev/null +++ b/tests/functional/persist_test_results/utils.py @@ -0,0 +1,74 @@ +from typing import Dict, Set, Tuple + + +def get_relation_summary_in_schema(project, schema: str) -> Set[Tuple]: + """ + Returns a summary like this: + { + ("my_table", "table", 0), + ("my_view", "view", 1), + } + """ + # postgres only supports schema names of 63 characters + # a schema with a longer name still gets created, but the name gets truncated + schema_name = schema[:63] + + sql = f""" + select + tablename as relation_name, + 'table' as relation_type + from pg_tables + where schemaname ilike '{schema_name}' + union all + select + viewname as relation_name, + 'view' as relation_type + from pg_views + where schemaname ilike '{schema_name}' + union all + select + matviewname as relation_name, + 'materialized_view' as relation_type + from pg_matviews + where schemaname ilike '{schema_name}' + """ + relation_types = project.run_sql(sql, fetch="all") + + results = set() + for relation_name, relation_type in relation_types: + row_count_sql = f"select count(*) from {schema_name}.{relation_name}" + row_count = project.run_sql(row_count_sql, fetch="one")[0] + summary = (relation_name, relation_type, row_count) + results.add(summary) + + return results + + +def insert_record(project, schema: str, table_name: str, record: Dict[str, str]): + # postgres only supports schema names of 63 characters + # a schema with a longer name still gets created, but the name gets truncated + schema_name = schema[:63] + field_names, field_values = [], [] + for field_name, field_value in record.items(): + field_names.append(field_name) + field_values.append(f"'{field_value}'") + field_name_clause = ", ".join(field_names) + field_value_clause = ", ".join(field_values) + + sql = f""" + insert into {schema_name}.{table_name} ({field_name_clause}) + values ({field_value_clause}) + """ + project.run_sql(sql) + + +def delete_record(project, schema: str, table_name: str, record: Dict[str, str]): + schema_name = schema[:63] + where_clause = " and ".join( + [f"{field_name} = '{field_value}'" for field_name, field_value in record.items()] + ) + sql = f""" + delete from {schema_name}.{table_name} + where {where_clause} + """ + project.run_sql(sql) From 661c5f6a2ef2cec00cbd9146c6a319d6fb5ff697 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 21 Sep 2023 16:44:41 -0400 Subject: [PATCH 14/37] use built-in utility to test relation types, reduce configuration to just row counts --- .../adapter/persist_test_results/basic.py | 71 +++++++++++-------- .../test_persist_test_results.py | 8 +-- .../functional/persist_test_results/utils.py | 43 ++--------- 3 files changed, 48 insertions(+), 74 deletions(-) diff --git a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py index b17a0534f20..40dcd24ea6f 100644 --- a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py +++ b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py @@ -1,9 +1,10 @@ -from typing import Dict, Set, Tuple +from collections import namedtuple +from typing import Dict import pytest from dbt.contracts.results import TestStatus -from dbt.tests.util import run_dbt +from dbt.tests.util import run_dbt, check_relation_types from dbt.tests.adapter.persist_test_results._files import ( SEED__CHIPMUNKS, @@ -61,19 +62,16 @@ def tests(self): "pass_with_table_strategy.sql": TEST__PASS_WITH_TABLE_STRATEGY, } - def get_audit_relation_summary(self, project) -> Set[Tuple]: + def row_count(self, project, relation_name: str) -> int: """ - Return summary stats about each relation in the audit schema to be verified in a test. + Return the row count for the relation. Args: project: the project fixture + relation_name: the name of the relation Returns: - Relation stats, e.g.: - { - ("my_table", "table", 0) - ("my_view", "view", 1) - } + the row count as an integer """ raise NotImplementedError( "To use this test, please implement `get_audit_relation_summary`, inherited from `PersistTestResults`." @@ -90,39 +88,50 @@ def delete_record(self, project, record: Dict[str, str]): ) def test_tests_run_successfully_and_are_persisted_correctly(self, project): + # set up the expected results + TestResult = namedtuple("TestResult", ["name", "status", "type", "row_count"]) + expected_results = { + TestResult("pass_with_view_strategy", TestStatus.Pass, "view", 0), + TestResult("fail_with_view_strategy", TestStatus.Fail, "view", 1), + TestResult("pass_with_table_strategy", TestStatus.Pass, "table", 0), + TestResult("fail_with_table_strategy", TestStatus.Fail, "table", 1), + } + # run the tests once results = run_dbt(["test"], expect_pass=False) - # make sure the results are what we expect - actual_results = {(result.node.name, result.status) for result in results} - expected_results = { - ("pass_with_view_strategy", TestStatus.Pass), - ("fail_with_view_strategy", TestStatus.Fail), - ("pass_with_table_strategy", TestStatus.Pass), - ("fail_with_table_strategy", TestStatus.Fail), - } - assert actual_results == expected_results + # show that the statuses are what we expect + actual = {(result.node.name, result.status) for result in results} + expected = {(result.name, result.status) for result in expected_results} + assert actual == expected # show that the results are persisted in the correct database objects - persisted_objects = self.get_audit_relation_summary(project) - assert persisted_objects == { - ("pass_with_view_strategy", "view", 0), - ("fail_with_view_strategy", "view", 1), - ("pass_with_table_strategy", "table", 0), - ("fail_with_table_strategy", "table", 1), + check_relation_types( + project.adapter, {result.name: result.type for result in expected_results} + ) + + # show that only the failed records show up + actual = { + (result.name, self.row_count(project, result.name)) for result in expected_results } + expected = {(result.name, result.row_count) for result in expected_results} + assert actual == expected # insert a new record in the model that fails the "pass" tests + # show that the view updates, but not the table self.insert_record(project, {"name": "dave", "shirt": "purple"}) + expected_results.remove(TestResult("pass_with_view_strategy", TestStatus.Pass, "view", 0)) + expected_results.add(TestResult("pass_with_view_strategy", TestStatus.Pass, "view", 1)) # delete the original record from the model that failed the "fail" tests + # show that the view updates, but not the table self.delete_record(project, {"name": "theodore", "shirt": "green"}) + expected_results.remove(TestResult("fail_with_view_strategy", TestStatus.Fail, "view", 1)) + expected_results.add(TestResult("fail_with_view_strategy", TestStatus.Fail, "view", 0)) - # show that the views update and the tables do not - persisted_objects = self.get_audit_relation_summary(project) - assert persisted_objects == { - ("pass_with_view_strategy", "view", 1), # the views update - ("fail_with_view_strategy", "view", 0), # the views update - ("pass_with_table_strategy", "table", 0), # the tables do not update - ("fail_with_table_strategy", "table", 1), # the tables do not update + # show that the views update without needing to run dbt, but the tables do not update + actual = { + (result.name, self.row_count(project, result.name)) for result in expected_results } + expected = {(result.name, result.row_count) for result in expected_results} + assert actual == expected diff --git a/tests/functional/persist_test_results/test_persist_test_results.py b/tests/functional/persist_test_results/test_persist_test_results.py index 82b9d5b1c4b..1809ec8c7b4 100644 --- a/tests/functional/persist_test_results/test_persist_test_results.py +++ b/tests/functional/persist_test_results/test_persist_test_results.py @@ -1,17 +1,17 @@ -from typing import Dict, Set, Tuple +from typing import Dict from dbt.tests.adapter.persist_test_results.basic import PersistTestResults from tests.functional.persist_test_results.utils import ( delete_record, - get_relation_summary_in_schema, insert_record, + row_count, ) class TestPersistTestResults(PersistTestResults): - def get_audit_relation_summary(self, project) -> Set[Tuple]: - return get_relation_summary_in_schema(project, self.audit_schema) + def row_count(self, project, relation_name) -> int: + return row_count(project, self.audit_schema, relation_name) def insert_record(self, project, record: Dict[str, str]): insert_record(project, project.test_schema, self.model_table, record) diff --git a/tests/functional/persist_test_results/utils.py b/tests/functional/persist_test_results/utils.py index 056c6bfbe01..4e3a7d8ea39 100644 --- a/tests/functional/persist_test_results/utils.py +++ b/tests/functional/persist_test_results/utils.py @@ -1,47 +1,12 @@ -from typing import Dict, Set, Tuple +from typing import Dict -def get_relation_summary_in_schema(project, schema: str) -> Set[Tuple]: - """ - Returns a summary like this: - { - ("my_table", "table", 0), - ("my_view", "view", 1), - } - """ +def row_count(project, schema: str, relation_name: str) -> int: # postgres only supports schema names of 63 characters # a schema with a longer name still gets created, but the name gets truncated schema_name = schema[:63] - - sql = f""" - select - tablename as relation_name, - 'table' as relation_type - from pg_tables - where schemaname ilike '{schema_name}' - union all - select - viewname as relation_name, - 'view' as relation_type - from pg_views - where schemaname ilike '{schema_name}' - union all - select - matviewname as relation_name, - 'materialized_view' as relation_type - from pg_matviews - where schemaname ilike '{schema_name}' - """ - relation_types = project.run_sql(sql, fetch="all") - - results = set() - for relation_name, relation_type in relation_types: - row_count_sql = f"select count(*) from {schema_name}.{relation_name}" - row_count = project.run_sql(row_count_sql, fetch="one")[0] - summary = (relation_name, relation_type, row_count) - results.add(summary) - - return results + sql = f"select count(*) from {schema_name}.{relation_name}" + return project.run_sql(sql, fetch="one")[0] def insert_record(project, schema: str, table_name: str, record: Dict[str, str]): From 3fa08583c7605cb2be265e287c0e072aeab6dfc5 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 21 Sep 2023 17:06:44 -0400 Subject: [PATCH 15/37] move configuration into base test class --- .../adapter/persist_test_results/basic.py | 33 +++++++++++----- .../test_persist_test_results.py | 17 +------- .../functional/persist_test_results/utils.py | 39 ------------------- 3 files changed, 25 insertions(+), 64 deletions(-) delete mode 100644 tests/functional/persist_test_results/utils.py diff --git a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py index 40dcd24ea6f..1d80300b400 100644 --- a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py +++ b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py @@ -34,7 +34,9 @@ def setup_teardown_method(self, project, setup_teardown_class): run_dbt(["run"]) # the name of the audit schema doesn't change in a class, but the fixtures run out of order for some reason - self.audit_schema = f"{project.test_schema}_dbt_test__audit" + # postgres only supports schema names of 63 characters + # a schema with a longer name still gets created, but the name gets truncated + self.audit_schema = f"{project.test_schema}_dbt_test__audit"[:63] yield @@ -73,19 +75,32 @@ def row_count(self, project, relation_name: str) -> int: Returns: the row count as an integer """ - raise NotImplementedError( - "To use this test, please implement `get_audit_relation_summary`, inherited from `PersistTestResults`." - ) + sql = f"select count(*) from {self.audit_schema}.{relation_name}" + return project.run_sql(sql, fetch="one")[0] def insert_record(self, project, record: Dict[str, str]): - raise NotImplementedError( - "To use this test, please implement `insert_record`, inherited from `PersistTestResults`." - ) + field_names, field_values = [], [] + for field_name, field_value in record.items(): + field_names.append(field_name) + field_values.append(f"'{field_value}'") + field_name_clause = ", ".join(field_names) + field_value_clause = ", ".join(field_values) + + sql = f""" + insert into {project.test_schema}.{self.model_table} ({field_name_clause}) + values ({field_value_clause}) + """ + project.run_sql(sql) def delete_record(self, project, record: Dict[str, str]): - raise NotImplementedError( - "To use this test, please implement `delete_record`, inherited from `PersistTestResults`." + where_clause = " and ".join( + [f"{field_name} = '{field_value}'" for field_name, field_value in record.items()] ) + sql = f""" + delete from {project.test_schema}.{self.model_table} + where {where_clause} + """ + project.run_sql(sql) def test_tests_run_successfully_and_are_persisted_correctly(self, project): # set up the expected results diff --git a/tests/functional/persist_test_results/test_persist_test_results.py b/tests/functional/persist_test_results/test_persist_test_results.py index 1809ec8c7b4..75bfb5accdd 100644 --- a/tests/functional/persist_test_results/test_persist_test_results.py +++ b/tests/functional/persist_test_results/test_persist_test_results.py @@ -1,20 +1,5 @@ -from typing import Dict - from dbt.tests.adapter.persist_test_results.basic import PersistTestResults -from tests.functional.persist_test_results.utils import ( - delete_record, - insert_record, - row_count, -) - class TestPersistTestResults(PersistTestResults): - def row_count(self, project, relation_name) -> int: - return row_count(project, self.audit_schema, relation_name) - - def insert_record(self, project, record: Dict[str, str]): - insert_record(project, project.test_schema, self.model_table, record) - - def delete_record(self, project, record: Dict[str, str]): - delete_record(project, project.test_schema, self.model_table, record) + pass diff --git a/tests/functional/persist_test_results/utils.py b/tests/functional/persist_test_results/utils.py deleted file mode 100644 index 4e3a7d8ea39..00000000000 --- a/tests/functional/persist_test_results/utils.py +++ /dev/null @@ -1,39 +0,0 @@ -from typing import Dict - - -def row_count(project, schema: str, relation_name: str) -> int: - # postgres only supports schema names of 63 characters - # a schema with a longer name still gets created, but the name gets truncated - schema_name = schema[:63] - sql = f"select count(*) from {schema_name}.{relation_name}" - return project.run_sql(sql, fetch="one")[0] - - -def insert_record(project, schema: str, table_name: str, record: Dict[str, str]): - # postgres only supports schema names of 63 characters - # a schema with a longer name still gets created, but the name gets truncated - schema_name = schema[:63] - field_names, field_values = [], [] - for field_name, field_value in record.items(): - field_names.append(field_name) - field_values.append(f"'{field_value}'") - field_name_clause = ", ".join(field_names) - field_value_clause = ", ".join(field_values) - - sql = f""" - insert into {schema_name}.{table_name} ({field_name_clause}) - values ({field_value_clause}) - """ - project.run_sql(sql) - - -def delete_record(project, schema: str, table_name: str, record: Dict[str, str]): - schema_name = schema[:63] - where_clause = " and ".join( - [f"{field_name} = '{field_value}'" for field_name, field_value in record.items()] - ) - sql = f""" - delete from {schema_name}.{table_name} - where {where_clause} - """ - project.run_sql(sql) From 49b797e3a44cf756b31aed2df5d5d10e45b4c367 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 21 Sep 2023 17:19:36 -0400 Subject: [PATCH 16/37] separate setup and teardown methods, move postgres piece back into dbt-postgres --- .../dbt/tests/adapter/persist_test_results/basic.py | 13 +++++++------ .../test_persist_test_results.py | 8 +++++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py index 1d80300b400..aa44a2b5d81 100644 --- a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py +++ b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py @@ -23,21 +23,22 @@ class PersistTestResults: audit_schema: str @pytest.fixture(scope="class", autouse=True) - def setup_teardown_class(self, project): + def setup_class(self, project): # the seed doesn't get touched, load it once run_dbt(["seed"]) yield @pytest.fixture(scope="function", autouse=True) - def setup_teardown_method(self, project, setup_teardown_class): + def setup_method(self, project, setup_class): # make sure the model is always right run_dbt(["run"]) - # the name of the audit schema doesn't change in a class, but the fixtures run out of order for some reason - # postgres only supports schema names of 63 characters - # a schema with a longer name still gets created, but the name gets truncated - self.audit_schema = f"{project.test_schema}_dbt_test__audit"[:63] + # the name of the audit schema doesn't change in a class, but this doesn't run at the class level + self.audit_schema = f"{project.test_schema}_dbt_test__audit" + yield + @pytest.fixture(scope="function", autouse=True) + def teardown_method(self, project): yield # clear out the audit schema after each test case diff --git a/tests/functional/persist_test_results/test_persist_test_results.py b/tests/functional/persist_test_results/test_persist_test_results.py index 75bfb5accdd..70151aeb330 100644 --- a/tests/functional/persist_test_results/test_persist_test_results.py +++ b/tests/functional/persist_test_results/test_persist_test_results.py @@ -1,5 +1,11 @@ +import pytest + from dbt.tests.adapter.persist_test_results.basic import PersistTestResults class TestPersistTestResults(PersistTestResults): - pass + @pytest.fixture(scope="function", autouse=True) + def setup_audit_schema(self, project, setup_method): + # postgres only supports schema names of 63 characters + # a schema with a longer name still gets created, but the name gets truncated + self.audit_schema = self.audit_schema[:63] From 069a9b262300c8ed9ce0589d5750487265da6e5a Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 21 Sep 2023 17:29:29 -0400 Subject: [PATCH 17/37] shorten inserted record values to avoid data type issues in the model table --- tests/adapter/dbt/tests/adapter/persist_test_results/basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py index aa44a2b5d81..ac9ad2193da 100644 --- a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py +++ b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py @@ -135,7 +135,7 @@ def test_tests_run_successfully_and_are_persisted_correctly(self, project): # insert a new record in the model that fails the "pass" tests # show that the view updates, but not the table - self.insert_record(project, {"name": "dave", "shirt": "purple"}) + self.insert_record(project, {"name": "dave", "shirt": "grape"}) expected_results.remove(TestResult("pass_with_view_strategy", TestStatus.Pass, "view", 0)) expected_results.add(TestResult("pass_with_view_strategy", TestStatus.Pass, "view", 1)) From 61d514b0d7975b173aa8c6d25c128a9a7357bcd1 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 21 Sep 2023 17:29:53 -0400 Subject: [PATCH 18/37] shorten inserted record values to avoid data type issues in the model table --- tests/adapter/dbt/tests/adapter/persist_test_results/_files.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/adapter/dbt/tests/adapter/persist_test_results/_files.py b/tests/adapter/dbt/tests/adapter/persist_test_results/_files.py index 5c2739374ef..97cd769bb97 100644 --- a/tests/adapter/dbt/tests/adapter/persist_test_results/_files.py +++ b/tests/adapter/dbt/tests/adapter/persist_test_results/_files.py @@ -24,7 +24,7 @@ {{ config(strategy="view") }} select * from {{ ref('chipmunks') }} -where shirt = 'purple' +where shirt = 'grape' """ From beb0ed43a62469808e5d4a15ceb506f3383c7198 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 21 Sep 2023 21:42:49 -0400 Subject: [PATCH 19/37] add audit schema suffix as class attribute --- tests/adapter/dbt/tests/adapter/persist_test_results/basic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py index ac9ad2193da..1acc846d1d4 100644 --- a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py +++ b/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py @@ -19,6 +19,7 @@ class PersistTestResults: seed_table: str = "chipmunks_stage" model_table: str = "chipmunks" + audit_schema_suffix: str = "dbt_test__audit" audit_schema: str @@ -34,7 +35,7 @@ def setup_method(self, project, setup_class): run_dbt(["run"]) # the name of the audit schema doesn't change in a class, but this doesn't run at the class level - self.audit_schema = f"{project.test_schema}_dbt_test__audit" + self.audit_schema = f"{project.test_schema}_{self.audit_schema_suffix}" yield @pytest.fixture(scope="function", autouse=True) From 5aa2031838ba85ed483dac67e981043d46000493 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 28 Sep 2023 20:48:01 -0400 Subject: [PATCH 20/37] rename `strategy` parameter to `store_failures_as`; allow `store_failures` to drive whether failures are stored, defaulted as a table; allow override to view --- core/dbt/contracts/graph/model_config.py | 4 +- core/dbt/contracts/graph/nodes.py | 4 +- .../macros/materializations/configs.sql | 5 +-- .../macros/materializations/tests/test.sql | 33 +++------------ .../_files.py | 16 ++++---- .../basic.py | 40 +++++++++---------- .../functional/artifacts/expected_manifest.py | 2 +- tests/functional/list/test_list.py | 6 +-- .../test_persist_test_results.py | 7 +++- 9 files changed, 46 insertions(+), 71 deletions(-) rename tests/adapter/dbt/tests/adapter/{persist_test_results => store_test_failures_tests}/_files.py (62%) rename tests/adapter/dbt/tests/adapter/{persist_test_results => store_test_failures_tests}/basic.py (77%) rename tests/functional/{persist_test_results => store_test_failures}/test_persist_test_results.py (53%) diff --git a/core/dbt/contracts/graph/model_config.py b/core/dbt/contracts/graph/model_config.py index d03d49e8186..7eb10a896ec 100644 --- a/core/dbt/contracts/graph/model_config.py +++ b/core/dbt/contracts/graph/model_config.py @@ -554,12 +554,12 @@ class TestConfig(NodeAndTestConfig): # Annotated is used by mashumaro for jsonschema generation severity: Annotated[Severity, Pattern(SEVERITY_PATTERN)] = Severity("ERROR") store_failures: Optional[bool] = None + store_failures_as: Optional[str] = None where: Optional[str] = None limit: Optional[int] = None fail_calc: str = "count(*)" warn_if: str = "!= 0" error_if: str = "!= 0" - strategy: Optional[str] = None @classmethod def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> bool: @@ -572,7 +572,7 @@ def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> boo "warn_if", "error_if", "store_failures", - "strategy", + "store_failures_as", ] seen = set() diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index 2cf580682a1..a6dac504f1f 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -983,9 +983,7 @@ def language(self): class TestShouldStoreFailures: @property def should_store_failures(self): - if self.config.strategy in ["view", "table"]: - return True - elif self.config.store_failures: + if self.config.store_failures: return self.config.store_failures return get_flags().STORE_FAILURES diff --git a/core/dbt/include/global_project/macros/materializations/configs.sql b/core/dbt/include/global_project/macros/materializations/configs.sql index a071b807098..d15ccb8e603 100644 --- a/core/dbt/include/global_project/macros/materializations/configs.sql +++ b/core/dbt/include/global_project/macros/materializations/configs.sql @@ -14,10 +14,7 @@ {% macro should_store_failures() %} {% set config_store_failures = config.get('store_failures') %} - {% set config_store_failures_strategy = config.get('strategy') %} - {% if config_store_failures_strategy in ['view', 'table'] %} - {% set config_store_failures = True %} - {% elif config_store_failures is none %} + {% if config_store_failures is none %} {% set config_store_failures = flags.STORE_FAILURES %} {% endif %} {% do return(config_store_failures) %} diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index a135e88f38d..230cd86207a 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -1,45 +1,22 @@ {%- materialization test, default -%} {% set relations = [] %} - {% set relation_type = config.get('strategy') %} - {% if relation_type in ['view', 'table'] %} - - {% set identifier = model['alias'] %} - {% set existing_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} - {% set target_relation = api.Relation.create( - database=database, schema=schema, identifier=identifier, type=relation_type - ) %} - - {% call statement(auto_begin=True) %} - {% if existing_relation %} - {{ get_replace_sql(existing_relation, target_relation, sql) }} - {% else %} - {{ get_create_sql(target_relation, sql) }} - {% endif %} - {% endcall %} - - {% do relations.append(target_relation) %} - - {% set main_sql %} - select * from {{ target_relation }} - {% endset %} - - {{ adapter.commit() }} - - {% elif should_store_failures() %} + {% if should_store_failures() %} {% set identifier = model['alias'] %} {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} + + {% set relation_type = config.get('store_failures_as', 'table') %} {% set target_relation = api.Relation.create( - identifier=identifier, schema=schema, database=database, type='table') -%} %} + identifier=identifier, schema=schema, database=database, type=relation_type) -%} %} {% if old_relation %} {% do adapter.drop_relation(old_relation) %} {% endif %} {% call statement(auto_begin=True) %} - {{ create_table_as(False, target_relation, sql) }} + {{ get_create_sql(target_relation, sql) }} {% endcall %} {% do relations.append(target_relation) %} diff --git a/tests/adapter/dbt/tests/adapter/persist_test_results/_files.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py similarity index 62% rename from tests/adapter/dbt/tests/adapter/persist_test_results/_files.py rename to tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py index 97cd769bb97..b1174a383be 100644 --- a/tests/adapter/dbt/tests/adapter/persist_test_results/_files.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py @@ -12,32 +12,32 @@ from {{ ref('chipmunks_stage') }} """ -TEST__FAIL_WITH_VIEW_STRATEGY = """ -{{ config(strategy="view") }} +TEST__FAIL_AS_VIEW = """ +{{ config(store_failures_as="view") }} select * from {{ ref('chipmunks') }} where shirt = 'green' """ -TEST__PASS_WITH_VIEW_STRATEGY = """ -{{ config(strategy="view") }} +TEST__PASS_AS_VIEW = """ +{{ config(store_failures_as="view") }} select * from {{ ref('chipmunks') }} where shirt = 'grape' """ -TEST__FAIL_WITH_TABLE_STRATEGY = """ -{{ config(strategy="table") }} +TEST__FAIL_AS_TABLE = """ +{{ config(store_failures_as="table") }} select * from {{ ref('chipmunks') }} where shirt = 'green' """ -TEST__PASS_WITH_TABLE_STRATEGY = """ -{{ config(strategy="table") }} +TEST__PASS_AS_TABLE = """ +{{ config(store_failures_as="table") }} select * from {{ ref('chipmunks') }} where shirt = 'purple' diff --git a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py similarity index 77% rename from tests/adapter/dbt/tests/adapter/persist_test_results/basic.py rename to tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py index 1acc846d1d4..b2480bdd3e7 100644 --- a/tests/adapter/dbt/tests/adapter/persist_test_results/basic.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py @@ -6,17 +6,17 @@ from dbt.contracts.results import TestStatus from dbt.tests.util import run_dbt, check_relation_types -from dbt.tests.adapter.persist_test_results._files import ( +from dbt.tests.adapter.store_test_failures_tests._files import ( SEED__CHIPMUNKS, MODEL__CHIPMUNKS, - TEST__FAIL_WITH_VIEW_STRATEGY, - TEST__PASS_WITH_VIEW_STRATEGY, - TEST__FAIL_WITH_TABLE_STRATEGY, - TEST__PASS_WITH_TABLE_STRATEGY, + TEST__FAIL_AS_VIEW, + TEST__PASS_AS_VIEW, + TEST__FAIL_AS_TABLE, + TEST__PASS_AS_TABLE, ) -class PersistTestResults: +class StoreTestFailures: seed_table: str = "chipmunks_stage" model_table: str = "chipmunks" audit_schema_suffix: str = "dbt_test__audit" @@ -60,10 +60,10 @@ def models(self): @pytest.fixture(scope="class") def tests(self): return { - "fail_with_view_strategy.sql": TEST__FAIL_WITH_VIEW_STRATEGY, - "pass_with_view_strategy.sql": TEST__PASS_WITH_VIEW_STRATEGY, - "fail_with_table_strategy.sql": TEST__FAIL_WITH_TABLE_STRATEGY, - "pass_with_table_strategy.sql": TEST__PASS_WITH_TABLE_STRATEGY, + "fail_as_view.sql": TEST__FAIL_AS_VIEW, + "pass_as_view.sql": TEST__PASS_AS_VIEW, + "fail_as_table.sql": TEST__FAIL_AS_TABLE, + "pass_as_table.sql": TEST__PASS_AS_TABLE, } def row_count(self, project, relation_name: str) -> int: @@ -104,18 +104,18 @@ def delete_record(self, project, record: Dict[str, str]): """ project.run_sql(sql) - def test_tests_run_successfully_and_are_persisted_correctly(self, project): + def test_tests_run_successfully_and_are_stored_as_expected(self, project): # set up the expected results TestResult = namedtuple("TestResult", ["name", "status", "type", "row_count"]) expected_results = { - TestResult("pass_with_view_strategy", TestStatus.Pass, "view", 0), - TestResult("fail_with_view_strategy", TestStatus.Fail, "view", 1), - TestResult("pass_with_table_strategy", TestStatus.Pass, "table", 0), - TestResult("fail_with_table_strategy", TestStatus.Fail, "table", 1), + TestResult("pass_as_view", TestStatus.Pass, "view", 0), + TestResult("fail_as_view", TestStatus.Fail, "view", 1), + TestResult("pass_as_table", TestStatus.Pass, "table", 0), + TestResult("fail_as_table", TestStatus.Fail, "table", 1), } # run the tests once - results = run_dbt(["test"], expect_pass=False) + results = run_dbt(["test", "--store-failures"], expect_pass=False) # show that the statuses are what we expect actual = {(result.node.name, result.status) for result in results} @@ -137,14 +137,14 @@ def test_tests_run_successfully_and_are_persisted_correctly(self, project): # insert a new record in the model that fails the "pass" tests # show that the view updates, but not the table self.insert_record(project, {"name": "dave", "shirt": "grape"}) - expected_results.remove(TestResult("pass_with_view_strategy", TestStatus.Pass, "view", 0)) - expected_results.add(TestResult("pass_with_view_strategy", TestStatus.Pass, "view", 1)) + expected_results.remove(TestResult("pass_as_view", TestStatus.Pass, "view", 0)) + expected_results.add(TestResult("pass_as_view", TestStatus.Pass, "view", 1)) # delete the original record from the model that failed the "fail" tests # show that the view updates, but not the table self.delete_record(project, {"name": "theodore", "shirt": "green"}) - expected_results.remove(TestResult("fail_with_view_strategy", TestStatus.Fail, "view", 1)) - expected_results.add(TestResult("fail_with_view_strategy", TestStatus.Fail, "view", 0)) + expected_results.remove(TestResult("fail_as_view", TestStatus.Fail, "view", 1)) + expected_results.add(TestResult("fail_as_view", TestStatus.Fail, "view", 0)) # show that the views update without needing to run dbt, but the tables do not update actual = { diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index 6ea3f6cf2b1..a45b5e060b4 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -132,7 +132,7 @@ def get_rendered_tst_config(**updates): "tags": [], "severity": "ERROR", "store_failures": None, - "strategy": None, + "store_failures_as": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", diff --git a/tests/functional/list/test_list.py b/tests/functional/list/test_list.py index f3793cb859a..fda026c143a 100644 --- a/tests/functional/list/test_list.py +++ b/tests/functional/list/test_list.py @@ -443,7 +443,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, - "strategy": None, + "store_failures_as": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", @@ -471,7 +471,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, - "strategy": None, + "store_failures_as": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", @@ -502,7 +502,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, - "strategy": None, + "store_failures_as": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", diff --git a/tests/functional/persist_test_results/test_persist_test_results.py b/tests/functional/store_test_failures/test_persist_test_results.py similarity index 53% rename from tests/functional/persist_test_results/test_persist_test_results.py rename to tests/functional/store_test_failures/test_persist_test_results.py index 70151aeb330..4ac8c533832 100644 --- a/tests/functional/persist_test_results/test_persist_test_results.py +++ b/tests/functional/store_test_failures/test_persist_test_results.py @@ -1,11 +1,14 @@ import pytest -from dbt.tests.adapter.persist_test_results.basic import PersistTestResults +from dbt.tests.adapter.store_test_failures_tests.basic import StoreTestFailures -class TestPersistTestResults(PersistTestResults): +class TestStoreTestFailures(StoreTestFailures): @pytest.fixture(scope="function", autouse=True) def setup_audit_schema(self, project, setup_method): # postgres only supports schema names of 63 characters # a schema with a longer name still gets created, but the name gets truncated self.audit_schema = self.audit_schema[:63] + + def test_tests_run_successfully_and_are_stored_as_expected(self, project): + super().test_tests_run_successfully_and_are_stored_as_expected(project) From 9971be7acf9b9fe1a4d37d0810acef8a62c7d9e0 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 28 Sep 2023 20:49:07 -0400 Subject: [PATCH 21/37] updated changelog entry to reflect correct parameters --- .changes/unreleased/Features-20230915-174428.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/Features-20230915-174428.yaml b/.changes/unreleased/Features-20230915-174428.yaml index 40e97f7e941..1558d349463 100644 --- a/.changes/unreleased/Features-20230915-174428.yaml +++ b/.changes/unreleased/Features-20230915-174428.yaml @@ -1,5 +1,5 @@ kind: Features -body: Support persisting test results as views +body: Support storing test failures as views time: 2023-09-15T17:44:28.833877-04:00 custom: Author: mikealfare From 3ddc7d4e7b8e92f20862d213a4069bfd6275e2ae Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 28 Sep 2023 20:49:55 -0400 Subject: [PATCH 22/37] updated changelog entry to reflect correct parameters --- core/dbt/contracts/graph/nodes.py | 1 + 1 file changed, 1 insertion(+) diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index a6dac504f1f..35d2ee4d810 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -779,6 +779,7 @@ def same_contract(self, old, adapter_type=None) -> bool: or enforced_column_constraint_removed or materialization_changed ): + breaking_changes = [] if contract_enforced_disabled: breaking_changes.append( From 4b81e22e7a1add313bf1d2a5cd215635e7bd714b Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 28 Sep 2023 20:51:52 -0400 Subject: [PATCH 23/37] revert unexpected formatting changes from black --- tests/functional/artifacts/expected_manifest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index a45b5e060b4..0c5521fd8f5 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -202,6 +202,7 @@ def __str__(self): def expected_seeded_manifest(project, model_database=None, quote_model=False): + model_sql_path = os.path.join("models", "model.sql") second_model_sql_path = os.path.join("models", "second_model.sql") model_schema_yml_path = os.path.join("models", "schema.yml") From 08337746a3aee8d756d01b171dd1d22f0011f90e Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 28 Sep 2023 20:53:34 -0400 Subject: [PATCH 24/37] revert test debugging artifacts --- .../store_test_failures/test_persist_test_results.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/functional/store_test_failures/test_persist_test_results.py b/tests/functional/store_test_failures/test_persist_test_results.py index 4ac8c533832..4158adb895f 100644 --- a/tests/functional/store_test_failures/test_persist_test_results.py +++ b/tests/functional/store_test_failures/test_persist_test_results.py @@ -9,6 +9,3 @@ def setup_audit_schema(self, project, setup_method): # postgres only supports schema names of 63 characters # a schema with a longer name still gets created, but the name gets truncated self.audit_schema = self.audit_schema[:63] - - def test_tests_run_successfully_and_are_stored_as_expected(self, project): - super().test_tests_run_successfully_and_are_stored_as_expected(project) From eb1f3b4727e97cce7e1616f83591374386db138e Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 28 Sep 2023 20:56:57 -0400 Subject: [PATCH 25/37] update local variable to be more intuitive --- .../global_project/macros/materializations/tests/test.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index 230cd86207a..61c46ec28b8 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -7,9 +7,9 @@ {% set identifier = model['alias'] %} {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} - {% set relation_type = config.get('store_failures_as', 'table') %} + {% set store_failures_as = config.get('store_failures_as', 'table') %} {% set target_relation = api.Relation.create( - identifier=identifier, schema=schema, database=database, type=relation_type) -%} %} + identifier=identifier, schema=schema, database=database, type=store_failures_as) -%} %} {% if old_relation %} {% do adapter.drop_relation(old_relation) %} From cf8656561fa86fb8e780026cd36a8e35a5f12dbb Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 28 Sep 2023 21:35:26 -0400 Subject: [PATCH 26/37] update default for store_test_failures_as; rename postgres test to reflect store failures name --- core/dbt/contracts/graph/model_config.py | 2 +- ...test_persist_test_results.py => test_store_test_failures.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename tests/functional/store_test_failures/{test_persist_test_results.py => test_store_test_failures.py} (100%) diff --git a/core/dbt/contracts/graph/model_config.py b/core/dbt/contracts/graph/model_config.py index 7eb10a896ec..1072ab04344 100644 --- a/core/dbt/contracts/graph/model_config.py +++ b/core/dbt/contracts/graph/model_config.py @@ -554,7 +554,7 @@ class TestConfig(NodeAndTestConfig): # Annotated is used by mashumaro for jsonschema generation severity: Annotated[Severity, Pattern(SEVERITY_PATTERN)] = Severity("ERROR") store_failures: Optional[bool] = None - store_failures_as: Optional[str] = None + store_failures_as: Optional[str] = "table" where: Optional[str] = None limit: Optional[int] = None fail_calc: str = "count(*)" diff --git a/tests/functional/store_test_failures/test_persist_test_results.py b/tests/functional/store_test_failures/test_store_test_failures.py similarity index 100% rename from tests/functional/store_test_failures/test_persist_test_results.py rename to tests/functional/store_test_failures/test_store_test_failures.py From 2f1aa91f9d8a831794eba8de9404fec7c3e921a6 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 28 Sep 2023 21:36:57 -0400 Subject: [PATCH 27/37] remove duplicated default for store_failures_as --- .../global_project/macros/materializations/tests/test.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index 61c46ec28b8..eb287dec74d 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -7,7 +7,7 @@ {% set identifier = model['alias'] %} {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} - {% set store_failures_as = config.get('store_failures_as', 'table') %} + {% set store_failures_as = config.get('store_failures_as') %} {% set target_relation = api.Relation.create( identifier=identifier, schema=schema, database=database, type=store_failures_as) -%} %} From 96d18a985f6bb56be6e3f00f30b9181b4806eb98 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 28 Sep 2023 22:17:22 -0400 Subject: [PATCH 28/37] update expected test config dicts to include the new default value for store_failures_as --- tests/functional/artifacts/expected_manifest.py | 2 +- tests/functional/list/test_list.py | 6 +++--- tests/unit/test_contracts_graph_compiled.py | 1 + tests/unit/test_contracts_graph_parsed.py | 2 ++ 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index 0c5521fd8f5..d90c945957a 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -132,7 +132,7 @@ def get_rendered_tst_config(**updates): "tags": [], "severity": "ERROR", "store_failures": None, - "store_failures_as": None, + "store_failures_as": "table", "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", diff --git a/tests/functional/list/test_list.py b/tests/functional/list/test_list.py index fda026c143a..7e5ec789946 100644 --- a/tests/functional/list/test_list.py +++ b/tests/functional/list/test_list.py @@ -443,7 +443,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, - "store_failures_as": None, + "store_failures_as": "table", "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", @@ -471,7 +471,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, - "store_failures_as": None, + "store_failures_as": "table", "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", @@ -502,7 +502,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, - "store_failures_as": None, + "store_failures_as": "table", "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", diff --git a/tests/unit/test_contracts_graph_compiled.py b/tests/unit/test_contracts_graph_compiled.py index 1b4f0bbd045..dde740845e8 100644 --- a/tests/unit/test_contracts_graph_compiled.py +++ b/tests/unit/test_contracts_graph_compiled.py @@ -544,6 +544,7 @@ def basic_compiled_schema_test_dict(): "config": { "enabled": True, "materialized": "test", + "store_failures_as": "table", "tags": [], "severity": "warn", "schema": "dbt_test__audit", diff --git a/tests/unit/test_contracts_graph_parsed.py b/tests/unit/test_contracts_graph_parsed.py index 498ec5480e1..ceeff8330ce 100644 --- a/tests/unit/test_contracts_graph_parsed.py +++ b/tests/unit/test_contracts_graph_parsed.py @@ -1038,6 +1038,7 @@ def basic_parsed_schema_test_dict(): "config": { "enabled": True, "materialized": "test", + "store_failures_as": "table", "tags": [], "severity": "ERROR", "warn_if": "!= 0", @@ -1117,6 +1118,7 @@ def complex_parsed_schema_test_dict(): "config": { "enabled": True, "materialized": "table", + "store_failures_as": "table", "tags": [], "severity": "WARN", "warn_if": "!= 0", From 0baf090c3e030c80d3927120a5f7eb1a88482c2c Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Sat, 30 Sep 2023 16:17:13 -0600 Subject: [PATCH 29/37] Add `store_failures_as` config for generic tests --- core/dbt/parser/generic_test_builders.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/core/dbt/parser/generic_test_builders.py b/core/dbt/parser/generic_test_builders.py index 69c86853162..d6ff1ad7382 100644 --- a/core/dbt/parser/generic_test_builders.py +++ b/core/dbt/parser/generic_test_builders.py @@ -101,6 +101,7 @@ class TestBuilder(Generic[Testable]): "error_if", "fail_calc", "store_failures", + "store_failures_as", "meta", "database", "schema", @@ -242,6 +243,10 @@ def severity(self) -> Optional[str]: def store_failures(self) -> Optional[bool]: return self.config.get("store_failures") + @property + def store_failures_as(self) -> Optional[bool]: + return self.config.get("store_failures_as") + @property def where(self) -> Optional[str]: return self.config.get("where") @@ -294,6 +299,8 @@ def get_static_config(self): config["fail_calc"] = self.fail_calc if self.store_failures is not None: config["store_failures"] = self.store_failures + if self.store_failures_as is not None: + config["store_failures_as"] = self.store_failures_as if self.meta is not None: config["meta"] = self.meta if self.database is not None: From 1b1d9c60205233e8628a85b5cd9ec36e589015c7 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Mon, 2 Oct 2023 20:29:16 -0400 Subject: [PATCH 30/37] revert code to prioritize store_failures_as over store_failures --- core/dbt/contracts/graph/model_config.py | 19 +- .../store_test_failures_tests/_files.py | 70 +++++- .../store_test_failures_tests/basic.py | 231 ++++++++++++------ .../functional/artifacts/expected_manifest.py | 2 +- tests/functional/list/test_list.py | 6 +- .../test_store_test_failures.py | 24 +- tests/unit/test_contracts_graph_compiled.py | 1 - tests/unit/test_contracts_graph_parsed.py | 2 - 8 files changed, 270 insertions(+), 85 deletions(-) diff --git a/core/dbt/contracts/graph/model_config.py b/core/dbt/contracts/graph/model_config.py index 1072ab04344..a633c97d89a 100644 --- a/core/dbt/contracts/graph/model_config.py +++ b/core/dbt/contracts/graph/model_config.py @@ -554,13 +554,30 @@ class TestConfig(NodeAndTestConfig): # Annotated is used by mashumaro for jsonschema generation severity: Annotated[Severity, Pattern(SEVERITY_PATTERN)] = Severity("ERROR") store_failures: Optional[bool] = None - store_failures_as: Optional[str] = "table" + store_failures_as: Optional[str] = None where: Optional[str] = None limit: Optional[int] = None fail_calc: str = "count(*)" warn_if: str = "!= 0" error_if: str = "!= 0" + def __post_init__(self): + """ + The presence of a setting for `store_failures_as` overrides any existing setting for `store_failures`, + regardless of level of granularity. If `store_failures_as` is not set, then `store_failures` takes effect. + At the time of implementation, `store_failures = True` would always create a table; the user could not + configure this. Hence, if `store_failures = True` and `store_failures_as` is not specified, then it + should be set to "table" to mimic the existing functionality. + + The intention of this block is to behave as if `store_failures_as` is the only setting, + but still allow for backwards compatibility for `store_failures`. + See https://github.com/dbt-labs/dbt-core/issues/6914 for more information. + """ + if self.store_failures_as: + self.store_failures = True + elif self.store_failures: + self.store_failures_as = "table" + @classmethod def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> bool: """This is like __eq__, except it explicitly checks certain fields.""" diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py index b1174a383be..87c81761d64 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py @@ -12,33 +12,89 @@ from {{ ref('chipmunks_stage') }} """ -TEST__FAIL_AS_VIEW = """ -{{ config(store_failures_as="view") }} + +TEST__VIEW_TRUE = """ +{{ config(store_failures_as="view", store_failures=True) }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__VIEW_FALSE = """ +{{ config(store_failures_as="view", store_failures=False) }} select * from {{ ref('chipmunks') }} where shirt = 'green' """ -TEST__PASS_AS_VIEW = """ +TEST__VIEW_UNSET = """ {{ config(store_failures_as="view") }} select * from {{ ref('chipmunks') }} -where shirt = 'grape' +where shirt = 'green' """ -TEST__FAIL_AS_TABLE = """ -{{ config(store_failures_as="table") }} +TEST__TABLE_TRUE = """ +{{ config(store_failures_as="table", store_failures=True) }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__TABLE_FALSE = """ +{{ config(store_failures_as="table", store_failures=False) }} select * from {{ ref('chipmunks') }} where shirt = 'green' """ -TEST__PASS_AS_TABLE = """ +TEST__TABLE_UNSET = """ {{ config(store_failures_as="table") }} select * from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__UNSET_TRUE = """ +{{ config(store_failures=True) }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__UNSET_FALSE = """ +{{ config(store_failures=False) }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__UNSET_UNSET = """ +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__VIEW_UNSET_PASS = """ +{{ config(store_failures_as="view") }} +select * +from {{ ref('chipmunks') }} where shirt = 'purple' """ + + +TEST__NONE_FALSE = """ +{{ config(store_failures_as=None, store_failures=False) }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py index b2480bdd3e7..0995825d312 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py @@ -1,5 +1,4 @@ from collections import namedtuple -from typing import Dict import pytest @@ -7,16 +6,26 @@ from dbt.tests.util import run_dbt, check_relation_types from dbt.tests.adapter.store_test_failures_tests._files import ( - SEED__CHIPMUNKS, MODEL__CHIPMUNKS, - TEST__FAIL_AS_VIEW, - TEST__PASS_AS_VIEW, - TEST__FAIL_AS_TABLE, - TEST__PASS_AS_TABLE, + SEED__CHIPMUNKS, + TEST__NONE_FALSE, + TEST__TABLE_FALSE, + TEST__TABLE_TRUE, + TEST__TABLE_UNSET, + TEST__UNSET_FALSE, + TEST__UNSET_TRUE, + TEST__UNSET_UNSET, + TEST__VIEW_FALSE, + TEST__VIEW_TRUE, + TEST__VIEW_UNSET, + TEST__VIEW_UNSET_PASS, ) -class StoreTestFailures: +TestResult = namedtuple("TestResult", ["name", "status", "type"]) + + +class StoreTestFailuresAsBase: seed_table: str = "chipmunks_stage" model_table: str = "chipmunks" audit_schema_suffix: str = "dbt_test__audit" @@ -57,15 +66,6 @@ def seeds(self): def models(self): return {f"{self.model_table}.sql": MODEL__CHIPMUNKS} - @pytest.fixture(scope="class") - def tests(self): - return { - "fail_as_view.sql": TEST__FAIL_AS_VIEW, - "pass_as_view.sql": TEST__PASS_AS_VIEW, - "fail_as_table.sql": TEST__FAIL_AS_TABLE, - "pass_as_table.sql": TEST__PASS_AS_TABLE, - } - def row_count(self, project, relation_name: str) -> int: """ Return the row count for the relation. @@ -78,44 +78,62 @@ def row_count(self, project, relation_name: str) -> int: the row count as an integer """ sql = f"select count(*) from {self.audit_schema}.{relation_name}" - return project.run_sql(sql, fetch="one")[0] - - def insert_record(self, project, record: Dict[str, str]): - field_names, field_values = [], [] - for field_name, field_value in record.items(): - field_names.append(field_name) - field_values.append(f"'{field_value}'") - field_name_clause = ", ".join(field_names) - field_value_clause = ", ".join(field_values) - - sql = f""" - insert into {project.test_schema}.{self.model_table} ({field_name_clause}) - values ({field_value_clause}) - """ - project.run_sql(sql) + try: + return project.run_sql(sql, fetch="one")[0] + # this is the error we catch and re-raise in BaseAdapter + except BaseException: + return 0 + + +class StoreTestFailuresAsInteractions(StoreTestFailuresAsBase): + """ + These scenarios test interactions between `store_failures` and `store_failures_as` at the model level. + Granularity (e.g. setting one at the project level and another at the model level) is not considered. + + Test Scenarios: + + - If `store_failures_as = "view"` and `store_failures = True`, then store the failures in a view. + - If `store_failures_as = "view"` and `store_failures = False`, then store the failures in a view. + - If `store_failures_as = "view"` and `store_failures` is not set, then store the failures in a view. + - If `store_failures_as = "table"` and `store_failures = True`, then store the failures in a table. + - If `store_failures_as = "table"` and `store_failures = False`, then store the failures in a table. + - If `store_failures_as = "table"` and `store_failures` is not set, then store the failures in a table. + - If `store_failures_as` is not set and `store_failures = True`, then store the failures in a table. + - If `store_failures_as` is not set and `store_failures = False`, then do not store the failures. + - If `store_failures_as` is not set and `store_failures` is not set, then do not store the failures. + """ - def delete_record(self, project, record: Dict[str, str]): - where_clause = " and ".join( - [f"{field_name} = '{field_value}'" for field_name, field_value in record.items()] - ) - sql = f""" - delete from {project.test_schema}.{self.model_table} - where {where_clause} - """ - project.run_sql(sql) + @pytest.fixture(scope="class") + def tests(self): + return { + "view_unset_pass.sql": TEST__VIEW_UNSET_PASS, # control + "view_true.sql": TEST__VIEW_TRUE, + "view_false.sql": TEST__VIEW_FALSE, + "view_unset.sql": TEST__VIEW_UNSET, + "table_true.sql": TEST__TABLE_TRUE, + "table_false.sql": TEST__TABLE_FALSE, + "table_unset.sql": TEST__TABLE_UNSET, + "unset_true.sql": TEST__UNSET_TRUE, + "unset_false.sql": TEST__UNSET_FALSE, + "unset_unset.sql": TEST__UNSET_UNSET, + } def test_tests_run_successfully_and_are_stored_as_expected(self, project): - # set up the expected results - TestResult = namedtuple("TestResult", ["name", "status", "type", "row_count"]) expected_results = { - TestResult("pass_as_view", TestStatus.Pass, "view", 0), - TestResult("fail_as_view", TestStatus.Fail, "view", 1), - TestResult("pass_as_table", TestStatus.Pass, "table", 0), - TestResult("fail_as_table", TestStatus.Fail, "table", 1), + TestResult("view_unset_pass", TestStatus.Pass, "view"), # control + TestResult("view_true", TestStatus.Fail, "view"), + TestResult("view_false", TestStatus.Fail, "view"), + TestResult("view_unset", TestStatus.Fail, "view"), + TestResult("table_true", TestStatus.Fail, "table"), + TestResult("table_false", TestStatus.Fail, "table"), + TestResult("table_unset", TestStatus.Fail, "table"), + TestResult("unset_true", TestStatus.Fail, "table"), + TestResult("unset_false", TestStatus.Fail, None), + TestResult("unset_unset", TestStatus.Fail, None), } - # run the tests once - results = run_dbt(["test", "--store-failures"], expect_pass=False) + # run the tests + results = run_dbt(["test"], expect_pass=False) # show that the statuses are what we expect actual = {(result.node.name, result.status) for result in results} @@ -127,28 +145,105 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): project.adapter, {result.name: result.type for result in expected_results} ) - # show that only the failed records show up - actual = { - (result.name, self.row_count(project, result.name)) for result in expected_results + +class StoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsBase): + """ + These scenarios test that `store_failures_as` at the model level takes precedence over `store_failures` + at the project level. + + Test Scenarios: + + - If `store_failures = False` in the project and `store_failures_as = "view"` in the model, + then store the failures in a view. + - If `store_failures = False` in the project and `store_failures_as = "table"` in the model, + then store the failures in a table. + - If `store_failures = False` in the project and `store_failures_as` is not set, + then do not store the failures. + """ + + @pytest.fixture(scope="class") + def tests(self): + return { + "results_view.sql": TEST__VIEW_UNSET, + "results_table.sql": TEST__TABLE_UNSET, + "results_unset.sql": TEST__UNSET_UNSET, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return {"tests": {"store_failures": False}} + + def test_tests_run_successfully_and_are_stored_as_expected(self, project): + expected_results = { + TestResult("results_view", TestStatus.Fail, "view"), + TestResult("results_table", TestStatus.Fail, "table"), + TestResult("results_unset", TestStatus.Fail, None), } - expected = {(result.name, result.row_count) for result in expected_results} + + # run the tests + results = run_dbt(["test"], expect_pass=False) + + # show that the statuses are what we expect + actual = {(result.node.name, result.status) for result in results} + expected = {(result.name, result.status) for result in expected_results} assert actual == expected - # insert a new record in the model that fails the "pass" tests - # show that the view updates, but not the table - self.insert_record(project, {"name": "dave", "shirt": "grape"}) - expected_results.remove(TestResult("pass_as_view", TestStatus.Pass, "view", 0)) - expected_results.add(TestResult("pass_as_view", TestStatus.Pass, "view", 1)) - - # delete the original record from the model that failed the "fail" tests - # show that the view updates, but not the table - self.delete_record(project, {"name": "theodore", "shirt": "green"}) - expected_results.remove(TestResult("fail_as_view", TestStatus.Fail, "view", 1)) - expected_results.add(TestResult("fail_as_view", TestStatus.Fail, "view", 0)) - - # show that the views update without needing to run dbt, but the tables do not update - actual = { - (result.name, self.row_count(project, result.name)) for result in expected_results + # show that the results are persisted in the correct database objects + check_relation_types( + project.adapter, {result.name: result.type for result in expected_results} + ) + + +class StoreTestFailuresAsProjectLevelView(StoreTestFailuresAsBase): + """ + These scenarios test that `store_failures_as` at the project level takes precedence over `store_failures` + at the model level. + + Additionally, the fourth scenario demonstrates how to turn off `store_failures` at the model level + when `store_failures_as` is used at the project level. + + Test Scenarios: + + - If `store_failures_as = "view"` in the project and `store_failures = False` in the model, + then store the failures in a view. + - If `store_failures_as = "view"` in the project and `store_failures = True` in the model, + then store the failures in a view. + - If `store_failures_as = "view"` in the project and `store_failures` is not set, + then store the failures in a view. + - If `store_failures_as = "view"` in the project and `store_failures = False` in the model + and `store_failures_as = None` in the model, then do not store the failures. + """ + + @pytest.fixture(scope="class") + def tests(self): + return { + "results_true.sql": TEST__VIEW_TRUE, + "results_false.sql": TEST__VIEW_FALSE, + "results_unset.sql": TEST__VIEW_UNSET, + "results_turn_off.sql": TEST__NONE_FALSE, } - expected = {(result.name, result.row_count) for result in expected_results} + + @pytest.fixture(scope="class") + def project_config_update(self): + return {"tests": {"store_failures_as": "view"}} + + def test_tests_run_successfully_and_are_stored_as_expected(self, project): + expected_results = { + TestResult("results_true", TestStatus.Fail, "view"), + TestResult("results_false", TestStatus.Fail, "view"), + TestResult("results_unset", TestStatus.Fail, "view"), + TestResult("results_turn_off", TestStatus.Fail, None), + } + + # run the tests + results = run_dbt(["test"], expect_pass=False) + + # show that the statuses are what we expect + actual = {(result.node.name, result.status) for result in results} + expected = {(result.name, result.status) for result in expected_results} assert actual == expected + + # show that the results are persisted in the correct database objects + check_relation_types( + project.adapter, {result.name: result.type for result in expected_results} + ) diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index d90c945957a..0c5521fd8f5 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -132,7 +132,7 @@ def get_rendered_tst_config(**updates): "tags": [], "severity": "ERROR", "store_failures": None, - "store_failures_as": "table", + "store_failures_as": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", diff --git a/tests/functional/list/test_list.py b/tests/functional/list/test_list.py index 4962194f125..f27e30f4246 100644 --- a/tests/functional/list/test_list.py +++ b/tests/functional/list/test_list.py @@ -493,7 +493,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, - "store_failures_as": "table", + "store_failures_as": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", @@ -521,7 +521,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, - "store_failures_as": "table", + "store_failures_as": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", @@ -552,7 +552,7 @@ def expect_test_output(self): "materialized": "test", "severity": "ERROR", "store_failures": None, - "store_failures_as": "table", + "store_failures_as": None, "warn_if": "!= 0", "error_if": "!= 0", "fail_calc": "count(*)", diff --git a/tests/functional/store_test_failures/test_store_test_failures.py b/tests/functional/store_test_failures/test_store_test_failures.py index 4158adb895f..7c275024ff8 100644 --- a/tests/functional/store_test_failures/test_store_test_failures.py +++ b/tests/functional/store_test_failures/test_store_test_failures.py @@ -1,9 +1,29 @@ import pytest -from dbt.tests.adapter.store_test_failures_tests.basic import StoreTestFailures +from dbt.tests.adapter.store_test_failures_tests.basic import ( + StoreTestFailuresAsInteractions, + StoreTestFailuresAsProjectLevelOff, + StoreTestFailuresAsProjectLevelView, +) -class TestStoreTestFailures(StoreTestFailures): +class TestStoreTestFailuresAsInteractions(StoreTestFailuresAsInteractions): + @pytest.fixture(scope="function", autouse=True) + def setup_audit_schema(self, project, setup_method): + # postgres only supports schema names of 63 characters + # a schema with a longer name still gets created, but the name gets truncated + self.audit_schema = self.audit_schema[:63] + + +class TestStoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsProjectLevelOff): + @pytest.fixture(scope="function", autouse=True) + def setup_audit_schema(self, project, setup_method): + # postgres only supports schema names of 63 characters + # a schema with a longer name still gets created, but the name gets truncated + self.audit_schema = self.audit_schema[:63] + + +class TestStoreTestFailuresAsProjectLevelView(StoreTestFailuresAsProjectLevelView): @pytest.fixture(scope="function", autouse=True) def setup_audit_schema(self, project, setup_method): # postgres only supports schema names of 63 characters diff --git a/tests/unit/test_contracts_graph_compiled.py b/tests/unit/test_contracts_graph_compiled.py index dde740845e8..1b4f0bbd045 100644 --- a/tests/unit/test_contracts_graph_compiled.py +++ b/tests/unit/test_contracts_graph_compiled.py @@ -544,7 +544,6 @@ def basic_compiled_schema_test_dict(): "config": { "enabled": True, "materialized": "test", - "store_failures_as": "table", "tags": [], "severity": "warn", "schema": "dbt_test__audit", diff --git a/tests/unit/test_contracts_graph_parsed.py b/tests/unit/test_contracts_graph_parsed.py index ceeff8330ce..498ec5480e1 100644 --- a/tests/unit/test_contracts_graph_parsed.py +++ b/tests/unit/test_contracts_graph_parsed.py @@ -1038,7 +1038,6 @@ def basic_parsed_schema_test_dict(): "config": { "enabled": True, "materialized": "test", - "store_failures_as": "table", "tags": [], "severity": "ERROR", "warn_if": "!= 0", @@ -1118,7 +1117,6 @@ def complex_parsed_schema_test_dict(): "config": { "enabled": True, "materialized": "table", - "store_failures_as": "table", "tags": [], "severity": "WARN", "warn_if": "!= 0", From 6c93e34b1d5fe382ef83efbd20a0d44f31c70045 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Mon, 2 Oct 2023 20:43:16 -0400 Subject: [PATCH 31/37] cover --store-failures on CLI gap --- .../global_project/macros/materializations/tests/test.sql | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index eb287dec74d..75fb47421eb 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -7,7 +7,10 @@ {% set identifier = model['alias'] %} {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} - {% set store_failures_as = config.get('store_failures_as') %} + {% set store_failures_as = config.get('store_failures_as', 'table') %} + -- if `--store-failures` is invoked via command line and `store_failures_as` is not set, + -- config.get('store_failures_as', 'table') returns None, not 'table' + {% if store_failures_as == none %}{% set store_failures_as = 'table' %}{% endif %} {% set target_relation = api.Relation.create( identifier=identifier, schema=schema, database=database, type=store_failures_as) -%} %} From a47e59e0252c76311485c55fc033072f3efacc1c Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Tue, 3 Oct 2023 12:50:22 -0400 Subject: [PATCH 32/37] add generic tests test case for store_failures_as --- .../store_test_failures_tests/_files.py | 28 ++++++++++++++ .../store_test_failures_tests/basic.py | 38 +++++++++++++++++++ .../test_store_test_failures.py | 9 +++++ 3 files changed, 75 insertions(+) diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py index 87c81761d64..d08e9885cd1 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py @@ -3,6 +3,7 @@ alvin,red simon,blue theodore,green +dave, """.strip() @@ -98,3 +99,30 @@ from {{ ref('chipmunks') }} where shirt = 'green' """ + + +SCHEMA_YML = """ +version: 2 + +models: +- name: chipmunks + columns: + - name: str + tests: + - unique: + store_failures: true + - not_null: + store_failures_as: view + - accepted_values: + store_failures: false + store_failures_as: table + values: + - alvin + - simon + - theodore + - shirt: str + tests: + - not_null: + store_failures: true + store_failures_as: view +""" diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py index 0995825d312..aebe6549adb 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py @@ -7,6 +7,7 @@ from dbt.tests.adapter.store_test_failures_tests._files import ( MODEL__CHIPMUNKS, + SCHEMA_YML, SEED__CHIPMUNKS, TEST__NONE_FALSE, TEST__TABLE_FALSE, @@ -247,3 +248,40 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): check_relation_types( project.adapter, {result.name: result.type for result in expected_results} ) + + +class StoreTestFailuresAsGeneric(StoreTestFailuresAsBase): + """ + This tests that `store_failures_as` works with generic tests. + Test Scenarios: + + - If `store_failures_as = "view"` is used with the `not_null` test in the model, then store the failures in a view. + """ + + @pytest.fixture(scope="class") + def models(self): + return { + f"{self.model_table}.sql": MODEL__CHIPMUNKS, + "schema.yml": SCHEMA_YML, + } + + def test_tests_run_successfully_and_are_stored_as_expected(self, project): + expected_results = { + TestResult("name__unique", TestStatus.Pass, "table"), + TestResult("name__not_null", TestStatus.Pass, "view"), + TestResult("name__accepted_values", TestStatus.Fail, "table"), + TestResult("shirt__not_null", TestStatus.Fail, "view"), + } + + # run the tests + results = run_dbt(["test"], expect_pass=False) + + # show that the statuses are what we expect + actual = {(result.node.name, result.status) for result in results} + expected = {(result.name, result.status) for result in expected_results} + assert actual == expected + + # show that the results are persisted in the correct database objects + check_relation_types( + project.adapter, {result.name: result.type for result in expected_results} + ) diff --git a/tests/functional/store_test_failures/test_store_test_failures.py b/tests/functional/store_test_failures/test_store_test_failures.py index 7c275024ff8..658adde116c 100644 --- a/tests/functional/store_test_failures/test_store_test_failures.py +++ b/tests/functional/store_test_failures/test_store_test_failures.py @@ -4,6 +4,7 @@ StoreTestFailuresAsInteractions, StoreTestFailuresAsProjectLevelOff, StoreTestFailuresAsProjectLevelView, + StoreTestFailuresAsGeneric, ) @@ -29,3 +30,11 @@ def setup_audit_schema(self, project, setup_method): # postgres only supports schema names of 63 characters # a schema with a longer name still gets created, but the name gets truncated self.audit_schema = self.audit_schema[:63] + + +class TestStoreTestFailuresAsGeneric(StoreTestFailuresAsGeneric): + @pytest.fixture(scope="function", autouse=True) + def setup_audit_schema(self, project, setup_method): + # postgres only supports schema names of 63 characters + # a schema with a longer name still gets created, but the name gets truncated + self.audit_schema = self.audit_schema[:63] From c09b81dce9a2a111f77142d76593557d259826a1 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Tue, 3 Oct 2023 13:02:44 -0400 Subject: [PATCH 33/37] update object names for generic test case tests for store_failures_as --- .../store_test_failures_tests/_files.py | 34 +++++++++---------- .../store_test_failures_tests/basic.py | 10 +++--- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py index d08e9885cd1..71688ecbce3 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py @@ -105,24 +105,24 @@ version: 2 models: -- name: chipmunks + - name: chipmunks columns: - - name: str + - name: name tests: - - unique: - store_failures: true - - not_null: - store_failures_as: view - - accepted_values: - store_failures: false - store_failures_as: table - values: - - alvin - - simon - - theodore - - shirt: str + - unique: + store_failures: true + - not_null: + store_failures_as: view + - accepted_values: + store_failures: false + store_failures_as: table + values: + - alvin + - simon + - theodore + - name: shirt tests: - - not_null: - store_failures: true - store_failures_as: view + - not_null: + store_failures: true + store_failures_as: view """ diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py index aebe6549adb..bef78e7f8e5 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py @@ -267,10 +267,12 @@ def models(self): def test_tests_run_successfully_and_are_stored_as_expected(self, project): expected_results = { - TestResult("name__unique", TestStatus.Pass, "table"), - TestResult("name__not_null", TestStatus.Pass, "view"), - TestResult("name__accepted_values", TestStatus.Fail, "table"), - TestResult("shirt__not_null", TestStatus.Fail, "view"), + TestResult("unique_chipmunks_name", TestStatus.Pass, "table"), + TestResult("not_null_chipmunks_name", TestStatus.Pass, "view"), + TestResult( + "accepted_values_chipmunks_name__alvin__simon__theodore", TestStatus.Fail, "table" + ), + TestResult("not_null_chipmunks_shirt", TestStatus.Fail, "view"), } # run the tests From 729d939ecc48e6b3659f929e2c66c9b3477bf60f Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Wed, 4 Oct 2023 12:54:36 -0400 Subject: [PATCH 34/37] remove unique generic test, it was not testing `store_failures_as` --- .../dbt/tests/adapter/store_test_failures_tests/_files.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py index 71688ecbce3..38d73141271 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py @@ -109,8 +109,6 @@ columns: - name: name tests: - - unique: - store_failures: true - not_null: store_failures_as: view - accepted_values: From 9d3fb0648aeff494f99ada75852cc427e18b9161 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Wed, 4 Oct 2023 12:55:30 -0400 Subject: [PATCH 35/37] pull generic run and assertion into base test class to turn tests into quasi-parameterized tests --- .../store_test_failures_tests/basic.py | 90 ++++++------------- 1 file changed, 27 insertions(+), 63 deletions(-) diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py index bef78e7f8e5..4f84beb4960 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py @@ -1,4 +1,5 @@ from collections import namedtuple +from typing import Set import pytest @@ -67,23 +68,32 @@ def seeds(self): def models(self): return {f"{self.model_table}.sql": MODEL__CHIPMUNKS} - def row_count(self, project, relation_name: str) -> int: + def run_and_assert( + self, project, expected_results: Set[TestResult], expect_pass: bool = False + ) -> None: """ - Return the row count for the relation. + Run `dbt test` and assert the results are the expected results Args: - project: the project fixture - relation_name: the name of the relation + project: the `project` fixture; needed since we invoke `run_dbt` + expected_results: the expected results of the tests as instances of TestResult + expect_pass: passed directly into `run_dbt`; this is only needed if all expected results are tests that pass Returns: the row count as an integer """ - sql = f"select count(*) from {self.audit_schema}.{relation_name}" - try: - return project.run_sql(sql, fetch="one")[0] - # this is the error we catch and re-raise in BaseAdapter - except BaseException: - return 0 + # run the tests + results = run_dbt(["test"], expect_pass=expect_pass) + + # show that the statuses are what we expect + actual = {(result.node.name, result.status) for result in results} + expected = {(result.name, result.status) for result in expected_results} + assert actual == expected + + # show that the results are persisted in the correct database objects + check_relation_types( + project.adapter, {result.name: result.type for result in expected_results} + ) class StoreTestFailuresAsInteractions(StoreTestFailuresAsBase): @@ -132,19 +142,7 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): TestResult("unset_false", TestStatus.Fail, None), TestResult("unset_unset", TestStatus.Fail, None), } - - # run the tests - results = run_dbt(["test"], expect_pass=False) - - # show that the statuses are what we expect - actual = {(result.node.name, result.status) for result in results} - expected = {(result.name, result.status) for result in expected_results} - assert actual == expected - - # show that the results are persisted in the correct database objects - check_relation_types( - project.adapter, {result.name: result.type for result in expected_results} - ) + self.run_and_assert(project, expected_results) class StoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsBase): @@ -180,19 +178,7 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): TestResult("results_table", TestStatus.Fail, "table"), TestResult("results_unset", TestStatus.Fail, None), } - - # run the tests - results = run_dbt(["test"], expect_pass=False) - - # show that the statuses are what we expect - actual = {(result.node.name, result.status) for result in results} - expected = {(result.name, result.status) for result in expected_results} - assert actual == expected - - # show that the results are persisted in the correct database objects - check_relation_types( - project.adapter, {result.name: result.type for result in expected_results} - ) + self.run_and_assert(project, expected_results) class StoreTestFailuresAsProjectLevelView(StoreTestFailuresAsBase): @@ -235,19 +221,7 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): TestResult("results_unset", TestStatus.Fail, "view"), TestResult("results_turn_off", TestStatus.Fail, None), } - - # run the tests - results = run_dbt(["test"], expect_pass=False) - - # show that the statuses are what we expect - actual = {(result.node.name, result.status) for result in results} - expected = {(result.name, result.status) for result in expected_results} - assert actual == expected - - # show that the results are persisted in the correct database objects - check_relation_types( - project.adapter, {result.name: result.type for result in expected_results} - ) + self.run_and_assert(project, expected_results) class StoreTestFailuresAsGeneric(StoreTestFailuresAsBase): @@ -267,23 +241,13 @@ def models(self): def test_tests_run_successfully_and_are_stored_as_expected(self, project): expected_results = { - TestResult("unique_chipmunks_name", TestStatus.Pass, "table"), + # `store_failures` unset, `store_failures_as = "view"` TestResult("not_null_chipmunks_name", TestStatus.Pass, "view"), + # `store_failures = False`, `store_failures_as = "table"` TestResult( "accepted_values_chipmunks_name__alvin__simon__theodore", TestStatus.Fail, "table" ), + # `store_failures = True`, `store_failures_as = "view"` TestResult("not_null_chipmunks_shirt", TestStatus.Fail, "view"), } - - # run the tests - results = run_dbt(["test"], expect_pass=False) - - # show that the statuses are what we expect - actual = {(result.node.name, result.status) for result in results} - expected = {(result.name, result.status) for result in expected_results} - assert actual == expected - - # show that the results are persisted in the correct database objects - check_relation_types( - project.adapter, {result.name: result.type for result in expected_results} - ) + self.run_and_assert(project, expected_results) From e70dca3c33a1be2f5ce212af123b54ad35e9981c Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Mon, 9 Oct 2023 18:39:04 -0400 Subject: [PATCH 36/37] add ephemeral option for store_failures_as, as a way to easily turn off store_failures at the model level --- core/dbt/contracts/graph/model_config.py | 34 +++++- core/dbt/contracts/relation.py | 1 + .../macros/materializations/tests/test.sql | 2 +- .../store_test_failures_tests/_files.py | 32 +++-- .../store_test_failures_tests/basic.py | 115 +++++++++++------- .../test_store_test_failures.py | 39 +++--- 6 files changed, 148 insertions(+), 75 deletions(-) diff --git a/core/dbt/contracts/graph/model_config.py b/core/dbt/contracts/graph/model_config.py index a633c97d89a..4873ec83d19 100644 --- a/core/dbt/contracts/graph/model_config.py +++ b/core/dbt/contracts/graph/model_config.py @@ -569,14 +569,40 @@ def __post_init__(self): configure this. Hence, if `store_failures = True` and `store_failures_as` is not specified, then it should be set to "table" to mimic the existing functionality. + A side effect of this overriding functionality is that `store_failures_as="view"` at the project + level cannot be turned off at the model level without setting both `store_failures_as` and + `store_failures`. The former would cascade down and override `store_failures=False`. The proposal + is to include "ephemeral" as a value for `store_failures_as`, which effectively sets + `store_failures=False`. + The intention of this block is to behave as if `store_failures_as` is the only setting, but still allow for backwards compatibility for `store_failures`. See https://github.com/dbt-labs/dbt-core/issues/6914 for more information. """ - if self.store_failures_as: - self.store_failures = True - elif self.store_failures: - self.store_failures_as = "table" + + # if `store_failures_as` is set, it dictates what `store_failures` gets set to + store_failures_map = { + "ephemeral": False, + "table": True, + "view": True, + } + + # if `store_failures_as` is not set, it gets set by `store_failures` + store_failures_as_map = { + True: "table", + False: "ephemeral", + None: None, + } + + if self.store_failures_as in store_failures_map: + self.store_failures = store_failures_map[self.store_failures_as] + elif self.store_failures_as is None: + self.store_failures_as = store_failures_as_map[self.store_failures] + else: # `store_failures_as` is set to an unsupported value + raise CompilationError( + f"""{self.store_failures_as} is not a valid value for `store_failures_as`. """ + f"""Accepted values are: {str(list(store_failures_map.keys()))}""" + ) @classmethod def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> bool: diff --git a/core/dbt/contracts/relation.py b/core/dbt/contracts/relation.py index 2cf811f9f6c..52f7a07976f 100644 --- a/core/dbt/contracts/relation.py +++ b/core/dbt/contracts/relation.py @@ -19,6 +19,7 @@ class RelationType(StrEnum): CTE = "cte" MaterializedView = "materialized_view" External = "external" + Ephemeral = "ephemeral" class ComponentName(StrEnum): diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index 75fb47421eb..238faf3fd1c 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -7,7 +7,7 @@ {% set identifier = model['alias'] %} {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} - {% set store_failures_as = config.get('store_failures_as', 'table') %} + {% set store_failures_as = config.get('store_failures_as') %} -- if `--store-failures` is invoked via command line and `store_failures_as` is not set, -- config.get('store_failures_as', 'table') returns None, not 'table' {% if store_failures_as == none %}{% set store_failures_as = 'table' %}{% endif %} diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py index 38d73141271..d6302ff56c8 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py @@ -62,6 +62,30 @@ """ +TEST__EPHEMERAL_TRUE = """ +{{ config(store_failures_as="ephemeral", store_failures=True) }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__EPHEMERAL_FALSE = """ +{{ config(store_failures_as="ephemeral", store_failures=False) }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__EPHEMERAL_UNSET = """ +{{ config(store_failures_as="ephemeral") }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + TEST__UNSET_TRUE = """ {{ config(store_failures=True) }} select * @@ -93,14 +117,6 @@ """ -TEST__NONE_FALSE = """ -{{ config(store_failures_as=None, store_failures=False) }} -select * -from {{ ref('chipmunks') }} -where shirt = 'green' -""" - - SCHEMA_YML = """ version: 2 diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py index 4f84beb4960..cfdd59d3c14 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py @@ -6,22 +6,7 @@ from dbt.contracts.results import TestStatus from dbt.tests.util import run_dbt, check_relation_types -from dbt.tests.adapter.store_test_failures_tests._files import ( - MODEL__CHIPMUNKS, - SCHEMA_YML, - SEED__CHIPMUNKS, - TEST__NONE_FALSE, - TEST__TABLE_FALSE, - TEST__TABLE_TRUE, - TEST__TABLE_UNSET, - TEST__UNSET_FALSE, - TEST__UNSET_TRUE, - TEST__UNSET_UNSET, - TEST__VIEW_FALSE, - TEST__VIEW_TRUE, - TEST__VIEW_UNSET, - TEST__VIEW_UNSET_PASS, -) +from dbt.tests.adapter.store_test_failures_tests import _files TestResult = namedtuple("TestResult", ["name", "status", "type"]) @@ -62,11 +47,11 @@ def teardown_method(self, project): @pytest.fixture(scope="class") def seeds(self): - return {f"{self.seed_table}.csv": SEED__CHIPMUNKS} + return {f"{self.seed_table}.csv": _files.SEED__CHIPMUNKS} @pytest.fixture(scope="class") def models(self): - return {f"{self.model_table}.sql": MODEL__CHIPMUNKS} + return {f"{self.model_table}.sql": _files.MODEL__CHIPMUNKS} def run_and_assert( self, project, expected_results: Set[TestResult], expect_pass: bool = False @@ -109,6 +94,9 @@ class StoreTestFailuresAsInteractions(StoreTestFailuresAsBase): - If `store_failures_as = "table"` and `store_failures = True`, then store the failures in a table. - If `store_failures_as = "table"` and `store_failures = False`, then store the failures in a table. - If `store_failures_as = "table"` and `store_failures` is not set, then store the failures in a table. + - If `store_failures_as = "ephemeral"` and `store_failures = True`, then do not store the failures. + - If `store_failures_as = "ephemeral"` and `store_failures = False`, then do not store the failures. + - If `store_failures_as = "ephemeral"` and `store_failures` is not set, then do not store the failures. - If `store_failures_as` is not set and `store_failures = True`, then store the failures in a table. - If `store_failures_as` is not set and `store_failures = False`, then do not store the failures. - If `store_failures_as` is not set and `store_failures` is not set, then do not store the failures. @@ -117,16 +105,19 @@ class StoreTestFailuresAsInteractions(StoreTestFailuresAsBase): @pytest.fixture(scope="class") def tests(self): return { - "view_unset_pass.sql": TEST__VIEW_UNSET_PASS, # control - "view_true.sql": TEST__VIEW_TRUE, - "view_false.sql": TEST__VIEW_FALSE, - "view_unset.sql": TEST__VIEW_UNSET, - "table_true.sql": TEST__TABLE_TRUE, - "table_false.sql": TEST__TABLE_FALSE, - "table_unset.sql": TEST__TABLE_UNSET, - "unset_true.sql": TEST__UNSET_TRUE, - "unset_false.sql": TEST__UNSET_FALSE, - "unset_unset.sql": TEST__UNSET_UNSET, + "view_unset_pass.sql": _files.TEST__VIEW_UNSET_PASS, # control + "view_true.sql": _files.TEST__VIEW_TRUE, + "view_false.sql": _files.TEST__VIEW_FALSE, + "view_unset.sql": _files.TEST__VIEW_UNSET, + "table_true.sql": _files.TEST__TABLE_TRUE, + "table_false.sql": _files.TEST__TABLE_FALSE, + "table_unset.sql": _files.TEST__TABLE_UNSET, + "ephemeral_true.sql": _files.TEST__EPHEMERAL_TRUE, + "ephemeral_false.sql": _files.TEST__EPHEMERAL_FALSE, + "ephemeral_unset.sql": _files.TEST__EPHEMERAL_UNSET, + "unset_true.sql": _files.TEST__UNSET_TRUE, + "unset_false.sql": _files.TEST__UNSET_FALSE, + "unset_unset.sql": _files.TEST__UNSET_UNSET, } def test_tests_run_successfully_and_are_stored_as_expected(self, project): @@ -138,6 +129,9 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): TestResult("table_true", TestStatus.Fail, "table"), TestResult("table_false", TestStatus.Fail, "table"), TestResult("table_unset", TestStatus.Fail, "table"), + TestResult("ephemeral_true", TestStatus.Fail, None), + TestResult("ephemeral_false", TestStatus.Fail, None), + TestResult("ephemeral_unset", TestStatus.Fail, None), TestResult("unset_true", TestStatus.Fail, "table"), TestResult("unset_false", TestStatus.Fail, None), TestResult("unset_unset", TestStatus.Fail, None), @@ -156,6 +150,8 @@ class StoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsBase): then store the failures in a view. - If `store_failures = False` in the project and `store_failures_as = "table"` in the model, then store the failures in a table. + - If `store_failures = False` in the project and `store_failures_as = "ephemeral"` in the model, + then do not store the failures. - If `store_failures = False` in the project and `store_failures_as` is not set, then do not store the failures. """ @@ -163,9 +159,10 @@ class StoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsBase): @pytest.fixture(scope="class") def tests(self): return { - "results_view.sql": TEST__VIEW_UNSET, - "results_table.sql": TEST__TABLE_UNSET, - "results_unset.sql": TEST__UNSET_UNSET, + "results_view.sql": _files.TEST__VIEW_UNSET, + "results_table.sql": _files.TEST__TABLE_UNSET, + "results_ephemeral.sql": _files.TEST__EPHEMERAL_UNSET, + "results_unset.sql": _files.TEST__UNSET_UNSET, } @pytest.fixture(scope="class") @@ -176,6 +173,7 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): expected_results = { TestResult("results_view", TestStatus.Fail, "view"), TestResult("results_table", TestStatus.Fail, "table"), + TestResult("results_ephemeral", TestStatus.Fail, None), TestResult("results_unset", TestStatus.Fail, None), } self.run_and_assert(project, expected_results) @@ -186,9 +184,6 @@ class StoreTestFailuresAsProjectLevelView(StoreTestFailuresAsBase): These scenarios test that `store_failures_as` at the project level takes precedence over `store_failures` at the model level. - Additionally, the fourth scenario demonstrates how to turn off `store_failures` at the model level - when `store_failures_as` is used at the project level. - Test Scenarios: - If `store_failures_as = "view"` in the project and `store_failures = False` in the model, @@ -197,17 +192,14 @@ class StoreTestFailuresAsProjectLevelView(StoreTestFailuresAsBase): then store the failures in a view. - If `store_failures_as = "view"` in the project and `store_failures` is not set, then store the failures in a view. - - If `store_failures_as = "view"` in the project and `store_failures = False` in the model - and `store_failures_as = None` in the model, then do not store the failures. """ @pytest.fixture(scope="class") def tests(self): return { - "results_true.sql": TEST__VIEW_TRUE, - "results_false.sql": TEST__VIEW_FALSE, - "results_unset.sql": TEST__VIEW_UNSET, - "results_turn_off.sql": TEST__NONE_FALSE, + "results_true.sql": _files.TEST__VIEW_TRUE, + "results_false.sql": _files.TEST__VIEW_FALSE, + "results_unset.sql": _files.TEST__VIEW_UNSET, } @pytest.fixture(scope="class") @@ -219,7 +211,44 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): TestResult("results_true", TestStatus.Fail, "view"), TestResult("results_false", TestStatus.Fail, "view"), TestResult("results_unset", TestStatus.Fail, "view"), - TestResult("results_turn_off", TestStatus.Fail, None), + } + self.run_and_assert(project, expected_results) + + +class StoreTestFailuresAsProjectLevelEphemeral(StoreTestFailuresAsBase): + """ + This scenario tests that `store_failures_as` at the project level takes precedence over `store_failures` + at the model level. In particular, setting `store_failures_as = "ephemeral"` at the project level + turns off `store_failures` regardless of the setting of `store_failures` anywhere. Turning `store_failures` + back on at the model level requires `store_failures_as` to be set at the model level. + + Test Scenarios: + + - If `store_failures_as = "ephemeral"` in the project and `store_failures = True` in the project, + then do not store the failures. + - If `store_failures_as = "ephemeral"` in the project and `store_failures = True` in the project and the model, + then do not store the failures. + - If `store_failures_as = "ephemeral"` in the project and `store_failures_as = "view"` in the model, + then store the failures in a view. + """ + + @pytest.fixture(scope="class") + def tests(self): + return { + "results_unset.sql": _files.TEST__UNSET_UNSET, + "results_true.sql": _files.TEST__UNSET_TRUE, + "results_view.sql": _files.TEST__VIEW_UNSET, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return {"tests": {"store_failures_as": "ephemeral", "store_failures": True}} + + def test_tests_run_successfully_and_are_stored_as_expected(self, project): + expected_results = { + TestResult("results_unset", TestStatus.Fail, None), + TestResult("results_true", TestStatus.Fail, None), + TestResult("results_view", TestStatus.Fail, "view"), } self.run_and_assert(project, expected_results) @@ -235,8 +264,8 @@ class StoreTestFailuresAsGeneric(StoreTestFailuresAsBase): @pytest.fixture(scope="class") def models(self): return { - f"{self.model_table}.sql": MODEL__CHIPMUNKS, - "schema.yml": SCHEMA_YML, + f"{self.model_table}.sql": _files.MODEL__CHIPMUNKS, + "schema.yml": _files.SCHEMA_YML, } def test_tests_run_successfully_and_are_stored_as_expected(self, project): diff --git a/tests/functional/store_test_failures/test_store_test_failures.py b/tests/functional/store_test_failures/test_store_test_failures.py index 658adde116c..39d2e08dc9f 100644 --- a/tests/functional/store_test_failures/test_store_test_failures.py +++ b/tests/functional/store_test_failures/test_store_test_failures.py @@ -4,11 +4,14 @@ StoreTestFailuresAsInteractions, StoreTestFailuresAsProjectLevelOff, StoreTestFailuresAsProjectLevelView, + StoreTestFailuresAsProjectLevelEphemeral, StoreTestFailuresAsGeneric, ) -class TestStoreTestFailuresAsInteractions(StoreTestFailuresAsInteractions): +class PostgresMixin: + audit_schema: str + @pytest.fixture(scope="function", autouse=True) def setup_audit_schema(self, project, setup_method): # postgres only supports schema names of 63 characters @@ -16,25 +19,23 @@ def setup_audit_schema(self, project, setup_method): self.audit_schema = self.audit_schema[:63] -class TestStoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsProjectLevelOff): - @pytest.fixture(scope="function", autouse=True) - def setup_audit_schema(self, project, setup_method): - # postgres only supports schema names of 63 characters - # a schema with a longer name still gets created, but the name gets truncated - self.audit_schema = self.audit_schema[:63] +class TestStoreTestFailuresAsInteractions(StoreTestFailuresAsInteractions, PostgresMixin): + pass -class TestStoreTestFailuresAsProjectLevelView(StoreTestFailuresAsProjectLevelView): - @pytest.fixture(scope="function", autouse=True) - def setup_audit_schema(self, project, setup_method): - # postgres only supports schema names of 63 characters - # a schema with a longer name still gets created, but the name gets truncated - self.audit_schema = self.audit_schema[:63] +class TestStoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsProjectLevelOff, PostgresMixin): + pass -class TestStoreTestFailuresAsGeneric(StoreTestFailuresAsGeneric): - @pytest.fixture(scope="function", autouse=True) - def setup_audit_schema(self, project, setup_method): - # postgres only supports schema names of 63 characters - # a schema with a longer name still gets created, but the name gets truncated - self.audit_schema = self.audit_schema[:63] +class TestStoreTestFailuresAsProjectLevelView(StoreTestFailuresAsProjectLevelView, PostgresMixin): + pass + + +class TestStoreTestFailuresAsProjectLevelEphemeral( + StoreTestFailuresAsProjectLevelEphemeral, PostgresMixin +): + pass + + +class TestStoreTestFailuresAsGeneric(StoreTestFailuresAsGeneric, PostgresMixin): + pass From 8b8f9bbf04d297a4da38c9716bf4686a43fd01b1 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Mon, 9 Oct 2023 19:38:37 -0400 Subject: [PATCH 37/37] add compilation error for invalid setting of store_failures_as --- core/dbt/contracts/graph/model_config.py | 39 +++++++++++-------- .../macros/materializations/tests/test.sql | 7 ++++ .../store_test_failures_tests/_files.py | 8 ++++ .../store_test_failures_tests/basic.py | 23 +++++++++++ .../test_store_test_failures.py | 5 +++ 5 files changed, 65 insertions(+), 17 deletions(-) diff --git a/core/dbt/contracts/graph/model_config.py b/core/dbt/contracts/graph/model_config.py index 4873ec83d19..aea8eb117c9 100644 --- a/core/dbt/contracts/graph/model_config.py +++ b/core/dbt/contracts/graph/model_config.py @@ -575,34 +575,39 @@ def __post_init__(self): is to include "ephemeral" as a value for `store_failures_as`, which effectively sets `store_failures=False`. + The exception handling for this is tricky. If we raise an exception here, the entire run fails at + parse time. We would rather well-formed models run successfully, leaving only exceptions to be rerun + if necessary. Hence, the exception needs to be raised in the test materialization. In order to do so, + we need to make sure that we go down the `store_failures = True` route with the invalid setting for + `store_failures_as`. This results in the `.get()` defaulted to `True` below, instead of a normal + dictionary lookup as is done in the `if` block. Refer to the test materialization for the + exception that is raise as a result of an invalid value. + The intention of this block is to behave as if `store_failures_as` is the only setting, but still allow for backwards compatibility for `store_failures`. See https://github.com/dbt-labs/dbt-core/issues/6914 for more information. """ - # if `store_failures_as` is set, it dictates what `store_failures` gets set to - store_failures_map = { - "ephemeral": False, - "table": True, - "view": True, - } - # if `store_failures_as` is not set, it gets set by `store_failures` - store_failures_as_map = { + # the settings below mimic existing behavior prior to `store_failures_as` + get_store_failures_as_map = { True: "table", False: "ephemeral", None: None, } - if self.store_failures_as in store_failures_map: - self.store_failures = store_failures_map[self.store_failures_as] - elif self.store_failures_as is None: - self.store_failures_as = store_failures_as_map[self.store_failures] - else: # `store_failures_as` is set to an unsupported value - raise CompilationError( - f"""{self.store_failures_as} is not a valid value for `store_failures_as`. """ - f"""Accepted values are: {str(list(store_failures_map.keys()))}""" - ) + # if `store_failures_as` is set, it dictates what `store_failures` gets set to + # the settings below overrides whatever `store_failures` is set to by the user + get_store_failures_map = { + "ephemeral": False, + "table": True, + "view": True, + } + + if self.store_failures_as is None: + self.store_failures_as = get_store_failures_as_map[self.store_failures] + else: + self.store_failures = get_store_failures_map.get(self.store_failures_as, True) @classmethod def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> bool: diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index 238faf3fd1c..ba205a9b295 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -11,6 +11,13 @@ -- if `--store-failures` is invoked via command line and `store_failures_as` is not set, -- config.get('store_failures_as', 'table') returns None, not 'table' {% if store_failures_as == none %}{% set store_failures_as = 'table' %}{% endif %} + {% if store_failures_as not in ['table', 'view'] %} + {{ exceptions.raise_compiler_error( + "'" ~ store_failures_as ~ "' is not a valid value for `store_failures_as`. " + "Accepted values are: ['ephemeral', 'table', 'view']" + ) }} + {% endif %} + {% set target_relation = api.Relation.create( identifier=identifier, schema=schema, database=database, type=store_failures_as) -%} %} diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py index d6302ff56c8..b3e296e730a 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py @@ -117,6 +117,14 @@ """ +TEST__ERROR_UNSET = """ +{{ config(store_failures_as="error") }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + SCHEMA_YML = """ version: 2 diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py index cfdd59d3c14..e8beb0f1fde 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py @@ -280,3 +280,26 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): TestResult("not_null_chipmunks_shirt", TestStatus.Fail, "view"), } self.run_and_assert(project, expected_results) + + +class StoreTestFailuresAsExceptions(StoreTestFailuresAsBase): + """ + This tests that `store_failures_as` raises exceptions in appropriate scenarios. + Test Scenarios: + + - If `store_failures_as = "error"`, a helpful exception is raised. + """ + + @pytest.fixture(scope="class") + def tests(self): + return { + "store_failures_as_error.sql": _files.TEST__ERROR_UNSET, + } + + def test_tests_run_unsuccessfully_and_raise_appropriate_exception(self, project): + results = run_dbt(["test"], expect_pass=False) + assert len(results) == 1 + result = results[0] + assert "Compilation Error" in result.message + assert "'error' is not a valid value" in result.message + assert "Accepted values are: ['ephemeral', 'table', 'view']" in result.message diff --git a/tests/functional/store_test_failures/test_store_test_failures.py b/tests/functional/store_test_failures/test_store_test_failures.py index 39d2e08dc9f..8783e1903e3 100644 --- a/tests/functional/store_test_failures/test_store_test_failures.py +++ b/tests/functional/store_test_failures/test_store_test_failures.py @@ -6,6 +6,7 @@ StoreTestFailuresAsProjectLevelView, StoreTestFailuresAsProjectLevelEphemeral, StoreTestFailuresAsGeneric, + StoreTestFailuresAsExceptions, ) @@ -39,3 +40,7 @@ class TestStoreTestFailuresAsProjectLevelEphemeral( class TestStoreTestFailuresAsGeneric(StoreTestFailuresAsGeneric, PostgresMixin): pass + + +class TestStoreTestFailuresAsExceptions(StoreTestFailuresAsExceptions, PostgresMixin): + pass