Create issue template for new dbt tests #423
Conversation
There was a problem hiding this comment.
This is a good start! I'm worried that we've gotten ahead of ourselves by attempting to write an issue template before drafting detailed dbt test docs, though; as you can tell from my comments, there's a lot of missing context, but I don't think an issue template is the appropriate way to provide that context. My suggestion would be for us to pause this effort, but up a separate PR to beef up the testing documentation to explain how our test suites work in detail, and then come back to this issue template once we can link out to our docs for necessary context. But I'm curious what @dfsnow thinks!
|
|
||
| ## Running Tests | ||
|
|
||
| Execute the tests against development or production environments using the following commands: |
There was a problem hiding this comment.
[Suggestion, non-blocking] I think we should decide whether we want folks to run development or production tests; otherwise, I expect readers will think that they should run both. I would lean in the direction of recommending that they just run development tests.
| 3. **Integrate Test with Data Model:** | ||
| - [ ] Include the new test in the `schema.yaml` file under the `dbt/models/` directory for the specific data model. | ||
| - Example: Here's how the test is implemented for the `default.vw_pin_universe` view: | ||
|
|
||
| ```yaml | ||
| - unique_combination_of_columns: | ||
| name: default_vw_pin_universe_unique_by_14_digit_pin_and_year | ||
| combination_of_columns: | ||
| - pin | ||
| - year | ||
| ``` |
There was a problem hiding this comment.
[Suggestion] There are a couple pieces of key information that are missing here:
- QC tests need either the test or the model to be tagged with a tag that starts with
test_qc_*(usuallytest_qc_iasworldfor an iasworld test) in order to run as part of our QC test suite - QC tests require a few extra properties in order to be properly displayed in the output workbook:
meta.description(required): Short description of the testmeta.category(optional): Category for the test, if the test is not configured with a default category in thetransform_dbt_test_resultsscriptmeta.table_name(optional): The iasWorld table name that the test operates on, if one cannot be automatically determined from the test
This is a lot of information, and I'm not sure whether it's better documented here in the issue template or elsewhere in our dbt documentation.
There was a problem hiding this comment.
I added a link out to the docs. You'll have to let me know what the right balance is.
| - year | ||
| ``` | ||
|
|
||
| - Reference code implementation [here](https://github.com/ccao-data/data-architecture/blob/66ad8159bcb3d96dcdc62b7355f8fbce64affc78/dbt/models/default/schema/default.vw_pin_universe.yml#L248-L252). |
There was a problem hiding this comment.
[Nitpick] I would actually prefer we link to an iasworld test, since tests defined on our view are much more likely to be unit tests and so require a different process. Any test on iasworld.dweldat should be fine.
| 1. **Check for Existing Tests:** Ensure the test or a similar functionality does not already exist among our generic templates. Consider modifying an existing template if it closely aligns with the required functionality. | ||
|
|
||
| 2. **Create Test Template:** | ||
| - [ ] Add a new test template in the `dbt/tests/generic/` directory. | ||
| - Example: View an existing DBT test template [here](https://github.com/ccao-data/data-architecture/blob/master/dbt/tests/generic/test_unique_combination_of_columns.sql). | ||
| - _We use [Jinja](https://jinja.palletsprojects.com/en/3.1.x/templates/) to parameterize the generic test templates._ |
There was a problem hiding this comment.
[Nitpick] I think we need some kind of conditional branching here to indicate the different steps the reader should take based on which of the following situations is true:
- A generic test exists that meets the reader's needs
- A generic test exists that almost meets the reader's needs, but must be extended to support their use case
- No generic test exists that meets the reader's needs
Currently these docs imply that the third bullet is the most important step, but I actually think the third bullet should only be a last resort, because writing a new generic requires advanced understanding of dbt.
There was a problem hiding this comment.
Ah right, because this issue could still be invoked to add an existing generic test to the schema.yaml for a given data model.
Co-authored-by: Jean Cochrane <jeancochrane@users.noreply.github.com>
Co-authored-by: Jean Cochrane <jeancochrane@users.noreply.github.com>
Co-authored-by: Jean Cochrane <jeancochrane@users.noreply.github.com>
I agree with this, better to document our various testing patterns in the dbt README. Could also be built on out #148. |
|
@wagnerlmichael What's the deal with this PR? Can we close it out or does it still need some work? |
|
I believe it still needs some work. I think I forgot to ping Jean after I made some changes. Let me take a look. @jeancochrane should unit tests be added to this doc? |
This PR adds an issue template for new DBT tests.