Skip to content
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

[Feature] Allow overriding dynamic variables in Unit Tests #10908

Open
3 tasks done
devmessias opened this issue Oct 23, 2024 · 0 comments
Open
3 tasks done

[Feature] Allow overriding dynamic variables in Unit Tests #10908

devmessias opened this issue Oct 23, 2024 · 0 comments
Labels
enhancement New feature or request triage

Comments

@devmessias
Copy link
Contributor

devmessias commented Oct 23, 2024

Is this your first time submitting a feature request?

  • I have read the expectations for open source contributors
  • I have searched the existing issues, and I could not find an existing issue for this feature
  • I am requesting a straightforward extension of existing dbt functionality, rather than a Big Idea better suited to a discussion

Describe the feature

Although this is a feature proposal, it helps address certain issues related to the limitations of macro overrides. As pointed out by @DmytroSly in this comment, when a macro is called multiple times with different arguments, macro overrides are not a viable solution. We also have other limitations in unit tests that blocks using them in models with more complex behaviors.

Within a Jinja template, when we have an assignment to a dynamic variable like this:

{%- set columns_a = get_columns_in_relation(ref('model_a')) -%} 
{% set column_names = columns_a | map(attribute='name') | reject("in", ['createdat','updatedat']) | list %}
{%- set columns_b = get_columns_in_relation(ref('model_b')) -%} 
-- code 

We could benefit from the ability to mock these dynamic variables (e.g., columns_a and columns_b) in unit tests.

unit_tests: 
  - name: my_other_unit_test 
    model: my_model_that_uses_macro 
    overrides: 
      dynamic_vars: 
        columns_a: ex1, ex2 
        columns_b: other1, other2	 

As mentioned earlier, this could also serve as a workaround for issues that currently require #8499 to be resolved, which is not be performant in simpler cases.

Describe alternatives you've considered

There are some simple ways to create this feature (I’ve already made some modifications in a branch to test two of these approaches).

Preferred Approach

Modify the Jinja MacroFuzzyParser and template in dbt-common to intercept assign calls and replace them with a constant Node.

The dbt-core unit test should allow receiving an overrides.dynamic_vars, which should be passed to the get_environment method that will create the Jinja environment with this attribute.

Second Approach: Not a Good Solution (I Tried This and Couldn't Make It Work Well)

  • Create a Jinja extension in dbt-core that defines a new assign method in the Jinja template, such as set_dynamic_var. This method will receive the dynamic_vars override object as an argument.

  • If the unit test has a dynamic_vars override, replace set <var> with set_dynamic_var.

Convert the model.sql to Jinja AST and Back to a Jinja Template

I look in the jinja issues and according to the maintainers there is no way to convert the Jinja AST back to a Jinja template. Therefore, although this was my initial approach, it would create too much work for just one small feature.

Who will this benefit?

  • If you want to test different scenarios in a model by enforcing different values for a dynamic variable.
  • If you can override a macro, but a more straightforward solution is to simply mock the variable, especially when the assignment expression is complex, for example:
    {% set column_names = columns|map(attribute='name')|reject("in", ['createdat', 'updatedat'])|list %}
    
  • If you cannot use unit tests due to limitations, but this could be resolved by just mocking the values.

Are you interested in contributing this feature?

Yes, and I've already tested the approach to help me, but I have more PRs with unit test fixes that I would like to have reviewed first (#10889, #10849), so that I don't have too many open stuff

Anything else?

No response

@devmessias devmessias added enhancement New feature or request triage labels Oct 23, 2024
@devmessias devmessias changed the title [Feature] Allow Overriding Dynamic Variables in Unit Tests [Feature] Allow Overriding dynamic variables in Unit Tests Oct 23, 2024
@devmessias devmessias changed the title [Feature] Allow Overriding dynamic variables in Unit Tests [Feature] Allow overriding dynamic variables in Unit Tests Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triage
Projects
None yet
Development

No branches or pull requests

1 participant