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

Fix docs generation integration test #70

Merged
merged 4 commits into from
Feb 11, 2022

Conversation

VersusFacit
Copy link
Contributor

Resolves bug to an integration test introduced by merge of multiple unique ids for integration tests

Description

The test diffs a pair of configs, in which must be certain parameters. Changes to core have necessitated the addition of unique_id to two configs in this test.

Note:

Although this PR is small, it represents the same contextual work done with the matching Snowflake fix.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-snowflake next" section.

@cla-bot cla-bot bot added the cla:yes label Feb 10, 2022
@VersusFacit VersusFacit self-assigned this Feb 10, 2022
@VersusFacit VersusFacit changed the title Add unique_key field to config sections where it is now expected as a… Fix docs generation integration test Feb 10, 2022
@VersusFacit
Copy link
Contributor Author

VersusFacit commented Feb 10, 2022

@leahwicz Thanks for the pointer on consulting core merges as part of the debugging process. Got Redshift tests happy again with a touch of debugging following the read through of the Core PR! I'll be sure to follow that process in the future should "random" bugs arise

@@ -183,6 +183,7 @@ def assert_has_keys(
'invocation_id',
'modules',
'flags',
'print'
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this addition needed b/c of this PR dbt-labs/dbt-core#4701?

Copy link
Contributor

Choose a reason for hiding this comment

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

If so, I'm guessing the other adapters might need this update as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was. I'll merge this PR and look at the others

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doubled checked snowflake, bigquery, and spark. The unit tests of those are passing just fine! No additional fixes needed for this issue looks like. 😄

@VersusFacit VersusFacit merged commit 64f6f7b into main Feb 11, 2022
@VersusFacit VersusFacit deleted the CT-213/correct_docs_generation_int_test_catalogs branch February 11, 2022 08:34
abbywh pushed a commit to abbywh/dbt-redshift that referenced this pull request Oct 11, 2023
* Add unique_key field to config sections where it is now expected as a result of core PR 4618.

* Remove duplicate key.

* Fix unit test bug introduced by merge of dbt-core PR 4701.

Co-authored-by: Mila Page <versusfacit@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants