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

[CT-3460] [unit testing] Update unique id for data tests #9220

Open
1 task done
Tracked by #8283
emmyoop opened this issue Dec 5, 2023 · 3 comments
Open
1 task done
Tracked by #8283

[CT-3460] [unit testing] Update unique id for data tests #9220

emmyoop opened this issue Dec 5, 2023 · 3 comments
Assignees
Labels
Impact: CA unit tests Issues related to built-in dbt unit testing functionality user docs [docs.getdbt.com] Needs better documentation

Comments

@emmyoop
Copy link
Member

emmyoop commented Dec 5, 2023

Housekeeping

  • I am a maintainer of dbt-core

Short description

New state

With the addition of unit_tests, tests now include both unit_tests and data_tests.

Update the unique_id for data_tests to use a resource_type of data_test. The new format would be data_test.package.name

Current State

Historically, tests were exclusively data_tests so there was no need to distinguish them.

unique id for unit_tests: unit_test.package.name
unique id for data_tests: test.package.name

configs for data_tests are also defined as data_tests so it's confusing for the unique id to continue to use test.

Acceptance criteria

  • unique id for data tests is in the format: data_test.package.name
  • resource type for data_tests would be updated to data_test
  • updated manifest json schema
  • allow selection by data tests ie dbt ls resource-type data_test

Impact to Other Teams

Collaborate with CA team to determine best path.
Cloud Artifact would be affected since this would create a backwards incompatible change to the manifest

Will backports be required?

no

Context

This would make the manifest not backwards compatible. It would affect retry and defer when users upgrade. Manifest would possibly need to be rebuilt when upgrades happen.

Ensure tests for defer and state modified, don't run out of characters for test names (postgres)

Backwards compatibility

It's important that if we decide to rename these unique_ids that compatibility issues are handled. We have discussed separating out the various 'nodes' dictionary resources into separate dictionaries, and one possibility might be to rename test unique ids when we do that.

In addition, we are working on creating a cleaner separation between dbt Core and the artifacts definition, including ways to handle compatibility between different versions of artifacts. When that work is in place, it could be used to mitigate the compatibility issues by enabling conversions or by setting feature flags or using whatever mechanisms are developed.

@emmyoop emmyoop added the user docs [docs.getdbt.com] Needs better documentation label Dec 5, 2023
@github-actions github-actions bot changed the title [unit testing] Update unique id for data tests [CT-3460] [unit testing] Update unique id for data tests Dec 5, 2023
@dbeatty10
Copy link
Contributor

open question

  • allow selection by data tests ie dbt ls resource-type data_test

The impact of inconsistencies is that it makes the various commands harder to document, reason about, and use.

So for consistency, then it seems like we should allow all resource types here with dbt ls --resource-type unless we have a really good reason to exclude a particular resource type.

@emmyoop
Copy link
Member Author

emmyoop commented Dec 6, 2023

@dbeatty10 your comment may be a bigger, but related, ticket. Right now these are the allowed resource-types. Definitively a subset of what you linked above. I do agree and am of the opinion data_test, unit_test and test should allow be allowed as resource-types if we change the resource_type of from test -> data_tests as part of this ticket.

@graciegoheen
Copy link
Contributor

https://getdbt.slack.com/archives/CMZ2V0X8V/p1701820009435659?thread_ts=1701787446.634629&cid=CMZ2V0X8V

For deferral to work, sounds like we'd want to recommend folks run a dbt parse with the new version to defer to.

@dbeatty10 dbeatty10 added the unit tests Issues related to built-in dbt unit testing functionality label Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: CA unit tests Issues related to built-in dbt unit testing functionality user docs [docs.getdbt.com] Needs better documentation
Projects
None yet
Development

No branches or pull requests

4 participants