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: test config parity #3257

Merged
merged 4 commits into from
Apr 15, 2021
Merged

Feature: test config parity #3257

merged 4 commits into from
Apr 15, 2021

Conversation

jtcohen6
Copy link
Contributor

@jtcohen6 jtcohen6 commented Apr 13, 2021

resolves #3252
resolves #3253

This PR makes it possible to:

  • Disable schema tests where they're defined
  • Configure tests from dbt_project.yml

models/*.yml:

      tests:
        - not_null:
            enabled: true
        - accepted_values:
            values: [1,2,3]
            enabled: false

dbt_project.yml:

tests:
  my_project:
    +severity: warn
  installed_package:
    +enabled: false

Differences from proposal in #3253

  • Rather than configuring schema tests underneath each of models/sources/etc, all tests are configured under tests:. I think that's okay.
  • (After we solve v0.19.1 regression): Generic tests' default configs/arguments will override configurations set in dbt_project.yml. That's not the order I had originally envisioned, but again I think that's okay; it's up for debate which one is actually more generic / specific.

Next issues

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@cla-bot cla-bot bot added the cla:yes label Apr 13, 2021
@jtcohen6 jtcohen6 mentioned this pull request Apr 13, 2021
@jtcohen6 jtcohen6 marked this pull request as ready for review April 14, 2021 00:26
@jtcohen6 jtcohen6 requested a review from kwigley April 14, 2021 00:26
if isinstance(value, str):
value = get_rendered(value, render_ctx)
self.modifiers[key] = value
value = get_rendered(value, render_ctx, native=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah!

@gshank
Copy link
Contributor

gshank commented Apr 14, 2021

I'm wondering about the use of ResourceType.Test.... It applies in some cases to everything in a schema file, not just tests. I introduced a ParseFileType in the load_all_files branch. Perhaps we should straighten out the ambiguous use of 'tests' for all schema files things versus just tests.

@jtcohen6
Copy link
Contributor Author

@gshank Could you say more about that? I just did a quick search for all the places NodeType.Test crops up, and all of them seem to be specific to data tests + schema tests. Granted, there are some wonky things we do to construct schema tests while reading schema (non-project .yml) files...

@@ -303,15 +311,25 @@ def get_test_name(self) -> Tuple[str, str]:
name = '{}_{}'.format(self.namespace, name)
return get_nice_schema_test_name(name, self.target.name, self.args)

def construct_config(self) -> str:
configs = ",".join([
f"{key}=" + (f"'{value}'" if isinstance(value, str) else str(value))
Copy link
Contributor Author

@jtcohen6 jtcohen6 Apr 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kwigley This is actually the line that fixed the Undefined Jinja error, because I was accidentally passing the config down on line 329 as:

{{ config(severity = warn) }}

Instead of severity = 'warn', i.e. string literal. And, well, warn is not a defined Jinja variable.

At the same time, we can't quote values that need to be non-string types (namely, enabled is boolean). This is a pretty janky way of writing that, I'm sure there's a more pythonic / f-string-smart way to do it.

@jtcohen6 jtcohen6 merged commit cee0bfb into develop Apr 15, 2021
@jtcohen6 jtcohen6 deleted the feature/test-config-parity branch April 15, 2021 18:15
@jtcohen6 jtcohen6 mentioned this pull request May 8, 2021
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tests are configurable from dbt_project.yml Support enabled configuration for generic tests
3 participants