Repository: dbt-labs/dbt-core
Pull request: Fix #9593: Validation of unit test parsing for incremental models
resolves #9593

### Problem

Unclear error message when `is_incremental` override wasn't specified for a unit test 

### Solution

Add parse-time errors when:
* unit testing an incremental model but not providing an override for is_incremental
* unit testing an incremental model overriding is_incremental: true but NOT providing an input for this

### Checklist

- [x] I have read [the contributing guide](https://github.com/dbt-labs/dbt-core/blob/main/CONTRIBUTING.md) and understand what's expected of me  
- [x] I have run this code in development and it appears to resolve the stated issue  
- [x] This PR includes tests, or tests are not required/relevant for this PR
- [x] This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX
- [x] This PR includes [type annotations](https://docs.python.org/3/library/typing.html) for new and modified functions


Problem statement:
[Bug] parse-time validation when unit testing incremental model
### Is this a new bug in dbt-core?

- [X] I believe this is a new bug in dbt-core
- [X] I have searched the existing issues, and I could not find an existing issue for this bug

### Current Behavior

Suppose a model includes the `is_incremental()` macro.

If a unit test is added for that model, then an override like the following is required or else an error will occur:

```yaml
    overrides:
      macros:
        is_incremental: true
```

### Acceptance Criteria

We should have a parse time error when:
- unit testing an incremental model but not providing an override for `is_incremental`
- unit testing an incremental model overriding `is_incremental: true` but NOT providing an input for `this`

### Steps To Reproduce

`models/my_model.sql`

```sql
-- is_incremental: {{ is_incremental() }}
select
    1 as id
```

`models/_unit_tests.yml`

```yaml
unit_tests:

  - name: demo_unit_test_overrides

    given: []

    model: my_model

    expect:
      format: csv
      rows: |
        id
        1
```

```shell
dbt run -s models/my_model.sql
dbt build -s models/my_model.sql
```

💥 See error log below.

In order to work, the following needs to be added within the unit test YAML above:

```yaml
    overrides:
      macros:
        is_incremental: true
```

### Relevant log output

```shell
21:41:03    Compilation Error in unit_test my_model__demo_unit_test_overrides (models/_unit_tests.yml)
  'None' has no attribute 'database'
```


### Environment

```markdown
- OS:
- Python:
- dbt: 1.8 beta
```


### Which database adapter are you using with dbt?

_No response_

### Additional Context

_No response_

Hints from issue discussion:
Curious what @MichelleArk @jtcohen6 you think here...

I actually think it's a good thing that we require you to be explicit about whether this is a unit test for the incremental model in "incremental" mode or in "full refresh" mode. I'm not convinced we _should_ set a default here (is that a spicy take?). However, if we don't set a default, we **should**:
- update our [docs on unit testing incremental models](https://github.com/dbt-labs/docs.getdbt.com/pull/4959)
- update the error message returned to be explicit "For incremental models you must set is_incremental to true or false in your unit test overrides" (or something similar) - right now, the error message is unreadable 
This particular error is probably happening because [`this` was not overwritten](https://github.com/dbt-labs/docs.getdbt.com/pull/4959/files#diff-93cd3964ff9d3a505e3620438d7b0594b473f47a26cec74e57d3c24e36df1a24R313-R316), and the 'real' `is_incremental` macro was attempting to access `this.database`: https://github.com/dbt-labs/dbt-adapters/blob/main/dbt/include/global_project/macros/materializations/models/incremental/is_incremental.sql#L7


> I'm not convinced we should set a default here (is that a spicy take?).

I'm not convinced either. It feels like either setting (is_incremental: true or is_incremental: false) could be perceived as surprising behaviour and we'd get issue reports either way. 

> update the error message returned to be explicit "For incremental models you must set is_incremental to true or false in your unit test overrides" (or something similar) 

I like the idea of parse-time validation here! 👍 

---- 
Sounds good! Updated the issue name and acceptance criteria accordingly 
From refinement: 
* Can determine whether the model being tested relies on the is_incremental macro based on its depends_on.macros (rather than checking for materialized=incremental, which is less precise)
* can also validate that is_incremental is being overriden with a boolean value

Commit message: Update Fixes-20240317-005611.yaml

Patch to review:
diff --git a/.changes/unreleased/Fixes-20240317-005611.yaml b/.changes/unreleased/Fixes-20240317-005611.yaml
new file mode 100644
index 00000000000..0878779174a
--- /dev/null
+++ b/.changes/unreleased/Fixes-20240317-005611.yaml
@@ -0,0 +1,6 @@
+kind: Fixes
+body: 'Validation of unit test parsing for incremental models'
+time: 2024-03-17T00:56:11.855232-07:00
+custom:
+  Author: aranke
+  Issue: "9593"
diff --git a/core/dbt/parser/unit_tests.py b/core/dbt/parser/unit_tests.py
index bdcbb9d4428..454bceee21e 100644
--- a/core/dbt/parser/unit_tests.py
+++ b/core/dbt/parser/unit_tests.py
@@ -420,7 +420,6 @@ def find_tested_model_node(
 def process_models_for_unit_test(
     manifest: Manifest, current_project: str, unit_test_def: UnitTestDefinition, models_to_versions
 ):
-
     # If the unit tests doesn't have a depends_on.nodes[0] then we weren't able to resolve
     # the model, either because versions hadn't been processed yet, or it's not a valid model name
     if not unit_test_def.depends_on.nodes:
@@ -438,6 +437,31 @@ def process_models_for_unit_test(
     target_model_id = unit_test_def.depends_on.nodes[0]
     target_model = manifest.nodes[target_model_id]
     assert isinstance(target_model, ModelNode)
+
+    target_model_is_incremental = "macro.dbt.is_incremental" in target_model.depends_on.macros
+    unit_test_def_has_incremental_override = unit_test_def.overrides and isinstance(
+        unit_test_def.overrides.macros.get("is_incremental"), bool
+    )
+
+    if target_model_is_incremental and (not unit_test_def_has_incremental_override):
+        raise ParsingError(
+            f"Boolean override for 'is_incremental' must be provided for unit testing model '{target_model.name}'"
+        )
+
+    unit_test_def_incremental_override_true = (
+        unit_test_def.overrides and unit_test_def.overrides.macros.get("is_incremental")
+    )
+    unit_test_def_has_this_input = "this" in [i.input for i in unit_test_def.given]
+
+    if (
+        target_model_is_incremental
+        and unit_test_def_incremental_override_true
+        and (not unit_test_def_has_this_input)
+    ):
+        raise ParsingError(
+            f"Unit test '{unit_test_def.name}' for incremental model '{target_model.name}' must have a 'this' input"
+        )
+
     # unit_test_versions = unit_test_def.versions
     # We're setting up unit tests for versioned models, so if
     # the model isn't versioned, we don't need to do anything
diff --git a/tests/functional/unit_testing/fixtures.py b/tests/functional/unit_testing/fixtures.py
index 02af9b418ba..54f0497250a 100644
--- a/tests/functional/unit_testing/fixtures.py
+++ b/tests/functional/unit_testing/fixtures.py
@@ -266,7 +266,7 @@
 {% endif %}
 """
 
-test_my_model_incremental_yml = """
+test_my_model_incremental_yml_basic = """
 unit_tests:
   - name: incremental_false
     model: my_incremental_model
@@ -300,6 +300,54 @@
         - {event_time: "2020-01-03", event: 3}
 """
 
+test_my_model_incremental_yml_no_override = """
+unit_tests:
+  - name: incremental_false
+    model: my_incremental_model
+    given:
+      - input: ref('events')
+        rows:
+          - {event_time: "2020-01-01", event: 1}
+    expect:
+      rows:
+        - {event_time: "2020-01-01", event: 1}
+"""
+
+test_my_model_incremental_yml_wrong_override = """
+unit_tests:
+  - name: incremental_false
+    model: my_incremental_model
+    overrides:
+      macros:
+        is_incremental: foobar
+    given:
+      - input: ref('events')
+        rows:
+          - {event_time: "2020-01-01", event: 1}
+    expect:
+      rows:
+        - {event_time: "2020-01-01", event: 1}
+"""
+
+test_my_model_incremental_yml_no_this_input = """
+unit_tests:
+  - name: incremental_true
+    model: my_incremental_model
+    overrides:
+      macros:
+        is_incremental: true
+    given:
+      - input: ref('events')
+        rows:
+          - {event_time: "2020-01-01", event: 1}
+          - {event_time: "2020-01-02", event: 2}
+          - {event_time: "2020-01-03", event: 3}
+    expect:
+      rows:
+        - {event_time: "2020-01-02", event: 2}
+        - {event_time: "2020-01-03", event: 3}
+"""
+
 # -- inline csv tests
 
 test_my_model_csv_yml = """
diff --git a/tests/functional/unit_testing/test_unit_testing.py b/tests/functional/unit_testing/test_unit_testing.py
index c1cb048dbfa..43fa719979d 100644
--- a/tests/functional/unit_testing/test_unit_testing.py
+++ b/tests/functional/unit_testing/test_unit_testing.py
@@ -18,13 +18,16 @@
     datetime_test,
     my_incremental_model_sql,
     event_sql,
-    test_my_model_incremental_yml,
+    test_my_model_incremental_yml_basic,
     test_my_model_yml_invalid,
     test_my_model_yml_invalid_ref,
     valid_emails_sql,
     top_level_domains_sql,
     external_package__accounts_seed_csv,
     external_package,
+    test_my_model_incremental_yml_no_override,
+    test_my_model_incremental_yml_wrong_override,
+    test_my_model_incremental_yml_no_this_input,
 )
 
 
@@ -109,13 +112,13 @@ def test_basic(self, project):
             run_dbt(["run", "--no-partial-parse", "--select", "my_model"])
 
 
-class TestUnitTestIncrementalModel:
+class TestUnitTestIncrementalModelBasic:
     @pytest.fixture(scope="class")
     def models(self):
         return {
             "my_incremental_model.sql": my_incremental_model_sql,
             "events.sql": event_sql,
-            "test_my_incremental_model.yml": test_my_model_incremental_yml,
+            "schema.yml": test_my_model_incremental_yml_basic,
         }
 
     def test_basic(self, project):
@@ -127,6 +130,57 @@ def test_basic(self, project):
         assert len(results) == 2
 
 
+class TestUnitTestIncrementalModelNoOverride:
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {
+            "my_incremental_model.sql": my_incremental_model_sql,
+            "events.sql": event_sql,
+            "schema.yml": test_my_model_incremental_yml_no_override,
+        }
+
+    def test_no_override(self, project):
+        with pytest.raises(
+            ParsingError,
+            match="Boolean override for 'is_incremental' must be provided for unit testing model 'my_incremental_model'",
+        ):
+            run_dbt(["parse"])
+
+
+class TestUnitTestIncrementalModelWrongOverride:
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {
+            "my_incremental_model.sql": my_incremental_model_sql,
+            "events.sql": event_sql,
+            "schema.yml": test_my_model_incremental_yml_wrong_override,
+        }
+
+    def test_str_override(self, project):
+        with pytest.raises(
+            ParsingError,
+            match="Boolean override for 'is_incremental' must be provided for unit testing model 'my_incremental_model'",
+        ):
+            run_dbt(["parse"])
+
+
+class TestUnitTestIncrementalModelNoThisInput:
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {
+            "my_incremental_model.sql": my_incremental_model_sql,
+            "events.sql": event_sql,
+            "schema.yml": test_my_model_incremental_yml_no_this_input,
+        }
+
+    def test_no_this_input(self, project):
+        with pytest.raises(
+            ParsingError,
+            match="Unit test 'incremental_true' for incremental model 'my_incremental_model' must have a 'this' input",
+        ):
+            run_dbt(["parse"])
+
+
 my_new_model = """
 select
 my_favorite_seed.id,


Code review comments:
diff_hunk: @@ -438,6 +437,31 @@ def process_models_for_unit_test(
     target_model_id = unit_test_def.depends_on.nodes[0]
     target_model = manifest.nodes[target_model_id]
     assert isinstance(target_model, ModelNode)
+
+    target_model_is_incremental = "macro.dbt.is_incremental" in target_model.depends_on.macros
+    unit_test_def_has_incremental_override = unit_test_def.overrides and isinstance(
+        unit_test_def.overrides.macros.get("is_incremental"), bool
+    )
+
+    if target_model_is_incremental and (not unit_test_def_has_incremental_override):
+        raise ParsingError(
+            f"Boolean override for 'is_incremental' must be provided for unit testing model '{target_model.name}'"
path: core/dbt/parser/unit_tests.py
text: @user1:
suggestion -- let's also add the unit test name here for ease of user debugging since there can be multiple unit tests for a single model