[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:
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: truebut NOT providing an input forthis
Steps To Reproduce
models/my_model.sql
-- is_incremental: {{ is_incremental() }}
select
1 as id
models/_unit_tests.yml
unit_tests:
- name: demo_unit_test_overrides
given: []
model: my_model
expect:
format: csv
rows: |
id
1
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:
overrides:
macros:
is_incremental: true
Relevant log output
21:41:03 Compilation Error in unit_test my_model__demo_unit_test_overrides (models/_unit_tests.yml)
'None' has no attribute 'database'
Environment
- OS:
- Python:
- dbt: 1.8 beta
Which database adapter are you using with dbt?
No response
Additional Context
No response
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
- 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, 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