-
Notifications
You must be signed in to change notification settings - Fork 148
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
Adds test for column presence #149
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.
Hi @rcaddell , thanks for this PR! I don't super understand the Fivetran use case, but I think the test makes sense and has applicability in general terms.
Couple of small code comments and a name change to get this over the finish line.
Also, we'll need to add a test in integration_tests/schema.yml
. Let me know if you need help with that and the integration tests in general.
macros/schema_tests/table_shape/expect_table_columns_not_to_contain_set.sql
Outdated
Show resolved
Hide resolved
macros/schema_tests/table_shape/expect_table_columns_not_to_contain_set.sql
Outdated
Show resolved
Hide resolved
macros/schema_tests/table_shape/expect_table_columns_not_to_contain_set.sql
Outdated
Show resolved
Hide resolved
name change and inner joined
Thanks @clausherther , I may need help with the integration tests. There is no data_test.csv file to seed when I run dbt seed from the integration_tests folder |
@rcaddell we don't use seeds, all test data comes from the models in the |
Hi @clausherther, I need some help with the integration testing if you could. I have an 'integration_tests' profile configured with needed access as per the readme. I added a test to schema.yml following the same pattern as the 'expect_table_columns_to_contain_set' entry, which errors with 2 results. I also receive an error in building the emails.sql model
Any guidance? Thanks! |
Hi @rcaddell let me check out your PR locally and give it a shot. I'll get back to you here by EOW. Sorry about the delay! |
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 wasn't able to commit changes to your PR for some reason. Is your fork private?
I fixed the issue with emails.sql
in main
, so could you please rebase your branch and try running the integration test again? At a minimum, you'll want a local postgres instance and run dbt build
. Thanks!
update test based on package maintainer feedback
Hi @clausherther, my fork is public from what I can see, possible my enterprise admin has a config set to block? Branch rebased and test updated based on your feedback. all tests passed. Thanks! |
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.
LGTM! Thanks!
…y_n_datepart` (#141) * implement exclusion_condition (#1) * move exclusion_condition to closing statement * Update README.md * Create expect_column_set_to_be_unique_case_insensitive.sql (#138) * Create expect_column_set_to_be_unique_case_insensitive.sql * adjusting consistent casing test and documentation * adjusting formatting and adding integration test * Fix formatting * Formatting updates * adjusting Readme and adding test instance * Fix spelling Co-authored-by: clausherther <[email protected]> * Change check for data date out of range to use timestamp (#145) * Update CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md * Added partition by in lag tests (#146) * Added partition by parameter * Added documentation * Modified the test to accept list instead of string * created data and implemented tests * corrected test in yml * Update formatting, test data and test macro Co-authored-by: clausherther <[email protected]> * Update CHANGELOG.md * Update CHANGELOG.md * Fix emails.sql (#153) * Adds test for column presence (#149) * add test for column not to exist * Update expect_table_columns_not_to_contain_set.sql * Update README.md * name change and inner joined * update readme * update test * Fix expect_row_values_to_have_recent_data issues on bigquery (#147) * Add datetime and timestamp test columns * Convert all evals to timestamps * Add datatypes * Update schema tests * Update CHANGELOG.md * Update README.md * Add pre-commit config (#156) * Add github actions workflow (#159) * Add github workflow * Remove pre-commit * Add .circleci/config.yml (#161) * Add .circleci/config.yml * Remove github action * Update postgres test * Add dbt install * Add env variables * Install dbt dependencies * Empty postgres password * Upgrade postgres image * Update connection settings * Fix postgres profile * Move profiles * Add BigQuery run * Fix job name * Update key path * Update config * Update schema * Add Snowflake tests * Update profile * Add back postgres * Show group_by columns in validation errors for column increasing test (#158) * Show group_by columns in validation errors for column increasing test * Add github actions workflow (#159) * Add github workflow * Remove pre-commit * Show group_by columns in validation errors for column increasing test * Add .circleci/config.yml (#161) * Add .circleci/config.yml * Remove github action * Update postgres test * Add dbt install * Add env variables * Install dbt dependencies * Empty postgres password * Upgrade postgres image * Update connection settings * Fix postgres profile * Move profiles * Add BigQuery run * Fix job name * Update key path * Update config * Update schema * Add Snowflake tests * Update profile * Add back postgres * Show group_by columns in validation errors for column increasing test * Jinja whitespace update (to force CI) Co-authored-by: Claus Herther <[email protected]> * implement exclusion_condition (#1) * move exclusion_condition to closing statement * Update README.md * Add integration test Co-authored-by: agusfigueroa-htg <[email protected]> Co-authored-by: clausherther <[email protected]> Co-authored-by: Josh Jones <[email protected]> Co-authored-by: Lucas Larbodiere <[email protected]> Co-authored-by: Randy Caddell <[email protected]> Co-authored-by: dluftspring <[email protected]>
Adds table shape test to look for the population of certain columns. Test is a copy of the expect_table_columns_to_contain_set with the final where clause changed
Specific use case is Servicenow data loaded via Fivetran. Fivetran will not add a _fivetran_deleted column to the Servicenow object unless and until at least 1 record from the source object was deleted. We've added this test to any Servicenow source that does not already have that column. This allows us to leave Fivetran set to bring in all new columns and only notify us if _fivetran_deleted was added and needs to be addressed in the dbt project. #148