-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: new unit tests for dbt models and macros #108
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't delved into the actual tests, just a few preliminary comments. Is there a doc that outlines the testing methodology you're using here, or is it all dbt built-ins?
@@ -0,0 +1,25 @@ | |||
name: Run all tests & checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we probably don't need this, as we're running dbt test here:
https://github.com/openedx/aspects-dbt/blob/main/.github/workflows/coverage.yml#L47
Since dbt needs ClickHouse running, and alembic to have been run, we're just doing these tests in the Tutor / Aspects context there.
@@ -14,7 +14,7 @@ profile: "aspects" | |||
model-paths: ["models"] | |||
analysis-paths: ["analyses"] | |||
test-paths: ["tests"] | |||
seed-paths: ["seeds"] | |||
seed-paths: ["seeds","seeds-test"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one I'm not sure how we would shoehorn into the Aspects context, we obviously don't want these seeds loaded outside of CI. We may need to find / add a way to override the dbt_project.yml in Aspects. 🤔
@@ -0,0 +1,53 @@ | |||
{% macro drop_stale_tables(dryrun=False) %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this for? :) Looks like it's not being used yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should get rid of this one. You should be able to add .DS_Store
to ~/.gitignore
to remove them globally. In VS Code you'd need to turn on 2 options for it to recognize the global file: "Search: Use Global Ignore Files" and "Search: Use Ignore Files", I think that's what fixed those files getting auto-added for me.
Part 1 of new unit tests
still using dev dbt-clickhouse version - need to update before deploying