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

Add tests on incremental materialization #37

Merged
merged 1 commit into from
Nov 8, 2021

Conversation

Kayrnt
Copy link
Contributor

@Kayrnt Kayrnt commented Nov 7, 2021

resolves dbt-labs/dbt-core#4144

Description

Following dbt-labs/dbt-core#4170, it makes sense to add a dedicated tests to avoid regression on that change for Redshift integration.

I didn't edited the changelog as it only affects the tests.

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

@cla-bot cla-bot bot added the cla:yes label Nov 7, 2021
@jtcohen6
Copy link
Contributor

jtcohen6 commented Nov 8, 2021

Closing and reopening to trigger integration test, now that ok to test label is added

@jtcohen6 jtcohen6 closed this Nov 8, 2021
@jtcohen6 jtcohen6 reopened this Nov 8, 2021
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 great @Kayrnt, just one comment.

Sorry the integration test is a bit more involved over in these plugins. We refactored the code for test_incremental_schema.py over in dbt-core, to keep it more narrowly focused, but that was after the adapter split-apart was complete.

There are some unrelated integration test failures (#36), we can ignore those for the purposes of this PR

@@ -91,6 +91,21 @@ def run_incremental_append_new_columns(self):
self.list_tests_and_assert(select, exclude, expected)
self.run_tests_and_assert(select, exclude, expected, compare_source, compare_target)

def run_incremental_append_new_columns_remove_one(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

This just needs to be added down below as well:

    @use_profile('redshift')
    def test__redshift__run_incremental_append_new_columns(self):
        self.run_incremental_append_new_columns()
        self.run_incremental_append_new_columns_remove_one()

@Kayrnt Kayrnt force-pushed the on_schema_change_columns_fix branch from 5056746 to 9ed3474 Compare November 8, 2021 10:15
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.

Failing tests are unrelated; the relevant test is passing. LGTM!

@jtcohen6 jtcohen6 merged commit 22fa8b9 into dbt-labs:main Nov 8, 2021
abbywh pushed a commit to abbywh/dbt-redshift that referenced this pull request Oct 11, 2023
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.

When on_schema_change is set, pass common columns as dest_columns in incremental merge macros
2 participants