Add detailed docs for dbt testing #432

jeancochrane · 2024-05-06T22:05:07Z

This PR updates our dbt documentation to included detailed docs on writing and running tests.

Connects #385.

…ction

…ults.py

jeancochrane · 2024-05-06T22:05:46Z

.github/scripts/transform_dbt_test_results.py

-CUSTOM_TEST_NAMES = {
+# Mapping that defines category names that should be reported for tests
+# based on their generics
+TEST_CATEGORIES = {


This variable previously had a confusing name, and I think TEST_CATEGORIES is clearer.

jeancochrane · 2024-05-06T22:06:10Z

.github/scripts/transform_dbt_test_results.py

-# Tests without a config.meta.category property will be grouped in
-# this default category
-DEFAULT_TEST_CATEGORY = "miscellaneous"


I haven't deleted this variable, just moved it below to be closer to the other category-definition constant.

jeancochrane · 2024-05-06T22:15:01Z

dbt/README.md

+[is coming soon](https://docs.getdbt.com/docs/build/unit-tests). Until unit
+tests are natively supported, however, we do not have a way of implementing them.


This is not entirely true -- we have a couple of unit tests so far, although they are implemented ad-hoc and without a clear framework for running them. I think we should leave this quirk undocumented for now, though, and revisit these docs (as well as our tests) once we implement a true unit test suite in #382.

Don't we kind of have a third set of tests outside the unit/data distinction? e.g.

data-architecture/dbt/models/reporting/schema.yml

Lines 5 to 15 in 48c7178

tables:

- name: ratio_stats

description: '{{ doc("table_ratio_stats") }}'

tests:

- expression_is_true:

name: reporting_ratio_stats_no_nulls

expression: |

year IS NOT NULL

AND triad IS NOT NULL

AND geography_type IS NOT NULL

AND property_group IS NOT NULL

That test is on a table that's downstream of transformations, but is not testing the transformation logic itself. So I suppose we'd count it as a unit test, but it doesn't actually match dot's upcoming unit test framework.

In my mind, these sorts of tests are still useful as they can catch logic errors when messing with transformations in models. They basically only get run when changing the view via a branch.

Edit: I am dumb and should have read further. These would be the "Non-QC test" defined below. IMO these might be worth breaking out into their own category, maybe "Model tests" or "Transformation tests". I feel like that's conceptually clearer, but open to ideas!

I think you're right, there are a few subtypes of "non-QC tests" that are all getting conflated in the current version of the docs. Still, I think we should probably just table this section of the docs for now and revisit it as part of #382 since I expect the way we write this kind of test will change once unit testing becomes available (hopefully this week 🤞🏻 ). Does that sound OK to you?

That sounds good! Can you just add a note in the issue that we need to clarify these distinctions in the docs once unit tests hit?

Good call, done!

jeancochrane · 2024-05-06T22:15:33Z

dbt/models/default/schema/default.vw_pin_universe.yml

+          - count_is_consistent:
+              name: default_vw_pin_universe_class_count_is_consistent_by_year
+              group_column: year
+              config:
+                error_if: ">25"


These tests are not new, I've simply moved them from table-level tests to the columns that they are operating on.

jeancochrane · 2024-05-06T22:16:14Z

dbt/models/iasworld/schema/iasworld.legdat.yml

@@ -343,7 +343,7 @@ sources:
                description: adrno should be <= 5 characters long
          - column_length:
              name: iasworld_legdat_adrsuf_zip1_taxdist_length_lte_5
-              columns:
+              column_names:


I've adjusted a few tests to standardize around the parameter names column_name and column_names where appropriate, so a few tests needed to be adjusted to reflect this change.

jeancochrane · 2024-05-06T22:17:26Z

dbt/tests/generic/README.md

+## Available generic tests
+
+- [`test_accepted_range`](#test_accepted_range)
+- [`test_accepted_values`](#test_accepted_values)
+- [`test_column_is_subset_of_external_column`](#test_column_is_subset_of_external_column)
+- [`test_column_length`](#test_column_length)
+- [`test_columns_match`](#test_columns_match)
+- [`test_count_is_consistent`](#test_count_is_consistent)
+- [`test_expression_is_true`](#test_expression_is_true)
+- [`test_is_null`](#test_is_null)
+- [`test_no_extra_whitespace`](#test_no_extra_whitespace)
+- [`test_not_accepted_values`](#test_not_accepted_values)
+- [`test_not_null`](#test_not_null)
+- [`test_relationships`](#test_relationships)
+- [`test_res_class_matches_pardat`](#test_res_class_matches_pardat)
+- [`test_row_count`](#test_row_count)
+- [`test_row_values_match_after_join`](#test_row_values_match_after_join)
+- [`test_sequential_values`](#test_sequential_values)
+- [`test_unique_combination_of_columns`](#test_unique_combination_of_columns)
+- [`test_value_is_present`](#test_value_is_present)


It would be nice if we could use some form of auto-documenter to generate this documentation based on the test docstrings, a la pkgdown. However, I think it's not worth spending too much time on that right now.

praise: This is awesome and a lot of work! Very nice job. My only suggestion is to make it more prominent in the docs, perhaps by linking it under the Table of Contents and within the dbt catalog docs.

I added a link to the README in ea8fe36! I'm not totally sure where it would fit in the data catalog docs, do you have a suggestion?

demand (blocking): This needs a better emoji before merge lol

Agreed, what do you think about 🧪?

Great, done in e3ba436!

dfsnow

This is awesome @jeancochrane! Nice work. See comments for some optional changes, otherwise this is good to go.

dfsnow · 2024-05-07T15:57:28Z

dbt/README.md

+[is coming soon](https://docs.getdbt.com/docs/build/unit-tests). Until unit
+tests are natively supported, however, we do not have a way of implementing them.


Don't we kind of have a third set of tests outside the unit/data distinction? e.g.

data-architecture/dbt/models/reporting/schema.yml

Lines 5 to 15 in 48c7178

tables:

- name: ratio_stats

description: '{{ doc("table_ratio_stats") }}'

tests:

- expression_is_true:

name: reporting_ratio_stats_no_nulls

expression: |

year IS NOT NULL

AND triad IS NOT NULL

AND geography_type IS NOT NULL

AND property_group IS NOT NULL

That test is on a table that's downstream of transformations, but is not testing the transformation logic itself. So I suppose we'd count it as a unit test, but it doesn't actually match dot's upcoming unit test framework.

In my mind, these sorts of tests are still useful as they can catch logic errors when messing with transformations in models. They basically only get run when changing the view via a branch.

Edit: I am dumb and should have read further. These would be the "Non-QC test" defined below. IMO these might be worth breaking out into their own category, maybe "Model tests" or "Transformation tests". I feel like that's conceptually clearer, but open to ideas!

dfsnow · 2024-05-07T16:13:51Z

dbt/tests/generic/README.md

+## Available generic tests
+
+- [`test_accepted_range`](#test_accepted_range)
+- [`test_accepted_values`](#test_accepted_values)
+- [`test_column_is_subset_of_external_column`](#test_column_is_subset_of_external_column)
+- [`test_column_length`](#test_column_length)
+- [`test_columns_match`](#test_columns_match)
+- [`test_count_is_consistent`](#test_count_is_consistent)
+- [`test_expression_is_true`](#test_expression_is_true)
+- [`test_is_null`](#test_is_null)
+- [`test_no_extra_whitespace`](#test_no_extra_whitespace)
+- [`test_not_accepted_values`](#test_not_accepted_values)
+- [`test_not_null`](#test_not_null)
+- [`test_relationships`](#test_relationships)
+- [`test_res_class_matches_pardat`](#test_res_class_matches_pardat)
+- [`test_row_count`](#test_row_count)
+- [`test_row_values_match_after_join`](#test_row_values_match_after_join)
+- [`test_sequential_values`](#test_sequential_values)
+- [`test_unique_combination_of_columns`](#test_unique_combination_of_columns)
+- [`test_value_is_present`](#test_value_is_present)


praise: This is awesome and a lot of work! Very nice job. My only suggestion is to make it more prominent in the docs, perhaps by linking it under the Table of Contents and within the dbt catalog docs.

jeancochrane and others added 14 commits May 2, 2024 14:15

Update README.md to extract dbt testing out into its own top-level se…

250d2e3

…ction

A bit of cleanup to testing docs in README.md

4018cb5

Update dbt/README.md to finish fleshing out testing docs

46c8aee

Rename CUSTOM_TEST_NAMES -> TEST_CATEGORIES in transform_dbt_test_res…

7b530f4

…ults.py

Add stub for dbt/tests/generic/README.md

42364cd

Sketch outline of dbt/tests/generic/README.md

edc32e5

Update dbt/README.md to add link to dbt/tests/generic/README.md

9a3a453

Flesh out some more docs in dbt/tests/generic/README.md

3831792

Finish fleshing out docs in dbt/tests/generic/README.md

fe92bbf

Update some tests to use more consistent attrs for column_names

6a204a7

Add a note on additional_select_columns to dbt/README.md

e31d1e9

Merge branch 'master' into jeancochrane/add-dbt-testing-docs

dacbbce

Move testing section of docs below model section

f031640

Fix a couple typos in dbt/README.md

5371707

jeancochrane commented May 6, 2024

View reviewed changes

jeancochrane marked this pull request as ready for review May 6, 2024 22:17

jeancochrane requested a review from a team as a code owner May 6, 2024 22:17

jeancochrane requested review from dfsnow and wagnerlmichael May 6, 2024 22:17

dfsnow approved these changes May 7, 2024

View reviewed changes

Update dbt/README.md to add a top-level link to generic test docs

ea8fe36

wagnerlmichael approved these changes May 7, 2024

View reviewed changes

jeancochrane mentioned this pull request May 7, 2024

Explicitly differentiate dbt unit tests from dbt data tests #382

Closed

Use better emoji for generic testing link in dbt/README.md

e3ba436

jeancochrane merged commit af87014 into master May 7, 2024
10 checks passed

jeancochrane deleted the jeancochrane/add-dbt-testing-docs branch May 7, 2024 18:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add detailed docs for dbt testing #432

Add detailed docs for dbt testing #432

jeancochrane commented May 6, 2024

jeancochrane May 6, 2024

jeancochrane May 6, 2024

jeancochrane May 6, 2024

dfsnow May 7, 2024

jeancochrane May 7, 2024

dfsnow May 7, 2024

jeancochrane May 7, 2024

jeancochrane May 6, 2024

jeancochrane May 6, 2024

jeancochrane May 6, 2024

dfsnow May 7, 2024

jeancochrane May 7, 2024

dfsnow May 7, 2024

jeancochrane May 7, 2024

dfsnow May 7, 2024

jeancochrane May 7, 2024

dfsnow left a comment

dfsnow May 7, 2024

dfsnow May 7, 2024

		[is coming soon](https://docs.getdbt.com/docs/build/unit-tests). Until unit
		tests are natively supported, however, we do not have a way of implementing them.

	tables:
	- name: ratio_stats
	description: '{{ doc("table_ratio_stats") }}'
	tests:
	- expression_is_true:
	name: reporting_ratio_stats_no_nulls
	expression: \|
	year IS NOT NULL
	AND triad IS NOT NULL
	AND geography_type IS NOT NULL
	AND property_group IS NOT NULL

Add detailed docs for dbt testing #432

Add detailed docs for dbt testing #432

Conversation

jeancochrane commented May 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dfsnow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment