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.

Sign up for GitHub

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

Jump to bottom

Add regression test case #188

Merged
merged 22 commits into from
Jun 1, 2022
Merged

Add regression test case #188

merged 22 commits into from
Jun 1, 2022

Conversation

McKnight-42
Copy link
Contributor

@McKnight-42 McKnight-42 commented May 18, 2022 •
edited
Loading

#180

Description

checks for backwards compatibility in an edge case that database/schema information can be grabbed from default

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-bigquery next" section.

@McKnight-42
init push or pytest conversion of override_database for bigquery
e955ff4
@McKnight-42
tests are currently passing, need to implement some test changes over…
2ab362f 
…s (commented out tests)
@McKnight-42
Merge branch 'main' of https://github.com/dbt-labs/dbt-bigquery into …
331fafc 
…mcknight/convert_override_test
@McKnight-42
implemented a vew versions of the check_realations_equal test on test…
e3b42d8 
…_bigquery_database_override test working on getting the rest to work for first test
@McKnight-42
trying to grab local env to test against need to find way to grab inf…
ce4fde9 
…ormation from configs
@McKnight-42
attempting to move over to using check_relations_equal_with_relations
a1242e2
@McKnight-42
ordering of models
09bb316
@McKnight-42
Merge branch 'main' of https://github.com/dbt-labs/dbt-bigquery into …
4086d18 
…mcknight/convert_override_test
@McKnight-42
post pair update
ee6a7c3
@McKnight-42
Merge branch 'main' of https://github.com/dbt-labs/dbt-bigquery into …
3760c59 
…mcknight/convert_override_test
@McKnight-42
debugged test, had passing locally
f8e3f28
@McKnight-42
Merge branch 'main' of https://github.com/dbt-labs/dbt-bigquery into …
44db9ce 
…mcknight/convert_override_test
@McKnight-42
adding change to conftest to see if it passes tests in ci/cd
d523ce9
@McKnight-42
removing conftest change
0823e3b
@McKnight-42
trying ci/cd after some changes
4ff161c
@McKnight-42
trying to take into account CT-604, remove old test, remove BIGQUERY_…
513bde1 
…TEST_DATABASE env
@McKnight-42
moving override_database out of adapter folder up one
5fda361
@McKnight-42
Merge branch 'main' of https://github.com/dbt-labs/dbt-bigquery into …
7ab0b5e 
…mcknight/convert_override_test
@McKnight-42
adding regression test case draft
452cd3c
@cla-bot cla-bot bot added the cla:yes label May 18, 2022
jtcohen6
jtcohen6 reviewed May 18, 2022
View reviewed changes
Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

Looks good, nearly there! Remember you'll want to include the fix as well when backporting to 1.1.latest.

dbt-bigquery/dbt/adapters/bigquery/connections.py

Lines 558 to 560 in 8aaa6ab

# backwards compatibility: fill in with defaults if not specified
database = database or conn.credentials.database
schema = schema or conn.credentials.schema

tests/functional/test_get_columns_incomplete_database.py Outdated Show resolved Hide resolved
tests/functional/test_get_columns_incomplete_database.py Outdated Show resolved Hide resolved
tests/functional/test_get_columns_incomplete_database.py Outdated Show resolved Hide resolved
@McKnight-42 McKnight-42 self-assigned this May 19, 2022
@McKnight-42
Copy link
Contributor Author

McKnight-42 commented May 19, 2022 •
edited
Loading

@jtcohen6 made the suggested fixes any other extensions you think would be good for the test case? the two docs_generate tests that are failing will hopefully be fixed with gerda's PR. I know we aren't necessarily doing a changelog for the test conversions but does this regression test case deserve one?

jtcohen6
jtcohen6 approved these changes Jun 1, 2022
View reviewed changes
Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

Test looks great! Confirmed that it fails without the change here, and succeeds with it:

dbt-bigquery/dbt/adapters/bigquery/connections.py

Lines 558 to 560 in 69d1bf2

# backwards compatibility: fill in with defaults if not specified
database = database or conn.credentials.database
schema = schema or conn.credentials.schema

Just remember to pull in that change as well when backporting to 1.1.latest

@McKnight-42 McKnight-42 merged commit 799790e into main Jun 1, 2022
@McKnight-42 McKnight-42 deleted the mcknight/convert_override_test branch June 1, 2022 16:16
@McKnight-42 McKnight-42 mentioned this pull request Jun 1, 2022
Merged
4 tasks
McKnight-42 added a commit that referenced this pull request Jun 8, 2022
@McKnight-42
[Backport] #165/#188 (#197)
643352a 
* adding regression test case draft

* backporting #165 to 1.1.0 latest

* trying to take into account CT-604, remove old test, remove BIGQUERY_TEST_DATABASE env
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtcohen6 jtcohen6 jtcohen6 approved these changes

@VersusFacit VersusFacit Awaiting requested review from VersusFacit

@ChenyuLInx ChenyuLInx Awaiting requested review from ChenyuLInx

@leahwicz leahwicz Awaiting requested review from leahwicz

Assignees

@McKnight-42 McKnight-42

Labels
cla:yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@McKnight-42 @jtcohen6