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

[Tech Debt] Fix dynamic table tests #1085

Merged
merged 6 commits into from
Jun 17, 2024
Merged

Conversation

mikealfare
Copy link
Contributor

@mikealfare mikealfare commented Jun 14, 2024

resolves #1084

Problem

We were manually running some integration tests.

Solution

Update the tests so that they could run automatically.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • 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
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

@mikealfare mikealfare self-assigned this Jun 14, 2024
@cla-bot cla-bot bot added the cla:yes label Jun 14, 2024
Copy link
Contributor

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the dbt-snowflake contributing guide.

@@ -103,6 +95,9 @@ def setup(self, project, my_dynamic_table):
# the tests touch these files, store their contents in memory
initial_model = get_model_file(project, my_dynamic_table)

# verify the initial settings are correct in Snowflake
self.check_start_state(project, my_dynamic_table)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This used to be commented at the beginning of every test. It was moved here and will run automatically. So it looks like the comment was removed from each test, but it as effectively just uncommented.

"""
See above about the two commented assertions. In the meantime, these have been validated manually.
"""
# self.check_start_state(adapter, my_dynamic_table)
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 looks like this was deleted, but it was just uncommented. See my GH comment on line 99. This is true of all lines like this, but I'm leaving one comment for the sake of brevity.

self.change_config_via_alter(project, my_dynamic_table)
_, logs = run_dbt_and_capture(["--debug", "run", "--models", my_dynamic_table.name])

# self.check_state_alter_change_is_applied(adapter, my_dynamic_table)
# verify the updated settings are correct in Snowflake
self.check_state_alter_change_is_applied(project, my_dynamic_table)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're now checking in the database to verify the settings were applied. Even though that is a stronger check than checking the logs, we still want to check the logs so that we know how the change was made. If a change that was supposed to be applied via an alter is actually applied via a drop/create, that's not passing.

@mikealfare mikealfare marked this pull request as ready for review June 14, 2024 21:25
@mikealfare mikealfare requested a review from a team as a code owner June 14, 2024 21:25
@mikealfare mikealfare merged commit efe6df3 into main Jun 17, 2024
18 checks passed
@mikealfare mikealfare deleted the fix-dynamic-table-tests branch June 17, 2024 15:26
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.

[Tech Debt] Fix DT testing that should be automated
2 participants