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

Adds test for column presence #149

Merged
merged 10 commits into from
Mar 16, 2022
Merged

Conversation

rcaddell
Copy link
Contributor

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

Copy link
Contributor

@clausherther clausherther left a 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.

@rcaddell
Copy link
Contributor Author

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

@clausherther
Copy link
Contributor

@rcaddell we don't use seeds, all test data comes from the models in the integration_tests/models folder. Since there are so many tests, we try to test the test macros on a small number of test models. E.g. a single model with a number of columns can be used to test a large number of test macros, just as you would use them in practice. Have a look at the integration_tests/models folder to see what I mean. But happy to commit an example to your branch once you push the other changes. Thanks!

@rcaddell
Copy link
Contributor Author

rcaddell commented Mar 10, 2022 via email

@rcaddell
Copy link
Contributor Author

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

syntax error line 1 at position 30 unexpected '-'

Any guidance? Thanks!

@clausherther
Copy link
Contributor

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!

@clausherther clausherther self-requested a review March 15, 2022 23:11
Copy link
Contributor

@clausherther clausherther left a 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!

integration_tests/models/schema_tests/schema.yml Outdated Show resolved Hide resolved
@rcaddell
Copy link
Contributor Author

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!

Copy link
Contributor

@clausherther clausherther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks!

@clausherther clausherther merged commit f46776f into calogica:main Mar 16, 2022
clausherther added a commit that referenced this pull request Apr 4, 2022
…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 <claus@calogica.com>

* 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 <claus@calogica.com>

* 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 <claus@calogica.com>

* implement exclusion_condition (#1)

* move exclusion_condition to closing statement

* Update README.md

* Add integration test

Co-authored-by: agusfigueroa-htg <77272542+agusfigueroa-htg@users.noreply.github.com>
Co-authored-by: clausherther <claus@calogica.com>
Co-authored-by: Josh Jones <josh.jones@accenture.com>
Co-authored-by: Lucas Larbodiere <62683844+Lucasthenoob@users.noreply.github.com>
Co-authored-by: Randy Caddell <64412616+rcaddell@users.noreply.github.com>
Co-authored-by: dluftspring <daniel.luftspring@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants