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 Request] Enable description tag for tests #313

Open
ChrisGutknecht opened this issue Jul 8, 2024 · 5 comments
Open

[Feature Request] Enable description tag for tests #313

ChrisGutknecht opened this issue Jul 8, 2024 · 5 comments

Comments

@ChrisGutknecht
Copy link

Is your feature request related to a problem? Please describe.
dbt expectations are often more specific and complex than dbt generic tests. While dbt generic tests allow a description field for tests, dbt expectations tests don't allow this tag.

Describe the solution you'd like
A dbt expectations test should support a test tag. This would allow the text to be populated in the description section of dbt explore and the docs generate artifacts.

Describe alternatives you've considered
Enriching the model descriptions is not ideal, custom tags or meta flags are not as visible in dbt explore.

Additional context
Test descriptions help easier debugging and evaluating test failures across the entire teams with much more context.

@clausherther
Copy link
Contributor

Hi @ChrisGutknecht, thanks for pointing this out. I'm not familiar with the test tag feature. How would we implement this for our tests?

@ChrisGutknecht
Copy link
Author

ChrisGutknecht commented Jul 8, 2024

Thanks @clausherther.

Generic tests can be enhanced like this with a description, which can add important business context:

columns:
      - name: order_id_awin
        description: the order id generated by Awin, not the ishop identifier. Used to directly amend the order
        meta: 
          primary-key: true
        tests:
          - not_null: 
              description: >
                Every Awin order should have an Awin-based identifier. 
                If not, it is a rare edge case or might be a manual import.

Currently we can only enhance dbt expectation tests like this with code comments, see # :

    - name: sale_amount_awin
      description: the net amount after return adjustments or cancellations
      tests:
        - not_null
        # alert sale amount values below 1
        - dbt_expectations.expect_column_values_to_be_between:
            min_value: 1
            row_condition: "awin_date between date_sub(current_date(), interval 7 day) and date_sub(current_date(), interval 1 day)" # (Optional)
            config:
              severity: warn
              warn_if: ">5"
        # alert very high sale amount values, which might indicate fraud
        - dbt_expectations.expect_column_values_to_be_between:
            max_value: 5000 # (Optional)
            row_condition: "awin_date between date_sub(current_date(), interval 3 day) and date_sub(current_date(), interval 1 day)" # (Optional)
            config:
              severity: warn

Currently, the important "why" context is added in comments, but these comments can not be surfaced beyond the code layer such as dbt explore. Ideally we could add a description to every dbt expectation test:

    - name: sale_amount_awin
      description: the net amount after return adjustments or cancellations
      tests:
        - not_null
        - dbt_expectations.expect_column_values_to_be_between:
            description: alert sale amount values below 1
            min_value: 1
            row_condition: "awin_date between date_sub(current_date(), interval 7 day) and date_sub(current_date(), interval 1 day)" # (Optional)
            config:
              severity: warn
              warn_if: ">5"
        
        - dbt_expectations.expect_column_values_to_be_between:
            description: alert very high sale amount values, which might indicate fraud
            max_value: 5000 # (Optional)
            row_condition: "awin_date between date_sub(current_date(), interval 3 day) and date_sub(current_date(), interval 1 day)" # (Optional)
            config:
              severity: warn

Does that make it more clear?

@clausherther
Copy link
Contributor

That makes sense re: usage, but I'm not sure how that would be implemented in each test. Does that mean each test has to take a description param and somehow pass that to dbt?

@clausherther
Copy link
Contributor

e.g. I don't see dbt_utils implementing anything like that?
https://github.com/dbt-labs/dbt-utils/blob/main/macros/generic_tests/unique_combination_of_columns.sql

@ChrisGutknecht
Copy link
Author

Yes, that's true. Unfortunately, as I just realized, dbt generic tests allow a description field in the compile stage, but it throws an error in the dbt test stage. My bad. I will have to check if there's a dbt core issue for this. The weird thing is that the dbt explore section in Cloud has a very visible description field for every test, but does not actually support it.

At least for us, an optional description flag would be helpful to add more business context.

We currently forward these test tables to microsoft teams for daily test resolution, but the long names are hard to read.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants