-
Notifications
You must be signed in to change notification settings - Fork 163
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
[CT-1630] Convert column_types test for dbt-bigquery #476
Conversation
…st, dev_requirements pin changed
@@ -0,0 +1,34 @@ | |||
import pytest |
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.
Is there a reason to separate this from test_alter_column_types.py
? The two fixtures (module level variables) are copies of fixtures in the other file. So the only new content is the class with two methods.
I would keep the name test_column_types.py
as it's more generic, so I guess I'm saying to move the other file's contents here technically.
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.
These are separated for now as it is only currently tested in bigquery and is how it was historically, this does also raise a fine question of how big of a change in functionality a test is doing should they be before they are their own file/test but still grouped together.
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'd suggest just revising this to have the fixtures in the same file, then import them, thereby reducing the duplicate code. Best of both worlds.
Nit: do that
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.
There are a few very slight differences in the fixtures for each test, one easily fixed by a project_config_update in that we materialize alt model as a table, the second for the schema being a change in the int64 type
…y are not overly large additions
resolves #dbt-labs/dbt-core#6404
Description
convesion of 056 to new functional test method
todo before merge: revert dev_requirements dbt-core pin
Checklist
changie new
to create a changelog entry