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 integration tests #3953

Merged
merged 4 commits into from
Sep 29, 2021
Merged

Conversation

kadero
Copy link
Contributor

@kadero kadero commented Sep 26, 2021

resolves #3952

Description

This PR aims to handle properly the datetime column in the integration test TestSnapshotHardDelete.test__postgres__snapshot_hard_delete.

The timestamp we extract from PG is in UTC and offset naive. We just need to make it offset aware to enable the comparaison.
astimezone(pytz.UTC) to .replace(tzinfo=pytz.UTC)

The fix was tested locally by launching the test under multiple Timezone:

# EDT
sudo timedatectl set-timezone America/New_York && timedatectl && python -m pytest -m profile_postgres test/integration/004_simple_snapshot_test/test_simple_snapshot.py::TestSnapshotHardDelete::test__postgres__snapshot_hard_delete

# PDT
sudo timedatectl set-timezone America/Los_Angeles && timedatectl && python -m pytest -m profile_postgres test/integration/004_simple_snapshot_test/test_simple_snapshot.py::TestSnapshotHardDelete::test__postgres__snapshot_hard_delete

# CEST
sudo timedatectl set-timezone Europe/Paris && timedatectl && python -m pytest -m profile_postgres test/integration/004_simple_snapshot_test/test_simple_snapshot.py::TestSnapshotHardDelete::test__postgres__snapshot_hard_delete

# NZDT
sudo timedatectl set-timezone Pacific/Auckland && timedatectl && python -m pytest -m profile_postgres test/integration/004_simple_snapshot_test/test_simple_snapshot.py::TestSnapshotHardDelete::test__postgres__snapshot_hard_delete

# IST
sudo timedatectl set-timezone Asia/Kolkata && timedatectl && python -m pytest -m profile_postgres test/integration/004_simple_snapshot_test/test_simple_snapshot.py::TestSnapshotHardDelete::test__postgres__snapshot_hard_delete

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

@cla-bot
Copy link

cla-bot bot commented Sep 26, 2021

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @kadero

@kadero kadero closed this Sep 26, 2021
@kadero kadero reopened this Sep 26, 2021
@kadero
Copy link
Contributor Author

kadero commented Sep 26, 2021

CLA signed ✔️

@kadero kadero marked this pull request as ready for review September 26, 2021 12:45
@jtcohen6
Copy link
Contributor

Closing and reopening to trigger adapter tests

@jtcohen6 jtcohen6 closed this Sep 26, 2021
@jtcohen6 jtcohen6 reopened this Sep 26, 2021
@jtcohen6
Copy link
Contributor

@cla-bot check

@cla-bot cla-bot bot added the cla:yes label Sep 26, 2021
@cla-bot
Copy link

cla-bot bot commented Sep 26, 2021

The cla-bot has been summoned, and re-checked this pull request!

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.

@kadero Thanks for this! There must have been something else going on in the previous issue (perhaps a non-UTC timestamp setting in Postgres?).

Since this is just a change to the integration test, this doesn't require a changelog entry, but if you're up for it, I'd still encourage you to add yourself to the list of Contributors there.

@kadero
Copy link
Contributor Author

kadero commented Sep 26, 2021

@kadero Thanks for this! There must have been something else going on in the previous issue (perhaps a non-UTC timestamp setting in Postgres?).

Yes probably, I not 100% sure of the root cause in the previous issue 😕

Since this is just a change to the integration test, this doesn't require a changelog entry, but if you're up for it, I'd still encourage you to add yourself to the list of Contributors there.

Done under ## dbt 0.21.0 (Release TBD) ✔️

Many thanks for your assist @jtcohen6 🙏

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.

Thanks for this @kadero! I just bumped up your contributor note to be under 1.0.0 :)

@jtcohen6 jtcohen6 merged commit c25b7a1 into dbt-labs:develop Sep 29, 2021
jtcohen6 added a commit to dbt-labs/dbt-redshift that referenced this pull request Oct 2, 2021
jtcohen6 added a commit to dbt-labs/dbt-redshift that referenced this pull request Oct 2, 2021
jtcohen6 added a commit to dbt-labs/dbt-redshift that referenced this pull request Oct 2, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

One Integration test broken locally TestSnapshotHardDelete.test__postgres__snapshot_hard_delete
2 participants