Merge pull request #2150 from fishtown-analytics/fix/source-test-arg-…

…rendering

Fix source test arg rendering (#2114)

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## dbt next (Release TBD)
 
+### Breaking changes
+- Arguments to source tests are not parsed in the config-rendering context, and are passed as their literal unparsed values to macros ([#2150](https://github.com/fishtown-analytics/dbt/pull/2150))
+
 ### Features
 - Add a "docs" field to models, with a "show" subfield ([#1671](https://github.com/fishtown-analytics/dbt/issues/1671), [#2107](https://github.com/fishtown-analytics/dbt/pull/2107))
 - Add a dbt-{dbt_version} user agent field to the bigquery connector ([#2121](https://github.com/fishtown-analytics/dbt/issues/2121), [#2146](https://github.com/fishtown-analytics/dbt/pull/2146))
@@ -8,6 +11,7 @@
 - Fix issue where dbt did not give an error in the presence of duplicate doc names ([#2054](https://github.com/fishtown-analytics/dbt/issues/2054), [#2080](https://github.com/fishtown-analytics/dbt/pull/2080))
 - Include vars provided to the cli method when running the actual method ([#2092](https://github.com/fishtown-analytics/dbt/issues/2092), [#2104](https://github.com/fishtown-analytics/dbt/pull/2104))
 - Improved error messages with malformed packages.yml ([#2017](https://github.com/fishtown-analytics/dbt/issues/2017), [#2078](https://github.com/fishtown-analytics/dbt/pull/2078))
+- Fix an issue where dbt rendered source test args, fix issue where dbt ran an extra compile pass over the wrapped SQL. ([#2114](https://github.com/fishtown-analytics/dbt/issues/2114), [#2150](https://github.com/fishtown-analytics/dbt/pull/2150))
 
 Contributors:
  - [@bubbomb](https://github.com/bubbomb) ([#2080](https://github.com/fishtown-analytics/dbt/pull/2080))

core/dbt/compilation.py

@@ -248,7 +248,6 @@ def _is_writable(node):
 def compile_node(adapter, config, node, manifest, extra_context, write=True):
     compiler = Compiler(config)
     node = compiler.compile_node(node, manifest, extra_context)
-    node = _inject_runtime_config(adapter, node, extra_context)
 
     if write and _is_writable(node):
         logger.debug('Writing injected SQL for node "{}"'.format(
@@ -261,21 +260,3 @@ def compile_node(adapter, config, node, manifest, extra_context, write=True):
         )
 
     return node
-
-
-def _inject_runtime_config(adapter, node, extra_context):
-    wrapped_sql = node.wrapped_sql
-    context = _node_context(adapter, node)
-    context.update(extra_context)
-    sql = dbt.clients.jinja.get_rendered(wrapped_sql, context)
-    node.wrapped_sql = sql
-    return node
-
-
-def _node_context(adapter, node):
-    if dbt.tracking.active_user is not None:
-        return {
-            "run_started_at": dbt.tracking.active_user.run_started_at,
-            "invocation_id": dbt.tracking.active_user.invocation_id,
-        }
-    return {}   # this never happens, but make mypy happy

core/dbt/config/renderer.py

@@ -72,11 +72,25 @@ def _render_profile_data(self, value, keypath):
                 pass
         return result
 
+    @staticmethod
+    def _is_schema_test(keypath) -> bool:
+        # we got passed an UnparsedSourceDefinition
+        if len(keypath) > 2 and keypath[0] == 'tables':
+            if keypath[2] == 'tests':
+                return True
+            elif keypath[2] == 'columns':
+                if len(keypath) > 4 and keypath[4] == 'tests':
+                    return True
+        return False
+
     def _render_schema_source_data(self, value, keypath):
         # things to not render:
         # - descriptions
+        # - test arguments
         if len(keypath) > 0 and keypath[-1] == 'description':
             return value
+        elif self._is_schema_test(keypath):
+            return value
 
         return self.render_value(value)
 

test/integration/029_docs_generate_tests/test_docs_generate.py

@@ -962,7 +962,7 @@ def expected_seeded_manifest(self, model_database=None):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': ANY,
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'seed.test.seed': {
                     'build_path': None,
@@ -1044,7 +1044,7 @@ def expected_seeded_manifest(self, model_database=None):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': '',
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'test.test.not_null_model_id': {
                     'alias': 'not_null_model_id',
@@ -1373,7 +1373,7 @@ def expected_postgres_references_manifest(self, model_database=None):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': ANY,
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'model.test.ephemeral_summary': {
                     'alias': 'ephemeral_summary',
@@ -1437,7 +1437,7 @@ def expected_postgres_references_manifest(self, model_database=None):
                     'extra_ctes_injected': True,
                     'extra_ctes': [ANY],
                     'injected_sql': ANY,
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'model.test.view_summary': {
                     'alias': 'view_summary',
@@ -1500,7 +1500,7 @@ def expected_postgres_references_manifest(self, model_database=None):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': ANY,
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'seed.test.seed': {
                     'alias': 'seed',
@@ -1579,7 +1579,7 @@ def expected_postgres_references_manifest(self, model_database=None):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': '',
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'source.test.my_source.my_table': {
                     'columns': {
@@ -1952,7 +1952,7 @@ def expected_bigquery_complex_manifest(self):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': ANY,
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'model.test.multi_clustered': {
                     'alias': 'multi_clustered',
@@ -2031,7 +2031,7 @@ def expected_bigquery_complex_manifest(self):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': ANY,
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'model.test.nested_view': {
                     'alias': 'nested_view',
@@ -2111,7 +2111,7 @@ def expected_bigquery_complex_manifest(self):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': ANY,
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'model.test.nested_table': {
                     'alias': 'nested_table',
@@ -2155,7 +2155,7 @@ def expected_bigquery_complex_manifest(self):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': ANY,
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'seed.test.seed': {
                     'build_path': None,
@@ -2237,7 +2237,7 @@ def expected_bigquery_complex_manifest(self):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': '',
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
             },
             'child_map': {
@@ -2462,7 +2462,7 @@ def expected_redshift_incremental_view_manifest(self):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': ANY,
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'seed.test.seed': {
                     'build_path': None,
@@ -2544,7 +2544,7 @@ def expected_redshift_incremental_view_manifest(self):
                     'extra_ctes_injected': True,
                     'extra_ctes': [],
                     'injected_sql': ANY,
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
             },
             'parent_map': {
@@ -2784,7 +2784,7 @@ def expected_run_results(self, quote_schema=True, quote_model=False,
                     'database': model_database,
                     'tags': [],
                     'unique_id': 'model.test.model',
-                    'wrapped_sql': 'None'
+                    'wrapped_sql': None
                 },
                 'thread_id': ANY,
                 'timing': [ANY, ANY],
@@ -2873,7 +2873,7 @@ def expected_run_results(self, quote_schema=True, quote_model=False,
                     'database': self.default_database,
                     'tags': [],
                     'unique_id': 'seed.test.seed',
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'thread_id': ANY,
                 'timing': [ANY, ANY],
@@ -3163,7 +3163,7 @@ def expected_postgres_references_run_results(self):
                     'database': self.default_database,
                     'tags': [],
                     'unique_id': 'model.test.ephemeral_summary',
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'thread_id': ANY,
                 'timing': [ANY, ANY],
@@ -3242,7 +3242,7 @@ def expected_postgres_references_run_results(self):
                     'database': self.default_database,
                     'tags': [],
                     'unique_id': 'model.test.view_summary',
-                    'wrapped_sql': 'None',
+                    'wrapped_sql': None,
                 },
                 'thread_id': ANY,
                 'timing': [ANY, ANY],
@@ -3331,7 +3331,7 @@ def expected_postgres_references_run_results(self):
                     'database': self.default_database,
                     'tags': [],
                     'unique_id': 'seed.test.seed',
-                    'wrapped_sql': 'None'
+                    'wrapped_sql': None
                 },
                 'thread_id': ANY,
                 'timing': [ANY, ANY],

test/integration/042_sources_test/malformed_models/schema.yml

@@ -8,7 +8,7 @@ sources:
         identifier: source
         tests:
           - relationships:
-            # this is invalid
+            # this is invalid (list of 3 1-key dicts instead of a single 3-key dict)
               - column_name: favorite_color
               - to: ref('descendant_model')
               - field: favorite_color

test/integration/042_sources_test/malformed_schema_tests/model.sql

@@ -0,0 +1 @@
+select * from {{ source('test_source', 'test_table') }}

test/integration/042_sources_test/malformed_schema_tests/schema.yml

@@ -0,0 +1,14 @@
+version: 2
+sources:
+  - name: test_source
+    schema: "{{ var('test_run_schema') }}"
+    tables:
+      - name: test_table
+        identifier: source
+        columns:
+          - name: favorite_color
+            tests:
+              - relationships:
+                  to: ref('model')
+                  # this will get rendered as its literal
+                  field: "{{ 'favorite' ~ 'color' }}"

test/integration/042_sources_test/test_sources.py

@@ -377,6 +377,19 @@ def test_postgres_malformed_schema_strict_will_break_run(self):
             self.run_dbt_with_vars(['seed'], strict=True)
 
 
+class TestRenderingInSourceTests(BaseSourcesTest):
+    @property
+    def models(self):
+        return "malformed_schema_tests"
+
+    @use_profile('postgres')
+    def test_postgres_render_in_source_tests(self):
+        self.run_dbt_with_vars(['seed'])
+        self.run_dbt_with_vars(['run'])
+        # syntax error at or near "{", because the test isn't rendered
+        self.run_dbt_with_vars(['test'], expect_pass=False)
+
+
 class TestUnquotedSources(SuccessfulSourcesTest):
     @property
     def project_config(self):

test/unit/test_contracts_graph_compiled.py

@@ -139,7 +139,6 @@ def test_basic_compiled(self):
             'extra_ctes': [{'id': 'whatever', 'sql': 'select * from other'}],
             'extra_ctes_injected': True,
             'injected_sql': 'with whatever as (select * from other) select * from whatever',
-            'wrapped_sql': 'None',
         }
         node = self.ContractType(
             package_name='test',
@@ -166,7 +165,6 @@ def test_basic_compiled(self):
             extra_ctes=[InjectedCTE('whatever', 'select * from other')],
             extra_ctes_injected=True,
             injected_sql='with whatever as (select * from other) select * from whatever',
-            wrapped_sql='None',
         )
         self.assert_symmetric(node, node_dict)
         self.assertFalse(node.empty)