-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: dbt unit tests feat without proper context #10849
base: main
Are you sure you want to change the base?
Conversation
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10849 +/- ##
==========================================
- Coverage 89.18% 89.13% -0.06%
==========================================
Files 183 183
Lines 23429 23466 +37
==========================================
+ Hits 20895 20916 +21
- Misses 2534 2550 +16
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Sorry to bother you @gshank, I looked into the git history in those methods, and it seems you're probably the best person to ask these questions. |
Okay, I dove into the code and found a way to make this work for me by capturing the variable and environment variable using the same logic used for ref, source and metrics |
Hi @MichelleArk , sorry for to bother you, but since you're keeping an eye on the issues related to unit tests, could you help me with some questions I have in this PR? |
Resolves #10539, resolves #10353 , resolves #10410
I made a very simple change to the code just to solve a similar problem 10353, but this change is obviously not the correct solution. I’ll wait for a discussion before writing any more advanced code.
Problem
absence of
env_var
andvar
in thectx
dictionary, which must be passed to the Jinja render method.Solution
If a model has a unit test, the
get_rendered
method is called twice, each time with a different context. The issue arises withbuild_unit_test_manifest_from_test
. When this method is called (dbt-core/core/dbt/parser/unit_tests.py
Line 116 in f6088db
var
and environment variables (env_var
).Later, when the
get_rendered
method fromclients/jinja.py
callsrender_template
fromdbt_common/clients/jinja.py
, we encounter a compilation error due to the absence ofenv_var
andvar
in thectx
dictionary, which must be passed to the Jinja render method. **The error will not appear if we don't have any kind of mixing with Nones, for exampleselect {{ var('something,1) }} as result
instead ofselect {{ var('something,1)*2 }} as result
Some questions:
Is this the expected behavior? Shouldn't the
ctx
used to render the string for the unit test include these variables?Is there a reason why the same model is rendered more than once — once to build the model and again to render the unit test? Why is this string rendered a second time if it doesn’t seem to be used for any data manipulation?
models/_unit_tests.yml
models/test.sql
This will only work for #10539 if ctx is constructed using
env_var
andvar
, but before coding any stuff I'll wait for the dbt-core teamChecklist