Fix/bad integration tests (#539)

* only issue commit if there was a begin for the view

* fix some tests that tested nothing, fixed broken code that now broke tests

* s/constant_statement/noop_statement/g

dbt/include/global_project/macros/core.sql

@@ -14,3 +14,17 @@
 
   {%- endif -%}
 {%- endmacro %}
+
+{% macro noop_statement(name=None, status=None, res=None) -%}
+  {%- set sql = render(caller()) -%}
+
+  {%- if name == 'main' -%}
+    {{ log('Writing runtime SQL for node "{}"'.format(model['unique_id'])) }}
+    {{ write(sql) }}
+  {%- endif -%}
+
+  {%- if name is not none -%}
+    {{ store_result(name, status=status, data=res) }}
+  {%- endif -%}
+
+{%- endmacro %}

dbt/include/global_project/macros/materializations/view.sql

@@ -6,15 +6,27 @@
   {%- set existing = adapter.query_for_existing(schema) -%}
   {%- set existing_type = existing.get(identifier) -%}
 
+  {%- set has_transactional_hooks = (hooks | selectattr('transaction', 'equalto', True) | list | length) > 0 %}
+  {%- set should_ignore = non_destructive_mode and existing_type == 'view' %}
+
   {{ run_hooks(pre_hooks, inside_transaction=False) }}
   {{ drop_if_exists(existing, schema, tmp_identifier) }}
 
   -- `BEGIN` happens here:
   {{ run_hooks(pre_hooks, inside_transaction=True) }}
 
   -- build model
-  {% if non_destructive_mode and existing_type == 'view' -%}
-    -- noop
+  {% if should_ignore -%}
+    {#
+      -- Materializations need to a statement with name='main'.
+      -- We could issue a no-op query here (like `select 1`), but that's wasteful. Instead:
+      --   1) write the sql contents out to the compiled dirs
+      --   2) return a status and result to the caller
+    #}
+    {% call noop_statement('main', status="PASS", res=[]) -%}
+      -- Not running : non-destructive mode
+      {{ sql }}
+    {%- endcall %}
   {%- else -%}
     {% call statement('main') -%}
       {{ create_view_as(tmp_identifier, sql) }}
@@ -24,15 +36,18 @@
   {{ run_hooks(post_hooks, inside_transaction=True) }}
 
   -- cleanup
-  {% if non_destructive_mode and existing_type == 'view' -%}
-    -- noop
-  {%- else -%}
+  {% if not should_ignore -%}
     {{ drop_if_exists(existing, schema, identifier) }}
     {{ adapter.rename(schema, tmp_identifier, identifier) }}
   {%- endif %}
 
