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

fix: Upgrade tf for testing #625

Merged
merged 13 commits into from
Jul 30, 2021
Merged

fix: Upgrade tf for testing #625

merged 13 commits into from
Jul 30, 2021

Conversation

alldoami
Copy link
Contributor

To address this problem: #622 (comment)

Upgrading our tf version for testing!

@alldoami alldoami requested a review from a team as a code owner July 29, 2021 17:26
@alldoami alldoami requested a review from edulop91 July 29, 2021 17:26
edulop91
edulop91 reviewed Jul 29, 2021
View reviewed changes
Makefile Outdated
@@ -4,7 +4,7 @@ export DIRTY=$(shell if `git diff-index --quiet HEAD --`; then echo false; else
LDFLAGS=-ldflags "-w -s -X github.com/chanzuckerberg/go-misc/ver.GitSha=${SHA} -X github.com/chanzuckerberg/go-misc/ver.Version=${VERSION} -X github.com/chanzuckerberg/go-misc/ver.Dirty=${DIRTY}"
export BASE_BINARY_NAME=terraform-provider-snowflake_v$(VERSION)
export GO111MODULE=on
export TF_ACC_TERRAFORM_VERSION=0.12.29
export TF_ACC_TERRAFORM_VERSION=0.13
Copy link

Choose a reason for hiding this comment

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

maybe add the readme that there's now a version requirement for the provider

Copy link
Contributor

Choose a reason for hiding this comment

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

@alldoami @edulop91 I think this is failing because it should be 0.13.x in this case 0.13.0 not only 0.13

@github-actions
Copy link

Integration tests failure for 82ac5bf9a68c14dfc265ad0d1408afe5b65488aa

@github-actions
Copy link

Integration tests failure for 8b6bfa1657f37beb879e5bb44acf1b6d6f0b9d69

@github-actions
Copy link

Integration tests failure for aaa8cb749dfc1a3659b379b3bb2bd85469e8f7b5

@github-actions
Copy link

Integration tests failure for 622c3266841d2249b88dc5ace38d11f57febe2ba

@github-actions
Copy link

Integration tests failure for ebb83498ef076bbfea1d0a652789b0022237fe09

@github-actions
Copy link

Integration tests failure for b6f3596dc4b4144e6e227b870d26bb02559f61e3

@github-actions
Copy link

Integration tests failure for 529c23cc8dde688334b2e912cc45c68ea599d680

jakeyheath and others added 4 commits July 29, 2021 15:57
@github-actions
Copy link

Integration tests failure for f17f84fbd5bb0b92cc9d24ba7bbb579cd580c88d

@github-actions
Copy link

Integration tests failure for b0b12d58b82af08f7d3a229b31ed7132339e70b3

@github-actions
Copy link

Integration tests failure for ac2e13e909120bbf4bc752fa02fbf9278d3d9ec3

@github-actions
Copy link

Integration tests failure for 0b54ac4d56d056c146ab0c30fc0d66ee47b8cd1b

@ChrisIsidora
Copy link
Contributor

ChrisIsidora commented Jul 30, 2021 •
edited
Loading

@alldoami I couldn't clone your branch, I upgraded locally and figured it out to fix the remaining failing tests:

For SCIM integration test failures:

  • Somehow because of the change in the behavior of datasources in 0.13 and the previous failed tests the role AAD_PROVISIONER was never destroyed. It should be dropped in the Acceptance Tests Snowflake Account manually. The reason it's not a random role name is because snowflake only expects a set of roles with a specific name

For the current datasources the behavior has changed with 0.13 so to pass these tests the direct dependency should be added:

  • materialized_views_acceptance_test.go => Add depends_on = [snowflake_materialized_view.v] in the data block
  • schemas_acceptance_test.go => Add depends_on = [snowflake_schema.s] in the data block
  • tables_acceptance_test.go => Add depends_on = [snowflake_table.t] in the data block
  • views_acceptance_test.go => Add depends_on = [snowflake_view.v] in the data block
  • system_generate_scim_access_token_acceptance_test.go => Add depends_on = [snowflake_scim_integration.azured] in the data block

For Storage Integration error message format has changed in 0.13

  • storage_integration_acceptance_test.go in the TestAccStorageIntegration_validation => Change ExpectedError to regexp.MustCompile("1 item minimum, but config has only 0 declared")

@github-actions
Copy link

Integration tests success for 4b1b863e7533da2651643dbec574f9ecc0e62162

@alldoami alldoami merged commit c03656f into main Jul 30, 2021
@alldoami alldoami deleted the adoami/upgrade-tf-for-testing branch July 30, 2021 17:38
jtzero pushed a commit to rxrevu/terraform-provider-snowflake that referenced this pull request Aug 19, 2021
@alldoami @jtzero
fix: Upgrade tf for testing (Snowflake-Labs#625)
4321564
anton-chekanov pushed a commit to anton-chekanov/terraform-provider-snowflake that referenced this pull request Jan 25, 2022
@alldoami @anton-chekanov
fix: Upgrade tf for testing (Snowflake-Labs#625)
84f77a1
daniepett pushed a commit to daniepett/terraform-provider-snowflake that referenced this pull request Feb 9, 2022
@alldoami
fix: Upgrade tf for testing (Snowflake-Labs#625)
b2fdd33
@snowflake-release-please snowflake-release-please bot mentioned this pull request May 25, 2022
Merged
This was referenced Nov 1, 2022
Closed
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrisIsidora ChrisIsidora ChrisIsidora left review comments

@edulop91 edulop91 edulop91 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alldoami @ChrisIsidora @edulop91 @jakeyheath