calogica / dbt-expectations

Port(ish) of Great Expectations to dbt test macros
https://calogica.github.io/dbt-expectations/
Apache License 2.0
1.04k stars 126 forks source link

[Feature Request] Enable description tag for tests #313

Open ChrisGutknecht opened 2 months ago

ChrisGutknecht commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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