-  -- `COMMIT` happens here
-  {{ adapter.commit() }}
+  {#
+      -- Don't commit in non-destructive mode _unless_ there are in-transaction hooks
+      -- TODO : Figure out some other way of doing this that isn't as fragile
+  #}
+  {% if has_transactional_hooks or not should_ignore %}
+      {{ adapter.commit() }}
+  {% endif %}
 
   {{ run_hooks(post_hooks, inside_transaction=False) }}
 

test/integration/010_permission_tests/seed.sql

@@ -1,6 +1,10 @@
-create schema {schema}_private_010;
 
-create table {schema}_private_010.seed (
+create schema if not exists {schema};
+
+revoke create on database dbt from noaccess;
+revoke usage on schema {schema} from noaccess;
+
+create table {schema}.seed (
 	id BIGSERIAL PRIMARY KEY,
 	first_name VARCHAR(50),
 	last_name VARCHAR(50),
@@ -10,13 +14,13 @@ create table {schema}_private_010.seed (
 );
 
 
-insert into {schema}_private_010.seed (first_name, last_name, email, gender, ip_address) values ('Kathryn', 'Walker', '[email protected]', 'Female', '194.121.179.35');
-insert into {schema}_private_010.seed (first_name, last_name, email, gender, ip_address) values ('Gerald', 'Ryan', '[email protected]', 'Male', '11.3.212.243');
-insert into {schema}_private_010.seed (first_name, last_name, email, gender, ip_address) values ('Bonnie', 'Spencer', '[email protected]', 'Female', '216.32.196.175');
-insert into {schema}_private_010.seed (first_name, last_name, email, gender, ip_address) values ('Harold', 'Taylor', '[email protected]', 'Male', '253.10.246.136');
-insert into {schema}_private_010.seed (first_name, last_name, email, gender, ip_address) values ('Jacqueline', 'Griffin', '[email protected]', 'Female', '16.13.192.220');
-insert into {schema}_private_010.seed (first_name, last_name, email, gender, ip_address) values ('Wanda', 'Arnold', '[email protected]', 'Female', '232.116.150.64');
-insert into {schema}_private_010.seed (first_name, last_name, email, gender, ip_address) values ('Craig', 'Ortiz', '[email protected]', 'Male', '199.126.106.13');
-insert into {schema}_private_010.seed (first_name, last_name, email, gender, ip_address) values ('Gary', 'Day', '[email protected]', 'Male', '35.81.68.186');
-insert into {schema}_private_010.seed (first_name, last_name, email, gender, ip_address) values ('Rose', 'Wright', '[email protected]', 'Female', '236.82.178.100');
-insert into {schema}_private_010.seed (first_name, last_name, email, gender, ip_address) values ('Raymond', 'Kelley', '[email protected]', 'Male', '213.65.166.67');
+insert into {schema}.seed (first_name, last_name, email, gender, ip_address) values ('Kathryn', 'Walker', '[email protected]', 'Female', '194.121.179.35');
+insert into {schema}.seed (first_name, last_name, email, gender, ip_address) values ('Gerald', 'Ryan', '[email protected]', 'Male', '11.3.212.243');
+insert into {schema}.seed (first_name, last_name, email, gender, ip_address) values ('Bonnie', 'Spencer', '[email protected]', 'Female', '216.32.196.175');
+insert into {schema}.seed (first_name, last_name, email, gender, ip_address) values ('Harold', 'Taylor', '[email protected]', 'Male', '253.10.246.136');
+insert into {schema}.seed (first_name, last_name, email, gender, ip_address) values ('Jacqueline', 'Griffin', '[email protected]', 'Female', '16.13.192.220');
+insert into {schema}.seed (first_name, last_name, email, gender, ip_address) values ('Wanda', 'Arnold', '[email protected]', 'Female', '232.116.150.64');
+insert into {schema}.seed (first_name, last_name, email, gender, ip_address) values ('Craig', 'Ortiz', '[email protected]', 'Male', '199.126.106.13');
+insert into {schema}.seed (first_name, last_name, email, gender, ip_address) values ('Gary', 'Day', '[email protected]', 'Male', '35.81.68.186');
+insert into {schema}.seed (first_name, last_name, email, gender, ip_address) values ('Rose', 'Wright', '[email protected]', 'Female', '236.82.178.100');
+insert into {schema}.seed (first_name, last_name, email, gender, ip_address) values ('Raymond', 'Kelley', '[email protected]', 'Male', '213.65.166.67');

test/integration/010_permission_tests/tearDown.sql

@@ -1,2 +1,2 @@
 
-drop schema if exists {schema}_private_010 cascade;
+drop schema if exists {schema} cascade;

test/integration/010_permission_tests/test_permissions.py

@@ -28,14 +28,17 @@ def test_create_schema_permissions(self):
         failed = False
         self.run_sql('drop schema if exists "{}" cascade'.format(self.unique_schema()))
         try:
-            self.run_dbt(['run', '--target', 'noaccess'])
+            self.run_dbt(['run', '--target', 'noaccess'], expect_pass=False)
         except RuntimeError:
             failed = True
 
         self.assertTrue(failed)
 
+        self.run_sql_file("test/integration/010_permission_tests/seed.sql")
+
         # now it should work!
-        self.run_sql('create schema "{}"'.format(self.unique_schema()))
-        self.run_sql('grant usage on schema "{}" to noaccess'.format(self.unique_schema()))
+        self.run_sql('grant create on database dbt to noaccess'.format(self.unique_schema()))
+        self.run_sql('grant usage, create on schema "{}" to noaccess'.format(self.unique_schema()))
+        self.run_sql('grant select on all tables in schema "{}" to noaccess'.format(self.unique_schema()))
 
         self.run_dbt(['run', '--target', 'noaccess'])

test/integration/016_macro_tests/macros/my_macros.sql

@@ -10,4 +10,6 @@
 
 {% macro with_ref() %}
 
+    {{ ref('table') }}
+
 {% endmacro %}

test/integration/016_macro_tests/seed.sql

@@ -8,11 +8,17 @@ create table {schema}.expected_local_macro (
 	bar2 TEXT
 );
 
+create table {schema}.seed (
+	id integer,
+	updated_at timestamp
+);
+
 insert into {schema}.expected_dep_macro (foo, bar)
 values ('arg1', 'arg2');
 
 insert into {schema}.expected_local_macro (foo2, bar2)
 values ('arg1', 'arg2'), ('arg3', 'arg4');
 
-
+insert into {schema}.seed (id, updated_at)
+values (1, '2017-01-01'), (2, '2017-01-02');
 

test/integration/016_macro_tests/test_macros.py

@@ -62,7 +62,7 @@ def project_config(self):
     def test_invalid_macro(self):
 
         try:
-            self.run_dbt(["run"])
+            self.run_dbt(["run"], expect_pass=False)
             self.assertTrue(False,
                             'compiling bad macro should raise a runtime error')
 

test/integration/021_concurrency_test/test_concurrency.py

@@ -21,7 +21,7 @@ def test__postgres__concurrency(self):
         self.use_profile('postgres')
         self.run_sql_file("test/integration/021_concurrency_test/seed.sql")
 
-        self.run_dbt()
+        self.run_dbt(expect_pass=False)
 
         self.assertTablesEqual("seed", "view")
         self.assertTablesEqual("seed", "dep")
@@ -32,7 +32,7 @@ def test__postgres__concurrency(self):
 
         self.run_sql_file("test/integration/021_concurrency_test/update.sql")
 
-        self.run_dbt()
+        self.run_dbt(expect_pass=False)
 
         self.assertTablesEqual("seed", "view")
         self.assertTablesEqual("seed", "dep")
@@ -47,7 +47,7 @@ def test__snowflake__concurrency(self):
         self.use_profile('snowflake')
         self.run_sql_file("test/integration/021_concurrency_test/seed.sql")
 
-        self.run_dbt()
+        self.run_dbt(expect_pass=False)
 
         self.assertTablesEqual("seed", "view")
         self.assertTablesEqual("seed", "dep")
@@ -56,7 +56,7 @@ def test__snowflake__concurrency(self):
 
         self.run_sql_file("test/integration/021_concurrency_test/update.sql")
 
-        self.run_dbt()
+        self.run_dbt(expect_pass=False)
 
         self.assertTablesEqual("seed", "view")
         self.assertTablesEqual("seed", "dep")

test/integration/base.py

@@ -264,13 +264,17 @@ def project_config(self):
     def profile_config(self):
         return {}
 
-    def run_dbt(self, args=None):
+    def run_dbt(self, args=None, expect_pass=True):
         if args is None:
             args = ["run"]
 
         args = ["--strict"] + args
         logger.info("Invoking dbt with {}".format(args))
-        return dbt.handle(args)
+
+        res, success =  dbt.handle_and_check(args)
+        self.assertEqual(success, expect_pass, "dbt exit state did not match expected")
+
+        return res
 
     def run_dbt_and_check(self, args=None):
         if args is None: