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

Upgrade to go 1.13 #372

Merged
merged 7 commits into from
Mar 5, 2020
Merged

Upgrade to go 1.13 #372

merged 7 commits into from
Mar 5, 2020

Conversation

martinssipenko
Copy link
Contributor

@martinssipenko martinssipenko commented Mar 1, 2020 •
edited
Loading

I started this work in order to make an upgrade to Go 1.13, but ended up doing some more things as they were required to make the 1.13 upgrade possible.

  • I've removed the use of terraform TLS provider and instead hardcoded an SSH public key in tests. This was required because it was impossible to upgrade to module version 2, due to the fact that the module does not follow Go Semantic Import Versioning, there is a ticket in the TLS module to fix this - Go module versioning needs fixed hashicorp/terraform-provider-tls#67
  • I've replaced the github.com/hashicorp/terraform dependency with github.com/hashicorp/terraform-plugin-sdk.
  • I've created a new self-signed certificate (old one expired on 7 Aug 2019, 2:06 p.m.), as I believe many acceptance cases were failing due to this, because this TLS cert is used when starting a mock server locally. I've purposely set far certificate expiration to prevent this happening again in next 10 years :)
  • Cleaned up Makefile by adding vet and removing missing errcheck
  • Fixed failing acceptance tests.

Fixes #370 #350

@ghost ghost added the size/S label Mar 1, 2020
@martinssipenko martinssipenko marked this pull request as ready for review March 1, 2020 12:25
@martinssipenko martinssipenko changed the title Gh 370 go1.13 WIP: Upgrade to go 1.13 Mar 2, 2020
@ghost ghost added size/XXL and removed size/S labels Mar 2, 2020
@martinssipenko
Copy link
Contributor Author

Acc test output:

