From fe948d6805f1192c7e67b84c9c8e535c4e9abfb6 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Fri, 8 Feb 2019 08:00:09 -0700 Subject: [PATCH 1/3] Add flag for raising errors on warnings fix strict mode fix some strict mode asserts add internal WARN_ERROR support and a warn_on_error function make everything that says "warning" go through warn_or_error add --warn-error flag to main.py remove sql_where from tests replace drop-existing with full-refresh in tests (re-)add integration tests for malformed schema/source tests and strict mode --- core/dbt/adapters/base/connections.py | 2 +- core/dbt/adapters/sql/connections.py | 1 + core/dbt/compilation.py | 5 +- core/dbt/config/project.py | 3 +- core/dbt/deprecations.py | 5 +- core/dbt/exceptions.py | 10 +++ core/dbt/flags.py | 4 +- core/dbt/graph/selector.py | 5 +- core/dbt/main.py | 16 ++++- core/dbt/parser/schemas.py | 5 +- core/dbt/utils.py | 5 +- .../models/advanced_incremental.sql | 1 - .../models/incremental_deprecated.sql | 8 --- .../001_simple_copy_test/test_simple_copy.py | 67 ++++++++++--------- .../models/incremental.sql | 5 +- .../models/incremental_copy.sql | 9 ++- .../005_simple_seed_test/test_simple_seed.py | 2 +- .../test_schema_v2_tests.py | 9 ++- .../models/incremental.sql | 9 ++- .../models-incremental/model_1.sql | 6 +- .../models/model.sql | 7 +- .../malformed_models/descendant_model.sql | 1 + .../malformed_models/schema.yml | 12 ++++ .../042_sources_test/test_sources.py | 25 +++++++ test/unit/test_config.py | 6 +- 25 files changed, 154 insertions(+), 74 deletions(-) delete mode 100644 test/integration/001_simple_copy_test/models/incremental_deprecated.sql create mode 100644 test/integration/042_sources_test/malformed_models/descendant_model.sql create mode 100644 test/integration/042_sources_test/malformed_models/schema.yml diff --git a/core/dbt/adapters/base/connections.py b/core/dbt/adapters/base/connections.py index 60036a9d93c..c65e932454e 100644 --- a/core/dbt/adapters/base/connections.py +++ b/core/dbt/adapters/base/connections.py @@ -296,7 +296,7 @@ def _rollback(self, connection): operation does not require the lock. """ if dbt.flags.STRICT_MODE: - Connection(**connection) + assert isinstance(connection, Connection) if connection.transaction_open is False: raise dbt.exceptions.InternalException( diff --git a/core/dbt/adapters/sql/connections.py b/core/dbt/adapters/sql/connections.py index e2a444bd8ba..a0c7bedf2ed 100644 --- a/core/dbt/adapters/sql/connections.py +++ b/core/dbt/adapters/sql/connections.py @@ -3,6 +3,7 @@ import dbt.clients.agate_helper import dbt.exceptions +from dbt.contracts.connection import Connection from dbt.adapters.base import BaseConnectionManager from dbt.compat import abstractclassmethod from dbt.logger import GLOBAL_LOGGER as logger diff --git a/core/dbt/compilation.py b/core/dbt/compilation.py index 3f46cb5f54b..4f2d583feae 100644 --- a/core/dbt/compilation.py +++ b/core/dbt/compilation.py @@ -71,8 +71,8 @@ def recursively_prepend_ctes(model, manifest): return (model, model.extra_ctes, manifest) if dbt.flags.STRICT_MODE: - # ensure that all the nodes in this manifest are compiled - CompiledGraph(**manifest.to_flat_graph()) + # ensure that the cte we're adding to is compiled + CompiledNode(**model.serialize()) prepended_ctes = [] @@ -81,7 +81,6 @@ def recursively_prepend_ctes(model, manifest): cte_to_add = manifest.nodes.get(cte_id) cte_to_add, new_prepended_ctes, manifest = recursively_prepend_ctes( cte_to_add, manifest) - _extend_prepended_ctes(prepended_ctes, new_prepended_ctes) new_cte_name = '__dbt__CTE__{}'.format(cte_to_add.get('name')) sql = ' {} as (\n{}\n)'.format(new_cte_name, cte_to_add.compiled_sql) diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 8366755ef89..444d29571a2 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -13,6 +13,7 @@ from dbt.exceptions import RecursionException from dbt.exceptions import SemverException from dbt.exceptions import ValidationException +from dbt.exceptions import warn_or_error from dbt.logger import GLOBAL_LOGGER as logger from dbt.semver import VersionSpecifier from dbt.semver import versions_compatible @@ -417,7 +418,7 @@ def warn_for_unused_resource_config_paths(self, resource_fqns, disabled): len(unused), '\n'.join('- {}'.format('.'.join(u)) for u in unused) ) - logger.info(printer.yellow(msg)) + warn_or_error(msg, log_fmt=printer.yellow('{}')) def validate_version(self): """Ensure this package works with the installed version of dbt.""" diff --git a/core/dbt/deprecations.py b/core/dbt/deprecations.py index 0bc580150a7..25e27d3ab8a 100644 --- a/core/dbt/deprecations.py +++ b/core/dbt/deprecations.py @@ -1,5 +1,6 @@ from dbt.logger import GLOBAL_LOGGER as logger import dbt.links +import dbt.flags class DBTDeprecation(object): @@ -9,7 +10,9 @@ class DBTDeprecation(object): def show(self, *args, **kwargs): if self.name not in active_deprecations: desc = self.description.format(**kwargs) - logger.info("* Deprecation Warning: {}\n".format(desc)) + dbt.exceptions.warn_or_error( + "* Deprecation Warning: {}\n".format(desc) + ) active_deprecations.add(self.name) diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index 8d0473acccc..c1180924426 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -1,5 +1,6 @@ from dbt.compat import basestring, builtins from dbt.logger import GLOBAL_LOGGER as logger +import dbt.flags import re @@ -580,6 +581,15 @@ def raise_not_implemented(msg): raise NotImplementedException(msg) +def warn_or_error(msg, node=None, log_fmt=None): + if dbt.flags.WARN_ERROR: + raise_compiler_error(msg, node) + else: + if log_fmt is not None: + msg = log_fmt.format(msg) + logger.warning(msg) + + # Update this when a new function should be added to the # dbt context's `exceptions` key! CONTEXT_EXPORTS = { diff --git a/core/dbt/flags.py b/core/dbt/flags.py index 9c8160c70dd..8bf43049a49 100644 --- a/core/dbt/flags.py +++ b/core/dbt/flags.py @@ -2,12 +2,14 @@ NON_DESTRUCTIVE = False FULL_REFRESH = False USE_CACHE = True +WARN_ERROR = False def reset(): - global STRICT_MODE, NON_DESTRUCTIVE, FULL_REFRESH + global STRICT_MODE, NON_DESTRUCTIVE, FULL_REFRESH, USE_CACHE, WARN_ERROR STRICT_MODE = False NON_DESTRUCTIVE = False FULL_REFRESH = False USE_CACHE = True + WARN_ERROR = False diff --git a/core/dbt/graph/selector.py b/core/dbt/graph/selector.py index 3b5659d6b30..4302955ff5f 100644 --- a/core/dbt/graph/selector.py +++ b/core/dbt/graph/selector.py @@ -125,10 +125,11 @@ def warn_if_useless_spec(spec, nodes): if len(nodes) > 0: return - logger.info( - "* Spec='{}' does not identify any models and was ignored\n" + msg = ( + "* Spec='{}' does not identify any models" .format(spec['raw']) ) + dbt.exceptions.warn_or_error(msg, log_fmt='{} and was ignored\n') class NodeSelector(object): diff --git a/core/dbt/main.py b/core/dbt/main.py index 011f2ff4c8a..ae235852844 100644 --- a/core/dbt/main.py +++ b/core/dbt/main.py @@ -284,6 +284,11 @@ def invoke_dbt(parsed): arg_drop_existing = getattr(parsed, 'drop_existing', False) arg_full_refresh = getattr(parsed, 'full_refresh', False) + flags.STRICT_MODE = getattr(parsed, 'strict', False) + flags.WARN_ERROR = ( + flags.STRICT_MODE or + getattr(parsed, 'warn_error', False) + ) if arg_drop_existing: dbt.deprecations.warn('drop-existing') @@ -338,7 +343,16 @@ def parse_args(args): '--strict', action='store_true', help='''Run schema validations at runtime. This will surface - bugs in dbt, but may incur a performance penalty.''') + bugs in dbt, but may incur a performance penalty. This flag implies + --warn-error''') + + p.add_argument( + '--warn-error', + action='store_true', + help='''If dbt would normally warn, instead raise an exception. + Examples include --models that selects nothing, deprecations, + configurations with no associated models, invalid test configurations, + and missing sources/refs in tests''') # if set, run dbt in single-threaded mode: thread count is ignored, and # calls go through `map` instead of the thread pool. This is useful for diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 4ad5897617b..d1fc2653334 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -164,9 +164,8 @@ def warn_invalid(filepath, key, value, explain): msg = ( "Invalid test config given in {} @ {}: {} {}" ).format(filepath, key, value, explain) - if dbt.flags.STRICT_MODE: - dbt.exceptions.raise_compiler_error(msg, value) - dbt.utils.compiler_warning(value, msg) + dbt.exceptions.warn_or_error(msg, value, + log_fmt='Compilation warning: {}\n') def _filter_validate(filepath, location, values, validate): diff --git a/core/dbt/utils.py b/core/dbt/utils.py index cf0b35fa463..a1cebce46d5 100644 --- a/core/dbt/utils.py +++ b/core/dbt/utils.py @@ -11,7 +11,6 @@ import os import dbt.exceptions -import dbt.flags from dbt.include.global_project import PACKAGES from dbt.compat import basestring, DECIMALS @@ -416,7 +415,7 @@ def invalid_ref_fail_unless_test(node, target_model_name, if disabled: logger.debug(msg) else: - logger.warning(msg) + dbt.exceptions.warn_or_error(msg) else: dbt.exceptions.ref_target_not_found( @@ -429,7 +428,7 @@ def invalid_source_fail_unless_test(node, target_name, target_table_name): if node.get('resource_type') == NodeType.Test: msg = dbt.exceptions.source_disabled_message(node, target_name, target_table_name) - logger.warning('WARNING: {}'.format(msg)) + dbt.exceptions.warn_or_error(msg, log_fmt='WARNING: {}') else: dbt.exceptions.source_target_not_found(node, target_name, target_table_name) diff --git a/test/integration/001_simple_copy_test/models/advanced_incremental.sql b/test/integration/001_simple_copy_test/models/advanced_incremental.sql index 0072463c1a0..05b0e6d1bae 100644 --- a/test/integration/001_simple_copy_test/models/advanced_incremental.sql +++ b/test/integration/001_simple_copy_test/models/advanced_incremental.sql @@ -1,7 +1,6 @@ {{ config( materialized = "incremental", - sql_where = "TRUE", unique_key = "id" ) }} diff --git a/test/integration/001_simple_copy_test/models/incremental_deprecated.sql b/test/integration/001_simple_copy_test/models/incremental_deprecated.sql deleted file mode 100644 index 72e0c872e6b..00000000000 --- a/test/integration/001_simple_copy_test/models/incremental_deprecated.sql +++ /dev/null @@ -1,8 +0,0 @@ -{{ - config( - materialized = "incremental", - sql_where = "id>(select max(id) from {{this}})" - ) -}} - -select * from {{ ref('seed') }} diff --git a/test/integration/001_simple_copy_test/test_simple_copy.py b/test/integration/001_simple_copy_test/test_simple_copy.py index 4b172400f8d..d11bd605abe 100644 --- a/test/integration/001_simple_copy_test/test_simple_copy.py +++ b/test/integration/001_simple_copy_test/test_simple_copy.py @@ -32,17 +32,17 @@ def test__postgres__simple_copy(self): self.assertEqual({'adapter:already_exists', 'sql_where'}, deprecations.active_deprecations) results = self.run_dbt() - self.assertEqual(len(results), 7) + self.assertEqual(len(results), 6) - self.assertManyTablesEqual(["seed", "view_model", "incremental", "incremental_deprecated", "materialized"]) + self.assertManyTablesEqual(["seed", "view_model", "incremental", "materialized"]) self.use_default_project({"data-paths": [self.dir("seed-update")]}) results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 7) + self.assertEqual(len(results), 6) - self.assertManyTablesEqual(["seed", "view_model", "incremental", "incremental_deprecated", "materialized"]) + self.assertManyTablesEqual(["seed", "view_model", "incremental", "materialized"]) @use_profile("postgres") def test__postgres__dbt_doesnt_run_empty_models(self): @@ -51,7 +51,7 @@ def test__postgres__dbt_doesnt_run_empty_models(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 7) + self.assertEqual(len(results), 6) models = self.get_models_in_schema() @@ -65,7 +65,7 @@ def test__presto__simple_copy(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt(expect_pass=False) - self.assertEqual(len(results), 7) + self.assertEqual(len(results), 6) for result in results: if 'incremental' in result.node.name: self.assertIn('not implemented for presto', result.error) @@ -76,16 +76,20 @@ def test__presto__simple_copy(self): def test__snowflake__simple_copy(self): self.use_default_project({"data-paths": [self.dir("seed-initial")]}) - self.run_dbt(["seed"]) - self.run_dbt() + results = self.run_dbt(["seed"]) + self.assertEqual(len(results), 1) + results = self.run_dbt() + self.assertEqual(len(results), 6) - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) self.use_default_project({"data-paths": [self.dir("seed-update")]}) - self.run_dbt(["seed"]) - self.run_dbt() + results = self.run_dbt(["seed"]) + self.assertEqual(len(results), 1) + results = self.run_dbt() + self.assertEqual(len(results), 6) - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) @use_profile("snowflake") def test__snowflake__simple_copy__quoting_off(self): @@ -97,9 +101,9 @@ def test__snowflake__simple_copy__quoting_off(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 7) + self.assertEqual(len(results), 6) - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) self.use_default_project({ "data-paths": [self.dir("seed-update")], @@ -108,9 +112,9 @@ def test__snowflake__simple_copy__quoting_off(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 7) + self.assertEqual(len(results), 6) - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) @use_profile("snowflake") def test__snowflake__seed__quoting_switch(self): @@ -135,10 +139,9 @@ def test__bigquery__simple_copy(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 7) + self.assertEqual(len(results), 6) self.assertTablesEqual("seed","view_model") - self.assertTablesEqual("seed","incremental_deprecated") self.assertTablesEqual("seed","incremental") self.assertTablesEqual("seed","materialized") @@ -147,10 +150,9 @@ def test__bigquery__simple_copy(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 7) + self.assertEqual(len(results), 6) self.assertTablesEqual("seed","view_model") - self.assertTablesEqual("seed","incremental_deprecated") self.assertTablesEqual("seed","incremental") self.assertTablesEqual("seed","materialized") @@ -173,9 +175,9 @@ def test__snowflake__simple_copy__quoting_on(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 7) + self.assertEqual(len(results), 6) - self.assertManyTablesEqual(["seed", "view_model", "incremental", "incremental_deprecated", "materialized"]) + self.assertManyTablesEqual(["seed", "view_model", "incremental", "materialized"]) self.use_default_project({ "data-paths": [self.dir("seed-update")], @@ -183,9 +185,9 @@ def test__snowflake__simple_copy__quoting_on(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 7) + self.assertEqual(len(results), 6) - self.assertManyTablesEqual(["seed", "view_model", "incremental", "incremental_deprecated", "materialized"]) + self.assertManyTablesEqual(["seed", "view_model", "incremental", "materialized"]) class BaseLowercasedSchemaTest(BaseTestSimpleCopy): @@ -200,16 +202,21 @@ class TestSnowflakeSimpleLowercasedSchemaCopy(BaseLowercasedSchemaTest): def test__snowflake__simple_copy(self): self.use_default_project({"data-paths": [self.dir("seed-initial")]}) - self.run_dbt(["seed"]) - self.run_dbt() + results = self.run_dbt(["seed"]) + self.assertEqual(len(results), 1) + results = self.run_dbt() + self.assertEqual(len(results), 6) - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) self.use_default_project({"data-paths": [self.dir("seed-update")]}) - self.run_dbt(["seed"]) - self.run_dbt() - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) + results = self.run_dbt(["seed"]) + self.assertEqual(len(results), 1) + results = self.run_dbt() + self.assertEqual(len(results), 6) + + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) class TestSnowflakeSimpleLowercasedSchemaQuoted(BaseLowercasedSchemaTest): diff --git a/test/integration/002_varchar_widening_test/models/incremental.sql b/test/integration/002_varchar_widening_test/models/incremental.sql index 10141cd3f13..96212205ab0 100644 --- a/test/integration/002_varchar_widening_test/models/incremental.sql +++ b/test/integration/002_varchar_widening_test/models/incremental.sql @@ -1,13 +1,12 @@ {{ config( - materialized = "incremental", - sql_where = "id>(select max(id) from {{this}})" + materialized = "incremental" ) }} select * from {{ this.schema }}.seed -{% if adapter.already_exists(this.schema, this.table) %} +{% if is_incremental() %} where id > (select max(id) from {{this}}) diff --git a/test/integration/003_simple_reference_test/models/incremental_copy.sql b/test/integration/003_simple_reference_test/models/incremental_copy.sql index d7e63d2a5d1..96212205ab0 100644 --- a/test/integration/003_simple_reference_test/models/incremental_copy.sql +++ b/test/integration/003_simple_reference_test/models/incremental_copy.sql @@ -1,8 +1,13 @@ {{ config( - materialized = "incremental", - sql_where = "id>(select max(id) from {{this}})" + materialized = "incremental" ) }} select * from {{ this.schema }}.seed + +{% if is_incremental() %} + + where id > (select max(id) from {{this}}) + +{% endif %} diff --git a/test/integration/005_simple_seed_test/test_simple_seed.py b/test/integration/005_simple_seed_test/test_simple_seed.py index c6a0b482d26..33701582fbc 100644 --- a/test/integration/005_simple_seed_test/test_simple_seed.py +++ b/test/integration/005_simple_seed_test/test_simple_seed.py @@ -43,7 +43,7 @@ def test_simple_seed_with_drop(self): self.assertTablesEqual("seed_actual","seed_expected") # this should drop the seed table, then re-create - results = self.run_dbt(["seed", "--drop-existing"]) + results = self.run_dbt(["seed", "--full-refresh"]) self.assertEqual(len(results), 1) self.assertTablesEqual("seed_actual","seed_expected") diff --git a/test/integration/008_schema_tests_test/test_schema_v2_tests.py b/test/integration/008_schema_tests_test/test_schema_v2_tests.py index ce91edb6c83..6b8c48ea1b8 100644 --- a/test/integration/008_schema_tests_test/test_schema_v2_tests.py +++ b/test/integration/008_schema_tests_test/test_schema_v2_tests.py @@ -88,11 +88,10 @@ def test_malformed_schema_test_wont_brick_run(self): self.assertEqual(len(ran_tests), 5) self.assertEqual(sum(x.status for x in ran_tests), 0) - # TODO: re-enable this test when we make --strict actually set strict mode - # @attr(type='postgres') - # def test_malformed_schema_strict_will_break_run(self): - # with self.assertRaises(CompilationException): - # self.run_dbt(strict=True) + @attr(type='postgres') + def test_malformed_schema_strict_will_break_run(self): + with self.assertRaises(CompilationException): + self.run_dbt(strict=True) class TestCustomSchemaTests(DBTIntegrationTest): diff --git a/test/integration/017_runtime_materialization_tests/models/incremental.sql b/test/integration/017_runtime_materialization_tests/models/incremental.sql index d7e63d2a5d1..96212205ab0 100644 --- a/test/integration/017_runtime_materialization_tests/models/incremental.sql +++ b/test/integration/017_runtime_materialization_tests/models/incremental.sql @@ -1,8 +1,13 @@ {{ config( - materialized = "incremental", - sql_where = "id>(select max(id) from {{this}})" + materialized = "incremental" ) }} select * from {{ this.schema }}.seed + +{% if is_incremental() %} + + where id > (select max(id) from {{this}}) + +{% endif %} diff --git a/test/integration/032_concurrent_transaction_test/models-incremental/model_1.sql b/test/integration/032_concurrent_transaction_test/models-incremental/model_1.sql index c244d8e33aa..3d8ac43bae5 100644 --- a/test/integration/032_concurrent_transaction_test/models-incremental/model_1.sql +++ b/test/integration/032_concurrent_transaction_test/models-incremental/model_1.sql @@ -1,5 +1,9 @@ -{{ config(materialized='incremental', sql_where=True, unique_key='id') }} +{{ config(materialized='incremental', unique_key='id') }} -- incremental model select 1 as id + +{% if is_incremental() %} + where TRUE +{% endif %} diff --git a/test/integration/035_changing_relation_type_test/models/model.sql b/test/integration/035_changing_relation_type_test/models/model.sql index a2363779a2d..0e0243126d5 100644 --- a/test/integration/035_changing_relation_type_test/models/model.sql +++ b/test/integration/035_changing_relation_type_test/models/model.sql @@ -1,5 +1,8 @@ - -{{ config(materialized=var('materialized'), sql_where='TRUE') }} +{{ config(materialized=var('materialized')) }} select '{{ var("materialized") }}' as materialization + +{% if is_incremental() %} + where TRUE +{% endif %} diff --git a/test/integration/042_sources_test/malformed_models/descendant_model.sql b/test/integration/042_sources_test/malformed_models/descendant_model.sql new file mode 100644 index 00000000000..55bbcba67b4 --- /dev/null +++ b/test/integration/042_sources_test/malformed_models/descendant_model.sql @@ -0,0 +1 @@ +select * from {{ source('test_source', 'test_table') }} diff --git a/test/integration/042_sources_test/malformed_models/schema.yml b/test/integration/042_sources_test/malformed_models/schema.yml new file mode 100644 index 00000000000..a7cb1df8db1 --- /dev/null +++ b/test/integration/042_sources_test/malformed_models/schema.yml @@ -0,0 +1,12 @@ +version: 2 +sources: + - name: test_source + loader: custom + tables: + - name: test_table + tests: + - relationships: + # this is invalid + - column_name: favorite_color + - to: ref('descendant_model') + - field: favorite_color diff --git a/test/integration/042_sources_test/test_sources.py b/test/integration/042_sources_test/test_sources.py index 761e3db6bc1..ec10c6496e8 100644 --- a/test/integration/042_sources_test/test_sources.py +++ b/test/integration/042_sources_test/test_sources.py @@ -1,5 +1,6 @@ from nose.plugins.attrib import attr from test.integration.base import DBTIntegrationTest, use_profile +from dbt.exceptions import CompilationException import os class TestSources(DBTIntegrationTest): @@ -86,3 +87,27 @@ def test_source_only_def(self): ['source', 'descendant_model'], ['expected_multi_source', 'multi_source_model']) self.assertTableDoesNotExist('nonsource_descendant') + + +class TestMalformedSources(DBTIntegrationTest): + @property + def schema(self): + return "sources_042" + + @property + def models(self): + return "test/integration/042_sources_test/malformed_models" + + def setUp(self): + super(TestMalformedSources, self).setUp() + self.run_sql_file('test/integration/042_sources_test/source.sql', + kwargs={'schema':self.unique_schema()}) + + @use_profile('postgres') + def test_malformed_schema_strict_will_break_run(self): + self.run_dbt(strict=False) + + @use_profile('postgres') + def test_malformed_schema_strict_will_break_run(self): + with self.assertRaises(CompilationException): + self.run_dbt(strict=True) diff --git a/test/unit/test_config.py b/test/unit/test_config.py index 8c9ac491337..1163468a535 100644 --- a/test/unit/test_config.py +++ b/test/unit/test_config.py @@ -884,13 +884,13 @@ def test__get_unused_resource_config_paths(self): self.assertEqual(len(unused), 1) self.assertEqual(unused[0], ('models', 'my_test_project', 'baz')) - @mock.patch.object(dbt.config.project, 'logger') - def test__warn_for_unused_resource_config_paths(self, mock_logger): + @mock.patch.object(dbt.config.project, 'warn_or_error') + def test__warn_for_unused_resource_config_paths(self, warn_or_error): project = dbt.config.Project.from_project_config( self.default_project_data ) unused = project.warn_for_unused_resource_config_paths(self.used, []) - mock_logger.info.assert_called_once() + warn_or_error.assert_called_once() @mock.patch.object(dbt.config.project, 'logger') def test__warn_for_unused_resource_config_paths_disabled(self, mock_logger): From 9b9319cbd03671c00d2e7ce4bf12b2dd0aadafca Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Thu, 7 Feb 2019 13:18:03 -0700 Subject: [PATCH 2/3] fix bigquery Remove unnecessary is_incremental() from test Fix strict mode type check --- plugins/bigquery/dbt/adapters/bigquery/impl.py | 2 +- .../035_changing_relation_type_test/models/model.sql | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/plugins/bigquery/dbt/adapters/bigquery/impl.py b/plugins/bigquery/dbt/adapters/bigquery/impl.py index dab04bff37a..f42f5830308 100644 --- a/plugins/bigquery/dbt/adapters/bigquery/impl.py +++ b/plugins/bigquery/dbt/adapters/bigquery/impl.py @@ -333,7 +333,7 @@ def execute_model(self, model, materialization, sql_override=None, if flags.STRICT_MODE: connection = self.connections.get(model.get('name')) - Connection(**connection) + assert isinstance(connection, Connection) if materialization == 'view': res = self._materialize_as_view(model) diff --git a/test/integration/035_changing_relation_type_test/models/model.sql b/test/integration/035_changing_relation_type_test/models/model.sql index 0e0243126d5..05b4efd430d 100644 --- a/test/integration/035_changing_relation_type_test/models/model.sql +++ b/test/integration/035_changing_relation_type_test/models/model.sql @@ -2,7 +2,3 @@ {{ config(materialized=var('materialized')) }} select '{{ var("materialized") }}' as materialization - -{% if is_incremental() %} - where TRUE -{% endif %} From 90497b1e4712c0f2c9009691e27bea43e750cef9 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Fri, 8 Feb 2019 08:20:57 -0700 Subject: [PATCH 3/3] give deprecation its own integration test --- .../models/advanced_incremental.sql | 2 +- .../001_simple_copy_test/test_simple_copy.py | 8 ----- .../models/already_exists.sql | 5 +++ .../models/sql_where.sql | 3 ++ .../test_deprecations.py | 34 +++++++++++++++++++ 5 files changed, 43 insertions(+), 9 deletions(-) create mode 100644 test/integration/012_deprecation_tests/models/already_exists.sql create mode 100644 test/integration/012_deprecation_tests/models/sql_where.sql create mode 100644 test/integration/012_deprecation_tests/test_deprecations.py diff --git a/test/integration/001_simple_copy_test/models/advanced_incremental.sql b/test/integration/001_simple_copy_test/models/advanced_incremental.sql index 05b0e6d1bae..0091644fcb8 100644 --- a/test/integration/001_simple_copy_test/models/advanced_incremental.sql +++ b/test/integration/001_simple_copy_test/models/advanced_incremental.sql @@ -9,7 +9,7 @@ select * from {{ ref('seed') }} -{% if adapter.already_exists(this.schema, this.table) and not flags.FULL_REFRESH %} +{% if is_incremental() %} where id > (select max(id) from {{this}}) diff --git a/test/integration/001_simple_copy_test/test_simple_copy.py b/test/integration/001_simple_copy_test/test_simple_copy.py index d11bd605abe..bbe9349e439 100644 --- a/test/integration/001_simple_copy_test/test_simple_copy.py +++ b/test/integration/001_simple_copy_test/test_simple_copy.py @@ -1,13 +1,8 @@ from nose.plugins.attrib import attr from test.integration.base import DBTIntegrationTest, use_profile -from dbt import deprecations class BaseTestSimpleCopy(DBTIntegrationTest): - def setUp(self): - super(BaseTestSimpleCopy, self).setUp() - deprecations.reset_deprecations() - @property def schema(self): return "simple_copy_001" @@ -26,11 +21,8 @@ class TestSimpleCopy(BaseTestSimpleCopy): def test__postgres__simple_copy(self): self.use_default_project({"data-paths": [self.dir("seed-initial")]}) - self.assertEqual(deprecations.active_deprecations, set()) results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) - self.assertEqual({'adapter:already_exists', 'sql_where'}, - deprecations.active_deprecations) results = self.run_dbt() self.assertEqual(len(results), 6) diff --git a/test/integration/012_deprecation_tests/models/already_exists.sql b/test/integration/012_deprecation_tests/models/already_exists.sql new file mode 100644 index 00000000000..0290d5e1c10 --- /dev/null +++ b/test/integration/012_deprecation_tests/models/already_exists.sql @@ -0,0 +1,5 @@ +select 1 as id + +{% if adapter.already_exists(this.schema, this.identifier) and not flags.FULL_REFRESH %} + where id > (select max(id) from {{this}}) +{% endif %} diff --git a/test/integration/012_deprecation_tests/models/sql_where.sql b/test/integration/012_deprecation_tests/models/sql_where.sql new file mode 100644 index 00000000000..34ca3c36464 --- /dev/null +++ b/test/integration/012_deprecation_tests/models/sql_where.sql @@ -0,0 +1,3 @@ +{{ config(sql_where='id > (select max(id) from {{this}})')}} + +select 1 as id diff --git a/test/integration/012_deprecation_tests/test_deprecations.py b/test/integration/012_deprecation_tests/test_deprecations.py new file mode 100644 index 00000000000..2642d9acefb --- /dev/null +++ b/test/integration/012_deprecation_tests/test_deprecations.py @@ -0,0 +1,34 @@ +from test.integration.base import DBTIntegrationTest, use_profile + +from dbt import deprecations +import dbt.exceptions + + +class TestDeprecations(DBTIntegrationTest): + def setUp(self): + super(TestDeprecations, self).setUp() + deprecations.reset_deprecations() + + @property + def schema(self): + return "deprecation_test_012" + + @staticmethod + def dir(path): + return "test/integration/012_deprecation_tests/" + path.lstrip("/") + + @property + def models(self): + return self.dir("models") + + @use_profile('postgres') + def test_postgres_deprecations_fail(self): + with self.assertRaises(dbt.exceptions.CompilationException): + self.run_dbt(strict=True) + + @use_profile('postgres') + def test_postgres_deprecations(self): + self.assertEqual(deprecations.active_deprecations, set()) + results = self.run_dbt(strict=False) + self.assertEqual({'adapter:already_exists', 'sql_where'}, + deprecations.active_deprecations)