$ make testacc 
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v  -timeout 120m
?       github.com/terraform-providers/terraform-provider-github    [no test files]
=== RUN   TestAccGithubCollaboratorsDataSource_basic
=== PAUSE TestAccGithubCollaboratorsDataSource_basic
=== RUN   TestAccGithubIpRangesDataSource_existing
=== PAUSE TestAccGithubIpRangesDataSource_existing
=== RUN   TestAccGithubReleaseDataSource_fetchByLatestNoReleaseReturnsError
=== PAUSE TestAccGithubReleaseDataSource_fetchByLatestNoReleaseReturnsError
=== RUN   TestAccGithubReleaseDataSource_latestExisting
=== PAUSE TestAccGithubReleaseDataSource_latestExisting
=== RUN   TestAccGithubReleaseDataSource_fetchByIdWithNoIdReturnsError
=== PAUSE TestAccGithubReleaseDataSource_fetchByIdWithNoIdReturnsError
=== RUN   TestAccGithubReleaseDataSource_fetchByIdExisting
=== PAUSE TestAccGithubReleaseDataSource_fetchByIdExisting
=== RUN   TestAccGithubReleaseDataSource_fetchByTagNoTagReturnsError
=== PAUSE TestAccGithubReleaseDataSource_fetchByTagNoTagReturnsError
=== RUN   TestAccGithubReleaseDataSource_fetchByTagExisting
=== PAUSE TestAccGithubReleaseDataSource_fetchByTagExisting
=== RUN   TestAccGithubReleaseDataSource_invalidRetrieveMethodReturnsError
=== PAUSE TestAccGithubReleaseDataSource_invalidRetrieveMethodReturnsError
=== RUN   TestAccGithubRepositoriesDataSource_basic
=== PAUSE TestAccGithubRepositoriesDataSource_basic
=== RUN   TestAccGithubRepositoriesDataSource_Sort
=== PAUSE TestAccGithubRepositoriesDataSource_Sort
=== RUN   TestAccGithubRepositoriesDataSource_noMatch
=== PAUSE TestAccGithubRepositoriesDataSource_noMatch
=== RUN   TestAccGithubRepositoryDataSource_fullName_noMatchReturnsError
=== PAUSE TestAccGithubRepositoryDataSource_fullName_noMatchReturnsError
=== RUN   TestAccGithubRepositoryDataSource_name_noMatchReturnsError
=== PAUSE TestAccGithubRepositoryDataSource_name_noMatchReturnsError
=== RUN   TestAccGithubRepositoryDataSource_fullName_existing
=== PAUSE TestAccGithubRepositoryDataSource_fullName_existing
=== RUN   TestAccGithubRepositoryDataSource_name_existing
=== PAUSE TestAccGithubRepositoryDataSource_name_existing
=== RUN   TestAccGithubTeamDataSource_noMatchReturnsError
=== PAUSE TestAccGithubTeamDataSource_noMatchReturnsError
=== RUN   TestAccGithubUserDataSource_noMatchReturnsError
=== PAUSE TestAccGithubUserDataSource_noMatchReturnsError
=== RUN   TestAccGithubUserDataSource_existing
=== PAUSE TestAccGithubUserDataSource_existing
=== RUN   TestMigrateGithubWebhookStateV0toV1
2020/03/02 12:45:35 [DEBUG] GitHub Webhook Attributes before migration: map[string]string{"configuration.%":"4", "configuration.content_type":"form", "configuration.insecure_ssl":"0", "configuration.secret":"blablah", "configuration.url":"https://google.co.uk/"}
2020/03/02 12:45:35 [DEBUG] GitHub Webhook Attributes after State Migration: map[string]string{"configuration.#":"1", "configuration.0.content_type":"form", "configuration.0.insecure_ssl":"0", "configuration.0.secret":"blablah", "configuration.0.url":"https://google.co.uk/"}
--- PASS: TestMigrateGithubWebhookStateV0toV1 (0.00s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestProvider_individual
--- PASS: TestProvider_individual (1.71s)
=== RUN   TestProvider_anonymous
--- PASS: TestProvider_anonymous (1.34s)
=== RUN   TestProvider_insecure
--- PASS: TestProvider_insecure (0.12s)
=== RUN   TestAccGithubBranchProtection_basic
=== PAUSE TestAccGithubBranchProtection_basic
=== RUN   TestAccGithubBranchProtection_users
=== PAUSE TestAccGithubBranchProtection_users
=== RUN   TestAccGithubBranchProtection_teams
=== PAUSE TestAccGithubBranchProtection_teams
=== RUN   TestAccGithubBranchProtection_emptyItems
=== PAUSE TestAccGithubBranchProtection_emptyItems
=== RUN   TestAccGithubIssueLabel_basic
=== PAUSE TestAccGithubIssueLabel_basic
=== RUN   TestAccGithubIssueLabel_existingLabel
=== PAUSE TestAccGithubIssueLabel_existingLabel
=== RUN   TestAccGithubIssueLabel_description
=== PAUSE TestAccGithubIssueLabel_description
=== RUN   TestAccGithubMembership_basic
=== PAUSE TestAccGithubMembership_basic
=== RUN   TestAccGithubMembership_caseInsensitive
--- PASS: TestAccGithubMembership_caseInsensitive (7.36s)
=== RUN   TestAccGithubOrganizationProject_basic
=== PAUSE TestAccGithubOrganizationProject_basic
=== RUN   TestAccGithubOrganizationWebhook_basic
=== PAUSE TestAccGithubOrganizationWebhook_basic
=== RUN   TestAccGithubOrganizationWebhook_secret
=== PAUSE TestAccGithubOrganizationWebhook_secret
=== RUN   TestAccGithubProjectColumn_basic
=== PAUSE TestAccGithubProjectColumn_basic
=== RUN   TestAccGithubRepositoryCollaborator_basic
=== PAUSE TestAccGithubRepositoryCollaborator_basic
=== RUN   TestAccGithubRepositoryCollaborator_caseInsensitive
=== PAUSE TestAccGithubRepositoryCollaborator_caseInsensitive
=== RUN   TestSuppressDeployKeyDiff
--- PASS: TestSuppressDeployKeyDiff (0.00s)
=== RUN   TestAccGithubRepositoryDeployKey_basic
=== PAUSE TestAccGithubRepositoryDeployKey_basic
=== RUN   TestAccGithubRepositoryFile_basic
--- PASS: TestAccGithubRepositoryFile_basic (22.99s)
=== RUN   TestAccGithubRepositoryFile_branch
--- PASS: TestAccGithubRepositoryFile_branch (21.10s)
=== RUN   TestAccGithubRepositoryFile_committer
--- PASS: TestAccGithubRepositoryFile_committer (21.10s)
=== RUN   TestAccGithubRepositoryProject_basic
=== PAUSE TestAccGithubRepositoryProject_basic
=== RUN   TestAccGithubRepository_basic
=== PAUSE TestAccGithubRepository_basic
=== RUN   TestAccGithubRepository_archive
=== PAUSE TestAccGithubRepository_archive
=== RUN   TestAccGithubRepository_archiveUpdate
=== PAUSE TestAccGithubRepository_archiveUpdate
=== RUN   TestAccGithubRepository_hasProjects
=== PAUSE TestAccGithubRepository_hasProjects
=== RUN   TestAccGithubRepository_defaultBranch
=== PAUSE TestAccGithubRepository_defaultBranch
=== RUN   TestAccGithubRepository_templates
=== PAUSE TestAccGithubRepository_templates
=== RUN   TestAccGithubRepository_topics
=== PAUSE TestAccGithubRepository_topics
=== RUN   TestAccGithubRepository_autoInitForceNew
=== PAUSE TestAccGithubRepository_autoInitForceNew
=== RUN   TestAccGithubRepository_createFromTemplate
=== PAUSE TestAccGithubRepository_createFromTemplate
=== RUN   TestAccGithubRepositoryWebhook_basic
=== PAUSE TestAccGithubRepositoryWebhook_basic
=== RUN   TestAccGithubRepositoryWebhook_secret
=== PAUSE TestAccGithubRepositoryWebhook_secret
=== RUN   TestAccGithubTeamMembership_basic
=== PAUSE TestAccGithubTeamMembership_basic
=== RUN   TestAccGithubTeamMembership_caseInsensitive
--- PASS: TestAccGithubTeamMembership_caseInsensitive (22.77s)
=== RUN   TestAccGithubTeamRepository_basic
=== PAUSE TestAccGithubTeamRepository_basic
=== RUN   TestAccCheckGetPermissions
--- PASS: TestAccCheckGetPermissions (0.00s)
=== RUN   TestAccGithubTeam_basic
=== PAUSE TestAccGithubTeam_basic
=== RUN   TestAccGithubTeam_slug
=== PAUSE TestAccGithubTeam_slug
=== RUN   TestAccGithubTeam_hierarchical
=== PAUSE TestAccGithubTeam_hierarchical
=== RUN   TestAccGithubUserGpgKey_basic
=== PAUSE TestAccGithubUserGpgKey_basic
=== RUN   TestAccGithubUserInvitationAccepter_basic
--- SKIP: TestAccGithubUserInvitationAccepter_basic (0.00s)
    resource_github_user_invitation_accepter_test.go:21: GITHUB_TEST_COLLABORATOR_TOKEN was not provided, skipping test
=== RUN   TestAccGithubUserSshKey_basic
=== PAUSE TestAccGithubUserSshKey_basic
=== RUN   TestAccOrganizationBlock_basic
=== PAUSE TestAccOrganizationBlock_basic
=== RUN   TestEtagTransport
--- PASS: TestEtagTransport (0.00s)
=== RUN   TestRateLimitTransport_abuseLimit_get
--- PASS: TestRateLimitTransport_abuseLimit_get (0.00s)
=== RUN   TestRateLimitTransport_abuseLimit_post
--- PASS: TestRateLimitTransport_abuseLimit_post (0.00s)
=== RUN   TestRateLimitTransport_abuseLimit_post_error
--- PASS: TestRateLimitTransport_abuseLimit_post_error (0.00s)
=== RUN   TestAccValidateTeamIDFunc
--- PASS: TestAccValidateTeamIDFunc (0.00s)
=== RUN   TestAccGithubUtilRole_validation
--- PASS: TestAccGithubUtilRole_validation (0.00s)
=== RUN   TestAccGithubUtilTwoPartID
--- PASS: TestAccGithubUtilTwoPartID (0.00s)
=== CONT  TestAccGithubCollaboratorsDataSource_basic
=== CONT  TestAccGithubOrganizationWebhook_basic
=== CONT  TestAccGithubRepository_topics
=== CONT  TestAccGithubRepository_autoInitForceNew
--- PASS: TestAccGithubCollaboratorsDataSource_basic (28.42s)
=== CONT  TestAccGithubRepository_templates
--- PASS: TestAccGithubOrganizationWebhook_basic (51.26s)
=== CONT  TestAccGithubRepository_defaultBranch
--- PASS: TestAccGithubRepository_templates (36.50s)
=== CONT  TestAccGithubRepository_hasProjects
--- PASS: TestAccGithubRepository_topics (84.74s)
=== CONT  TestAccGithubRepository_archiveUpdate
--- PASS: TestAccGithubRepository_hasProjects (23.09s)
=== CONT  TestAccGithubRepository_archive
--- PASS: TestAccGithubRepository_autoInitForceNew (96.41s)
=== CONT  TestAccGithubRepository_basic
--- PASS: TestAccGithubRepository_defaultBranch (52.81s)
=== CONT  TestAccGithubRepositoryProject_basic
--- PASS: TestAccGithubRepository_archive (32.66s)
=== CONT  TestAccGithubRepositoryDeployKey_basic
--- PASS: TestAccGithubRepository_archiveUpdate (45.95s)
=== CONT  TestAccGithubRepositoryCollaborator_caseInsensitive
--- PASS: TestAccGithubRepository_basic (42.52s)
=== CONT  TestAccGithubRepositoryCollaborator_basic
--- PASS: TestAccGithubRepositoryProject_basic (47.80s)
=== CONT  TestAccGithubProjectColumn_basic
--- PASS: TestAccGithubRepositoryDeployKey_basic (44.12s)
=== CONT  TestAccGithubOrganizationWebhook_secret
--- PASS: TestAccGithubOrganizationWebhook_secret (14.45s)
=== CONT  TestAccOrganizationBlock_basic
--- PASS: TestAccGithubRepositoryCollaborator_basic (47.08s)
=== CONT  TestAccGithubUserSshKey_basic
--- PASS: TestAccGithubRepositoryCollaborator_caseInsensitive (59.73s)
=== CONT  TestAccGithubTeamRepository_basic
--- PASS: TestAccOrganizationBlock_basic (22.01s)
=== CONT  TestAccGithubTeamMembership_basic
--- PASS: TestAccGithubUserSshKey_basic (26.35s)
=== CONT  TestAccGithubUserGpgKey_basic
--- PASS: TestAccGithubProjectColumn_basic (64.46s)
=== CONT  TestAccGithubTeam_hierarchical
--- PASS: TestAccGithubUserGpgKey_basic (12.66s)
=== CONT  TestAccGithubRepositoryWebhook_secret
--- PASS: TestAccGithubTeam_hierarchical (33.08s)
=== CONT  TestAccGithubTeam_slug
--- PASS: TestAccGithubRepositoryWebhook_secret (38.40s)
=== CONT  TestAccGithubTeam_basic
--- PASS: TestAccGithubTeamRepository_basic (73.48s)
=== CONT  TestAccGithubRepositoryDataSource_fullName_existing
--- PASS: TestAccGithubRepositoryDataSource_fullName_existing (6.60s)
=== CONT  TestAccGithubRepositoryWebhook_basic
--- PASS: TestAccGithubTeam_slug (33.48s)
=== CONT  TestAccGithubOrganizationProject_basic
--- PASS: TestAccGithubTeamMembership_basic (93.60s)
=== CONT  TestAccGithubRepository_createFromTemplate
--- PASS: TestAccGithubTeam_basic (45.57s)
=== CONT  TestAccGithubMembership_basic
--- PASS: TestAccGithubOrganizationProject_basic (28.37s)
=== CONT  TestAccGithubIssueLabel_description
--- PASS: TestAccGithubRepository_createFromTemplate (26.01s)
=== CONT  TestAccGithubBranchProtection_users
--- PASS: TestAccGithubMembership_basic (24.88s)
=== CONT  TestAccGithubBranchProtection_basic
--- PASS: TestAccGithubRepositoryWebhook_basic (80.28s)
=== CONT  TestAccGithubUserDataSource_existing
--- PASS: TestAccGithubUserDataSource_existing (21.85s)
=== CONT  TestAccGithubBranchProtection_emptyItems
--- PASS: TestAccGithubBranchProtection_users (52.02s)
=== CONT  TestAccGithubIssueLabel_basic
--- PASS: TestAccGithubBranchProtection_basic (74.14s)
=== CONT  TestAccGithubUserDataSource_noMatchReturnsError
--- PASS: TestAccGithubUserDataSource_noMatchReturnsError (2.97s)
=== CONT  TestAccGithubTeamDataSource_noMatchReturnsError
--- PASS: TestAccGithubIssueLabel_description (102.19s)
=== CONT  TestAccGithubRepositoryDataSource_name_existing
--- PASS: TestAccGithubTeamDataSource_noMatchReturnsError (2.77s)
=== CONT  TestAccGithubReleaseDataSource_fetchByIdWithNoIdReturnsError
--- PASS: TestAccGithubReleaseDataSource_fetchByIdWithNoIdReturnsError (0.01s)
=== CONT  TestAccGithubIssueLabel_existingLabel
--- PASS: TestAccGithubRepositoryDataSource_name_existing (6.09s)
=== CONT  TestAccGithubReleaseDataSource_fetchByTagNoTagReturnsError
--- PASS: TestAccGithubReleaseDataSource_fetchByTagNoTagReturnsError (0.01s)
=== CONT  TestAccGithubReleaseDataSource_fetchByIdExisting
--- PASS: TestAccGithubReleaseDataSource_fetchByIdExisting (5.57s)
=== CONT  TestAccGithubRepositoriesDataSource_noMatch
--- PASS: TestAccGithubBranchProtection_emptyItems (56.69s)
=== CONT  TestAccGithubReleaseDataSource_fetchByTagExisting
--- PASS: TestAccGithubRepositoriesDataSource_noMatch (6.49s)
=== CONT  TestAccGithubRepositoriesDataSource_Sort
--- PASS: TestAccGithubIssueLabel_basic (62.67s)
=== CONT  TestAccGithubRepositoriesDataSource_basic
--- PASS: TestAccGithubReleaseDataSource_fetchByTagExisting (6.55s)
=== CONT  TestAccGithubRepositoryDataSource_name_noMatchReturnsError
--- PASS: TestAccGithubRepositoryDataSource_name_noMatchReturnsError (4.00s)
=== CONT  TestAccGithubReleaseDataSource_invalidRetrieveMethodReturnsError
--- PASS: TestAccGithubReleaseDataSource_invalidRetrieveMethodReturnsError (0.01s)
=== CONT  TestAccGithubRepositoryDataSource_fullName_noMatchReturnsError
--- PASS: TestAccGithubRepositoryDataSource_fullName_noMatchReturnsError (2.03s)
=== CONT  TestAccGithubReleaseDataSource_fetchByLatestNoReleaseReturnsError
--- PASS: TestAccGithubReleaseDataSource_fetchByLatestNoReleaseReturnsError (5.40s)
=== CONT  TestAccGithubReleaseDataSource_latestExisting
--- PASS: TestAccGithubRepositoriesDataSource_basic (13.41s)
=== CONT  TestAccGithubIpRangesDataSource_existing
--- PASS: TestAccGithubReleaseDataSource_latestExisting (14.37s)
=== CONT  TestAccGithubBranchProtection_teams
--- PASS: TestAccGithubRepositoriesDataSource_Sort (30.90s)
--- PASS: TestAccGithubIpRangesDataSource_existing (13.73s)
--- PASS: TestAccGithubIssueLabel_existingLabel (49.14s)
--- PASS: TestAccGithubBranchProtection_teams (89.44s)
PASS
ok      github.com/terraform-providers/terraform-provider-github/github 650.105s

@martinssipenko martinssipenko mentioned this pull request Mar 2, 2020
Merged
@martinssipenko
Copy link
Contributor Author

@jcudit can you please take a look at this?

benj-fletch
benj-fletch reviewed Mar 2, 2020
View reviewed changes
github/resource_github_repository_webhook.go Outdated
Comment on lines 189 to 193
if hook.Config["insecure_ssl"] == "1" {
hook.Config["insecure_ssl"] = true
} else {
hook.Config["insecure_ssl"] = false
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This code should be pulled out to a function instead of repeating x3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@benj-fletch
Copy link
Contributor

This looks really good @martinssipenko!
Coincidentally, I was investigating the TLS provider and it seems to cause a number of issues. An added benefit of moving to ssh is that we support windows development envs better, since it does not require a firewall rule to be added to allow the go server.

jcudit
jcudit reviewed Mar 2, 2020
View reviewed changes
Copy link
Contributor

@jcudit jcudit left a comment

Choose a reason for hiding this comment

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

@martinssipenko thanks for the effort here. I'm largely onboard with the tradeoffs you've made and documented but have asked for clarification on a few things inline.

My next steps are to run this through acceptance testing to understand how this solves some commonly occurring failed test cases. I'm aiming to ship this by end of week if there are no further complications.

github/provider_test.go
@@ -174,7 +172,7 @@ func TestProvider_insecure(t *testing.T) {
`

username := "hashibot"
resource.ParallelTest(t, resource.TestCase{
resource.Test(t, resource.TestCase{
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a bit more context around this change? I'm curious how this helps to get the test passing.

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 was unintended change, I thought TLS issues were due to parallelization, reverting to original.

github/resource_github_branch_protection_test.go
@@ -480,7 +473,7 @@ resource "github_team" "first" {
resource "github_team_repository" "first" {
team_id = "${github_team.first.id}"
repository = "${github_repository.test.name}"
permission = "pull"
permission = "push"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a bit more context around this change? I'm curious how this helps to get the test passing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to GitHub API docs - The restrictions object has the following keys: teams The list of team slugs with push access.
image

github/resource_github_branch_protection_test.go
@@ -490,7 +483,7 @@ resource "github_team" "second" {
resource "github_team_repository" "second" {
team_id = "${github_team.second.id}"
repository = "${github_repository.test.name}"
permission = "pull"
permission = "push"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a bit more context around this change? I'm curious how this helps to get the test passing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to GitHub API docs - The restrictions object has the following keys: teams The list of team slugs with push access.
image

github/resource_github_organization_webhook_test.go
@@ -32,7 +32,7 @@ func TestAccGithubOrganizationWebhook_basic(t *testing.T) {
Configuration: map[string]interface{}{
"url": "https://google.de/webhook",
"content_type": "json",
"insecure_ssl": true,
"insecure_ssl": "1",
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/terraform-providers/terraform-provider-github/pull/365 merged recently, which may affect this.
Would rebasing make this change obsolete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think my branch is up to date with upstream master. As far as I understand the testAccCheckGithubOrganizationWebhookAttributes func takes two arguments:

  • 1st - a github.Hook struct that is populated by testAccCheckGithubOrganizationWebhookExists func.
  • 2nd the expectation

Given that GitHub API returns string "0" or "1" in the response the expectation should use the same values.

github/resource_github_organization_webhook_test.go
@@ -47,7 +47,7 @@ func TestAccGithubOrganizationWebhook_basic(t *testing.T) {
Configuration: map[string]interface{}{
"url": "https://google.de/webhooks",
"content_type": "form",
"insecure_ssl": false,
"insecure_ssl": "0",
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/terraform-providers/terraform-provider-github/pull/365 merged recently, which may affect this.
Would rebasing make this change obsolete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as https://github.com/terraform-providers/terraform-provider-github/pull/372#discussion_r386587188

github/resource_github_repository_webhook_test.go
@@ -33,7 +33,7 @@ func TestAccGithubRepositoryWebhook_basic(t *testing.T) {
Configuration: map[string]interface{}{
"url": "https://google.de/webhook",
"content_type": "json",
"insecure_ssl": true,
"insecure_ssl": "1",
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/terraform-providers/terraform-provider-github/pull/365 merged recently, which may affect this.
Would rebasing make this change obsolete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as https://github.com/terraform-providers/terraform-provider-github/pull/372#discussion_r386587188

github/resource_github_repository_webhook_test.go
@@ -48,7 +48,7 @@ func TestAccGithubRepositoryWebhook_basic(t *testing.T) {
Configuration: map[string]interface{}{
"url": "https://google.de/webhooks",
"content_type": "form",
"insecure_ssl": false,
"insecure_ssl": "0",
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/terraform-providers/terraform-provider-github/pull/365 merged recently, which may affect this.
Would rebasing make this change obsolete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as https://github.com/terraform-providers/terraform-provider-github/pull/372#discussion_r386587188

github/resource_github_repository_webhook_test.go
@@ -84,7 +84,7 @@ func TestAccGithubRepositoryWebhook_secret(t *testing.T) {
"url": "https://www.terraform.io/webhook",
"content_type": "json",
"secret": "********",
"insecure_ssl": false,
"insecure_ssl": "0",
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/terraform-providers/terraform-provider-github/pull/365 merged recently, which may affect this.
Would rebasing make this change obsolete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as https://github.com/terraform-providers/terraform-provider-github/pull/372#discussion_r386587188

@kmoe kmoe mentioned this pull request Mar 2, 2020
Closed
@martinssipenko
Copy link
Contributor Author

martinssipenko commented Mar 2, 2020 •
edited
Loading

After running acceptance test many times I can say they still feel a bit flaky, sometimes some tests fail randomly. I can't explain why this happens, but my gut feeling tells me it's due to test parallelization. Could there be some race conditions within state when running tests in parallel?

Of course, removing parallelization slows down the test suite, but maybe thats fine considering it becomes more stable?

@jcudit jcudit mentioned this pull request Mar 3, 2020
Closed
4 tasks
@jcudit
Copy link
Contributor

jcudit commented Mar 3, 2020

I too have noticed that removing parallelization has increased stability of tests. The acceptance test suite I have been working on runs tests individually to work around this.

With the changes so far in this branch, I am seeing considerable stability improvements in acceptance tests and am down to just one consistent failure (TestAccGithubUserInvitationAccepter_basic):

2020-03-03T16:20:17.7587604Z === RUN   TestAccGithubUserInvitationAccepter_basic
2020-03-03T16:20:17.7587781Z === PAUSE TestAccGithubUserInvitationAccepter_basic
2020-03-03T16:20:17.7587936Z === CONT  TestAccGithubUserInvitationAccepter_basic
2020-03-03T16:20:17.7589046Z --- FAIL: TestAccGithubUserInvitationAccepter_basic (6.36s)
2020-03-03T16:20:17.7589197Z     testing.go:654: Step 1 error: unknown provider "github"
2020-03-03T16:20:17.7589336Z     testing.go:715: Error destroying resource! WARNING: Dangling resources
2020-03-03T16:20:17.7589997Z         may exist. The full state and error is shown below.
2020-03-03T16:20:17.7590125Z         
2020-03-03T16:20:17.7590967Z         Error: config is invalid: 3 problems:
2020-03-03T16:20:17.7591093Z         
2020-03-03T16:20:17.7591877Z         - Provider configuration not present: To work with github_repository.test its original provider configuration at provider.github.main is required, but it has been removed. This occurs when a provider configuration is removed while objects created by that provider still exist in the state. Re-add the provider configuration to destroy github_repository.test, after which you can remove the provider configuration again.
2020-03-03T16:20:17.7592790Z         - Provider configuration not present: To work with github_repository_collaborator.test its original provider configuration at provider.github.main is required, but it has been removed. This occurs when a provider configuration is removed while objects created by that provider still exist in the state. Re-add the provider configuration to destroy github_repository_collaborator.test, after which you can remove the provider configuration again.
2020-03-03T16:20:17.7593858Z         - Provider configuration not present: To work with github_user_invitation_accepter.test its original provider configuration at provider.github.invitee is required, but it has been removed. This occurs when a provider configuration is removed while objects created by that provider still exist in the state. Re-add the provider configuration to destroy github_user_invitation_accepter.test, after which you can remove the provider configuration again.
2020-03-03T16:20:17.7594083Z         
2020-03-03T16:20:17.7594196Z         State: github_repository.test:
2020-03-03T16:20:17.7594470Z           ID = tf-acc-test-collab-9qj6v
2020-03-03T16:20:17.7594596Z           provider = provider.github.main
2020-03-03T16:20:17.7594715Z           allow_merge_commit = true
2020-03-03T16:20:17.7594820Z           allow_rebase_merge = true
2020-03-03T16:20:17.7594937Z           allow_squash_merge = true
2020-03-03T16:20:17.7595054Z           archived = false
2020-03-03T16:20:17.7595171Z           default_branch = master
2020-03-03T16:20:17.7595289Z           description = 
2020-03-03T16:20:17.7595412Z           etag = W/"43a5b9291f4d5db5167dcae357a8ffb7"
2020-03-03T16:20:17.7595712Z           full_name = terraformtesting/tf-acc-test-collab-9qj6v
2020-03-03T16:20:17.7596041Z           git_clone_url = git://github.com/terraformtesting/tf-acc-test-collab-9qj6v.git
2020-03-03T16:20:17.7596162Z           has_downloads = false
2020-03-03T16:20:17.7596285Z           has_issues = false
2020-03-03T16:20:17.7596505Z           has_projects = false
2020-03-03T16:20:17.7596624Z           has_wiki = false
2020-03-03T16:20:17.7596738Z           homepage_url = 
2020-03-03T16:20:17.7597125Z           html_url = https://github.com/terraformtesting/tf-acc-test-collab-9qj6v
2020-03-03T16:20:17.7597492Z           http_clone_url = https://github.com/terraformtesting/tf-acc-test-collab-9qj6v.git
2020-03-03T16:20:17.7597794Z           name = tf-acc-test-collab-9qj6v
2020-03-03T16:20:17.7597902Z           private = false
2020-03-03T16:20:17.7598210Z           ssh_clone_url = git@github.com:terraformtesting/tf-acc-test-collab-9qj6v.git
2020-03-03T16:20:17.7598546Z           svn_url = https://github.com/terraformtesting/tf-acc-test-collab-9qj6v
2020-03-03T16:20:17.7598693Z           template.# = 0
2020-03-03T16:20:17.7598816Z         github_repository_collaborator.test:
2020-03-03T16:20:17.7599117Z           ID = tf-acc-test-collab-9qj6v:github-terraform-test-collaborator
2020-03-03T16:20:17.7599255Z           provider = provider.github.main
2020-03-03T16:20:17.7599374Z           invitation_id = 24100359
2020-03-03T16:20:17.7599474Z           permission = push
2020-03-03T16:20:17.7599744Z           repository = tf-acc-test-collab-9qj6v
2020-03-03T16:20:17.7600024Z           username = github-terraform-test-collaborator
2020-03-03T16:20:17.7600154Z         
2020-03-03T16:20:17.7600266Z           Dependencies:
2020-03-03T16:20:17.7600380Z             github_repository.test
2020-03-03T16:20:17.7600500Z         github_user_invitation_accepter.test:
2020-03-03T16:20:17.7600618Z           ID = 24100359
2020-03-03T16:20:17.7600717Z           provider = provider.github.invitee
2020-03-03T16:20:17.7600838Z           invitation_id = 24100359
2020-03-03T16:20:17.7600953Z         
2020-03-03T16:20:17.7601063Z           Dependencies:
2020-03-03T16:20:17.7601179Z             github_repository_collaborator.test
2020-03-03T16:20:17.7601297Z FAIL
2020-03-03T16:20:17.7601600Z FAIL	github.com/terraform-providers/terraform-provider-github/github	6.376s
2020-03-03T16:20:17.7604829Z FAIL
2020-03-03T16:20:18.4318136Z ?   	github.com/terraform-providers/terraform-provider-github	[no test files]

I will continue iterating towards a pass and still plan on merging this branch by end of week.

This was referenced Mar 4, 2020
Merged
Closed
@jcudit
Copy link
Contributor

jcudit commented Mar 5, 2020

I've finished up root causing the final failing test and am nearly ready to merge this in.

My next steps are to request a new release be cut prior to this merging as there are changes that can be isolated from this fairly large one. Once that gets actioned, this will merge and another release will be cut to help future debugging efforts. Still on track for end of week 😄

@jcudit jcudit changed the title WIP: Upgrade to go 1.13 Upgrade to go 1.13 Mar 5, 2020
@jcudit jcudit merged commit 895ed90 into integrations:master Mar 5, 2020
@martinssipenko martinssipenko deleted the gh-370-go1.13 branch March 5, 2020 17:48
@martinssipenko
Copy link
Contributor Author

🎉

@arri-cc arri-cc mentioned this pull request Mar 5, 2020
Closed
@jcudit jcudit added this to the v2.5.0 milestone Mar 27, 2020
@tbehling tbehling mentioned this pull request Apr 9, 2021
Merged
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
Merge pull request integrations#372 from martinssipenko/integrationsg…
af0731c 
…h-370-go1.13

Upgrade to go 1.13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benj-fletch benj-fletch benj-fletch left review comments

@jcudit jcudit jcudit approved these changes

Assignees
No one assigned
Labels
size/XXL
Projects
None yet
Milestone
v2.5.0
Development

Successfully merging this pull request may close these issues.

Align with terraform and go-github Go versions
3 participants
@martinssipenko @benj-fletch @jcudit