From e4a5d4e8d144fc85ba9255f2804643e99eaceb97 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Fri, 14 Jun 2019 10:12:05 -0500 Subject: [PATCH 01/15] update usernames to case insensitive --- github/resource_github_branch_protection.go | 4 ++ .../resource_github_branch_protection_test.go | 70 ++++++++++++++++--- github/resource_github_membership.go | 12 ++-- github/resource_github_membership_test.go | 22 ++++++ ...resource_github_repository_collaborator.go | 7 +- ...rce_github_repository_collaborator_test.go | 27 +++++++ github/resource_github_team_membership.go | 7 +- .../resource_github_team_membership_test.go | 38 +++++++++- github/resource_organization_block.go | 7 +- github/resource_organization_block_test.go | 14 ++++ github/util.go | 6 ++ 11 files changed, 185 insertions(+), 29 deletions(-) diff --git a/github/resource_github_branch_protection.go b/github/resource_github_branch_protection.go index 7b5ca70f15..6109e6909f 100644 --- a/github/resource_github_branch_protection.go +++ b/github/resource_github_branch_protection.go @@ -87,6 +87,9 @@ func resourceGithubBranchProtection() *schema.Resource { Type: schema.TypeSet, Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, + //DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { + // return true + //}, }, "dismissal_teams": { Type: schema.TypeSet, @@ -159,6 +162,7 @@ func resourceGithubBranchProtectionCreate(d *schema.ResourceData, meta interface branch, protectionRequest, ) + if err != nil { return err } diff --git a/github/resource_github_branch_protection_test.go b/github/resource_github_branch_protection_test.go index 7bef788bb4..cc1abd37c9 100644 --- a/github/resource_github_branch_protection_test.go +++ b/github/resource_github_branch_protection_test.go @@ -5,6 +5,7 @@ import ( "fmt" "sort" "testing" + "unicode" "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/acctest" @@ -24,24 +25,34 @@ func TestAccGithubBranchProtection_basic(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccGithubBranchProtectionDestroy, Steps: []resource.TestStep{ + //{ + // Config: testAccGithubBranchProtectionConfig(repoName), + // Check: resource.ComposeTestCheckFunc( + // testAccCheckGithubProtectedBranchExists("github_branch_protection.master", repoName+":master", &protection), + // testAccCheckGithubBranchProtectionRequiredStatusChecks(&protection, true, []string{"github/foo"}), + // testAccCheckGithubBranchProtectionRestrictions(&protection, []string{testUser}, []string{}), + // testAccCheckGithubBranchProtectionPullRequestReviews(&protection, true, []string{testUser}, []string{}, true), + // resource.TestCheckResourceAttr("github_branch_protection.master", "repository", repoName), + // resource.TestCheckResourceAttr("github_branch_protection.master", "branch", "master"), + // resource.TestCheckResourceAttr("github_branch_protection.master", "enforce_admins", "true"), + // resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.strict", "true"), + // resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.contexts.#", "1"), + // resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismiss_stale_reviews", "true"), + // resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_users.#", "1"), + // resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_teams.#", "0"), + // resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.require_code_owner_reviews", "true"), + // resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.users.#", "1"), + // resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.teams.#", "0"), + // ), + //}, { - Config: testAccGithubBranchProtectionConfig(repoName), + Config: testAccGithubBranchProtectionConfig_usersCaseInsensitive(repoName), Check: resource.ComposeTestCheckFunc( testAccCheckGithubProtectedBranchExists("github_branch_protection.master", repoName+":master", &protection), - testAccCheckGithubBranchProtectionRequiredStatusChecks(&protection, true, []string{"github/foo"}), testAccCheckGithubBranchProtectionRestrictions(&protection, []string{testUser}, []string{}), testAccCheckGithubBranchProtectionPullRequestReviews(&protection, true, []string{testUser}, []string{}, true), - resource.TestCheckResourceAttr("github_branch_protection.master", "repository", repoName), - resource.TestCheckResourceAttr("github_branch_protection.master", "branch", "master"), - resource.TestCheckResourceAttr("github_branch_protection.master", "enforce_admins", "true"), - resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.strict", "true"), - resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.contexts.#", "1"), - resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismiss_stale_reviews", "true"), resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_users.#", "1"), - resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_teams.#", "0"), - resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.require_code_owner_reviews", "true"), resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.users.#", "1"), - resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.teams.#", "0"), ), }, { @@ -377,6 +388,43 @@ resource "github_branch_protection" "master" { `, repoName, repoName, testUser, testUser) } +func testAccGithubBranchProtectionConfig_usersCaseInsensitive(repoName string) string { + otherCase := []rune(testUser) + if unicode.IsUpper(otherCase[0]) { + otherCase[0] = unicode.ToLower(otherCase[0]) + } else { + otherCase[0] = unicode.ToUpper(otherCase[0]) + } + return fmt.Sprintf(` +resource "github_repository" "test" { + name = "%s" + description = "Terraform Acceptance Test %s" + auto_init = true +} + +resource "github_branch_protection" "master" { + repository = "${github_repository.test.name}" + branch = "master" + enforce_admins = true + + required_status_checks { + strict = true + contexts = ["github/foo"] + } + + required_pull_request_reviews { + dismiss_stale_reviews = true + dismissal_users = ["%s"] + require_code_owner_reviews = true + } + + restrictions { + users = ["%s"] + } +} +`, repoName, repoName, string(otherCase), string(otherCase)) +} + func testAccGithubBranchProtectionUpdateConfig(repoName string) string { return fmt.Sprintf(` resource "github_repository" "test" { diff --git a/github/resource_github_membership.go b/github/resource_github_membership.go index 914620f600..a720566829 100644 --- a/github/resource_github_membership.go +++ b/github/resource_github_membership.go @@ -2,11 +2,10 @@ package github import ( "context" - "log" - "net/http" - "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/schema" + "log" + "net/http" ) func resourceGithubMembership() *schema.Resource { @@ -21,9 +20,10 @@ func resourceGithubMembership() *schema.Resource { Schema: map[string]*schema.Schema{ "username": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + DiffSuppressFunc: caseInsensitive(), }, "role": { Type: schema.TypeString, diff --git a/github/resource_github_membership_test.go b/github/resource_github_membership_test.go index 3b28c76a48..cfc5693d17 100644 --- a/github/resource_github_membership_test.go +++ b/github/resource_github_membership_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "testing" + "unicode" "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/resource" @@ -25,6 +26,12 @@ func TestAccGithubMembership_basic(t *testing.T) { testAccCheckGithubMembershipRoleState("github_membership.test_org_membership", &membership), ), }, + { + Config: testAccGithubMembershipConfig_caseInsensitive(), + Check: resource.ComposeTestCheckFunc( + testAccCheckGithubMembershipExists("github_membership.test_org_membership", &membership), + ), + }, }, }) } @@ -140,3 +147,18 @@ var testAccGithubMembershipConfig string = fmt.Sprintf(` role = "member" } `, testCollaborator) + +func testAccGithubMembershipConfig_caseInsensitive() string { + otherCase := []rune(testCollaborator) + if unicode.IsUpper(otherCase[0]) { + otherCase[0] = unicode.ToLower(otherCase[0]) + } else { + otherCase[0] = unicode.ToUpper(otherCase[0]) + } + return fmt.Sprintf(` + resource "github_membership" "test_org_membership" { + username = "%s" + role = "member" + } +`, string(otherCase)) +} diff --git a/github/resource_github_repository_collaborator.go b/github/resource_github_repository_collaborator.go index 148b526247..e28eb17867 100644 --- a/github/resource_github_repository_collaborator.go +++ b/github/resource_github_repository_collaborator.go @@ -21,9 +21,10 @@ func resourceGithubRepositoryCollaborator() *schema.Resource { // editing repository collaborators are not supported by github api so forcing new on any changes Schema: map[string]*schema.Schema{ "username": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + DiffSuppressFunc: caseInsensitive(), }, "repository": { Type: schema.TypeString, diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index fc504d5a44..13f3e672c4 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -5,6 +5,7 @@ import ( "fmt" "regexp" "testing" + "unicode" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" @@ -31,6 +32,12 @@ func TestAccGithubRepositoryCollaborator_basic(t *testing.T) { resource.TestMatchResourceAttr(resourceName, "invitation_id", regexp.MustCompile(`^[0-9]+$`)), ), }, + { + Config: testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName), + Check: resource.ComposeTestCheckFunc( + testAccCheckGithubRepositoryCollaboratorExists(resourceName), + ), + }, }, }) } @@ -182,3 +189,23 @@ resource "github_repository" "test" { } `, repoName, testCollaborator, expectedPermission) } + +func testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName string) string { + otherCase := []rune(testCollaborator) + if unicode.IsUpper(otherCase[0]) { + otherCase[0] = unicode.ToLower(otherCase[0]) + } else { + otherCase[0] = unicode.ToUpper(otherCase[0]) + } + return fmt.Sprintf(` +resource "github_repository" "test" { + name = "%s" +} + + resource "github_repository_collaborator" "test_repo_collaborator" { + repository = "${github_repository.test.name}" + username = "%s" + permission = "%s" + } +`, repoName, string(otherCase), expectedPermission) +} diff --git a/github/resource_github_team_membership.go b/github/resource_github_team_membership.go index 64a8ec7a28..bfab4eb865 100644 --- a/github/resource_github_team_membership.go +++ b/github/resource_github_team_membership.go @@ -29,9 +29,10 @@ func resourceGithubTeamMembership() *schema.Resource { ForceNew: true, }, "username": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + DiffSuppressFunc: caseInsensitive(), }, "role": { Type: schema.TypeString, diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go index cc427816eb..793bcd9314 100644 --- a/github/resource_github_team_membership_test.go +++ b/github/resource_github_team_membership_test.go @@ -3,13 +3,13 @@ package github import ( "context" "fmt" - "strconv" - "testing" - "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" + "strconv" + "testing" + "unicode" ) func TestAccGithubTeamMembership_basic(t *testing.T) { @@ -28,6 +28,12 @@ func TestAccGithubTeamMembership_basic(t *testing.T) { testAccCheckGithubTeamMembershipRoleState("github_team_membership.test_team_membership", "member", &membership), ), }, + { + Config: testAccGithubTeamMembershipConfig_caseInsensitive(randString, testCollaborator, "member"), + Check: resource.ComposeTestCheckFunc( + testAccCheckGithubTeamMembershipExists("github_team_membership.test_team_membership", &membership), + ), + }, { Config: testAccGithubTeamMembershipConfig(randString, testCollaborator, "maintainer"), Check: resource.ComposeTestCheckFunc( @@ -184,3 +190,29 @@ resource "github_team_membership" "test_team_membership" { } `, username, randString, username, role) } + +func testAccGithubTeamMembershipConfig_caseInsensitive(randString, username, role string) string { + otherCase := []rune(testCollaborator) + if unicode.IsUpper(otherCase[0]) { + otherCase[0] = unicode.ToLower(otherCase[0]) + } else { + otherCase[0] = unicode.ToUpper(otherCase[0]) + } + return fmt.Sprintf(` +resource "github_membership" "test_org_membership" { + username = "%s" + role = "member" +} + +resource "github_team" "test_team" { + name = "tf-acc-test-team-membership-%s" + description = "Terraform acc test group" +} + +resource "github_team_membership" "test_team_membership" { + team_id = "${github_team.test_team.id}" + username = "%s" + role = "%s" +} +`, string(otherCase), randString, string(otherCase), role) +} diff --git a/github/resource_organization_block.go b/github/resource_organization_block.go index c86265a0df..4e80477766 100644 --- a/github/resource_organization_block.go +++ b/github/resource_organization_block.go @@ -20,9 +20,10 @@ func resourceOrganizationBlock() *schema.Resource { Schema: map[string]*schema.Schema{ "username": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + DiffSuppressFunc: caseInsensitive(), }, "etag": { diff --git a/github/resource_organization_block_test.go b/github/resource_organization_block_test.go index 8f292a4951..de78d4eb16 100644 --- a/github/resource_organization_block_test.go +++ b/github/resource_organization_block_test.go @@ -21,6 +21,12 @@ func TestAccOrganizationBlock_basic(t *testing.T) { testAccCheckOrganizationBlockExists("github_organization_block.test"), ), }, + { + Config: testAccOrganizationBlockConfig_caseInsensitive(), + Check: resource.ComposeTestCheckFunc( + testAccCheckOrganizationBlockExists("github_organization_block.test"), + ), + }, }, }) } @@ -85,6 +91,14 @@ func testAccCheckOrganizationBlockExists(n string) resource.TestCheckFunc { } } +func testAccOrganizationBlockConfig_caseInsensitive() string { + return ` +resource "github_organization_block" "test" { + username = "cGriggs01" +} +` +} + const testAccOrganizationBlockConfig = ` resource "github_organization_block" "test" { username = "cgriggs01" diff --git a/github/util.go b/github/util.go index b3d232ddd0..d53e7cb1b1 100644 --- a/github/util.go +++ b/github/util.go @@ -12,6 +12,12 @@ const ( maxPerPage = 100 ) +func caseInsensitive() schema.SchemaDiffSuppressFunc { + return func(k, old, new string, d *schema.ResourceData) bool { + return strings.ToLower(old) == strings.ToLower(new) + } +} + func validateValueFunc(values []string) schema.SchemaValidateFunc { return func(v interface{}, k string) (we []string, errors []error) { value := v.(string) From a9ba1cd1e290eefc1050934164b7cd29f6ce480d Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Fri, 14 Jun 2019 10:38:44 -0500 Subject: [PATCH 02/15] flatten stuff --- github/resource_github_branch_protection.go | 59 ++++++++++++++++++--- 1 file changed, 51 insertions(+), 8 deletions(-) diff --git a/github/resource_github_branch_protection.go b/github/resource_github_branch_protection.go index 6109e6909f..096a14e125 100644 --- a/github/resource_github_branch_protection.go +++ b/github/resource_github_branch_protection.go @@ -211,18 +211,27 @@ func resourceGithubBranchProtectionRead(d *schema.ResourceData, meta interface{} d.Set("branch", branch) d.Set("enforce_admins", githubProtection.EnforceAdmins.Enabled) - if err := flattenAndSetRequiredStatusChecks(d, githubProtection); err != nil { - return fmt.Errorf("Error setting required_status_checks: %v", err) - } + if githubProtection != nil { + if err := flattenAndSetRequiredStatusChecks(d, githubProtection); err != nil { + return fmt.Errorf("Error setting required_status_checks: %v", err) + } - if err := flattenAndSetRequiredPullRequestReviews(d, githubProtection); err != nil { - return fmt.Errorf("Error setting required_pull_request_reviews: %v", err) - } + if err := flattenAndSetRequiredPullRequestReviews(d, githubProtection); err != nil { + return fmt.Errorf("Error setting required_pull_request_reviews: %v", err) + } - if err := flattenAndSetRestrictions(d, githubProtection); err != nil { - return fmt.Errorf("Error setting restrictions: %v", err) + if restrictions := githubProtection.Restrictions; restrictions != nil { + if err := d.Set("restrictions", flattenRestrictions(restrictions)); err != nil { + return fmt.Errorf("Error setting restrictions: %v", err) + } + return fmt.Errorf("%+v \n\n\n %+v", flattenRestrictions(restrictions)[0].(map[string]interface{})["teams"], flattenRestrictions(restrictions)[0].(map[string]interface{})["users"]) + } } + //if err := flattenAndSetRestrictions(d, githubProtection); err != nil { + // return fmt.Errorf("Error setting restrictions: %v", err) + //} + return nil } @@ -361,6 +370,40 @@ func flattenAndSetRequiredPullRequestReviews(d *schema.ResourceData, protection return d.Set("required_pull_request_reviews", []interface{}{}) } +func flattenRestrictions(restrictions *github.BranchRestrictions) []interface{} { + restrictionDetails := make(map[string]interface{}, 0) + + if users := restrictions.Users; users != nil { + restrictionDetails["users"] = flattenRestrictionUsers(users) + } + + if teams := restrictions.Teams; teams != nil { + restrictionDetails["teams"] = flattenRestrictionTeams(teams) + } + + return []interface{}{restrictionDetails} +} + +func flattenRestrictionUsers(users []*github.User) *schema.Set { + s := &schema.Set{F: schema.HashString} + for _, v := range users { + if v != nil && v.Login != nil { + s.Add(*v.Login) + } + } + return s +} + +func flattenRestrictionTeams(teams []*github.Team) *schema.Set { + s := &schema.Set{F: schema.HashString} + for _, v := range teams { + if v != nil && v.Slug != nil { + s.Add(*v.Slug) + } + } + return s +} + func flattenAndSetRestrictions(d *schema.ResourceData, protection *github.Protection) error { restrictions := protection.Restrictions if restrictions != nil { From 0b784bbfe6a3421407faf865a54af6d5a74b35e9 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Fri, 14 Jun 2019 13:08:46 -0500 Subject: [PATCH 03/15] Undo branch protection work --- github/resource_github_branch_protection.go | 62 ++------------- .../resource_github_branch_protection_test.go | 75 ++++--------------- 2 files changed, 21 insertions(+), 116 deletions(-) diff --git a/github/resource_github_branch_protection.go b/github/resource_github_branch_protection.go index 096a14e125..8543119a00 100644 --- a/github/resource_github_branch_protection.go +++ b/github/resource_github_branch_protection.go @@ -87,9 +87,6 @@ func resourceGithubBranchProtection() *schema.Resource { Type: schema.TypeSet, Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, - //DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // return true - //}, }, "dismissal_teams": { Type: schema.TypeSet, @@ -211,26 +208,17 @@ func resourceGithubBranchProtectionRead(d *schema.ResourceData, meta interface{} d.Set("branch", branch) d.Set("enforce_admins", githubProtection.EnforceAdmins.Enabled) - if githubProtection != nil { - if err := flattenAndSetRequiredStatusChecks(d, githubProtection); err != nil { - return fmt.Errorf("Error setting required_status_checks: %v", err) - } - - if err := flattenAndSetRequiredPullRequestReviews(d, githubProtection); err != nil { - return fmt.Errorf("Error setting required_pull_request_reviews: %v", err) - } + if err := flattenAndSetRequiredStatusChecks(d, githubProtection); err != nil { + return fmt.Errorf("Error setting required_status_checks: %v", err) + } - if restrictions := githubProtection.Restrictions; restrictions != nil { - if err := d.Set("restrictions", flattenRestrictions(restrictions)); err != nil { - return fmt.Errorf("Error setting restrictions: %v", err) - } - return fmt.Errorf("%+v \n\n\n %+v", flattenRestrictions(restrictions)[0].(map[string]interface{})["teams"], flattenRestrictions(restrictions)[0].(map[string]interface{})["users"]) - } + if err := flattenAndSetRequiredPullRequestReviews(d, githubProtection); err != nil { + return fmt.Errorf("Error setting required_pull_request_reviews: %v", err) } - //if err := flattenAndSetRestrictions(d, githubProtection); err != nil { - // return fmt.Errorf("Error setting restrictions: %v", err) - //} + if err := flattenAndSetRestrictions(d, githubProtection); err != nil { + return fmt.Errorf("Error setting restrictions: %v", err) + } return nil } @@ -370,40 +358,6 @@ func flattenAndSetRequiredPullRequestReviews(d *schema.ResourceData, protection return d.Set("required_pull_request_reviews", []interface{}{}) } -func flattenRestrictions(restrictions *github.BranchRestrictions) []interface{} { - restrictionDetails := make(map[string]interface{}, 0) - - if users := restrictions.Users; users != nil { - restrictionDetails["users"] = flattenRestrictionUsers(users) - } - - if teams := restrictions.Teams; teams != nil { - restrictionDetails["teams"] = flattenRestrictionTeams(teams) - } - - return []interface{}{restrictionDetails} -} - -func flattenRestrictionUsers(users []*github.User) *schema.Set { - s := &schema.Set{F: schema.HashString} - for _, v := range users { - if v != nil && v.Login != nil { - s.Add(*v.Login) - } - } - return s -} - -func flattenRestrictionTeams(teams []*github.Team) *schema.Set { - s := &schema.Set{F: schema.HashString} - for _, v := range teams { - if v != nil && v.Slug != nil { - s.Add(*v.Slug) - } - } - return s -} - func flattenAndSetRestrictions(d *schema.ResourceData, protection *github.Protection) error { restrictions := protection.Restrictions if restrictions != nil { diff --git a/github/resource_github_branch_protection_test.go b/github/resource_github_branch_protection_test.go index cc1abd37c9..9f39649c57 100644 --- a/github/resource_github_branch_protection_test.go +++ b/github/resource_github_branch_protection_test.go @@ -3,15 +3,13 @@ package github import ( "context" "fmt" - "sort" - "testing" - "unicode" - "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" "github.com/kylelemons/godebug/pretty" + "sort" + "testing" ) func TestAccGithubBranchProtection_basic(t *testing.T) { @@ -25,34 +23,24 @@ func TestAccGithubBranchProtection_basic(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccGithubBranchProtectionDestroy, Steps: []resource.TestStep{ - //{ - // Config: testAccGithubBranchProtectionConfig(repoName), - // Check: resource.ComposeTestCheckFunc( - // testAccCheckGithubProtectedBranchExists("github_branch_protection.master", repoName+":master", &protection), - // testAccCheckGithubBranchProtectionRequiredStatusChecks(&protection, true, []string{"github/foo"}), - // testAccCheckGithubBranchProtectionRestrictions(&protection, []string{testUser}, []string{}), - // testAccCheckGithubBranchProtectionPullRequestReviews(&protection, true, []string{testUser}, []string{}, true), - // resource.TestCheckResourceAttr("github_branch_protection.master", "repository", repoName), - // resource.TestCheckResourceAttr("github_branch_protection.master", "branch", "master"), - // resource.TestCheckResourceAttr("github_branch_protection.master", "enforce_admins", "true"), - // resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.strict", "true"), - // resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.contexts.#", "1"), - // resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismiss_stale_reviews", "true"), - // resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_users.#", "1"), - // resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_teams.#", "0"), - // resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.require_code_owner_reviews", "true"), - // resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.users.#", "1"), - // resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.teams.#", "0"), - // ), - //}, { - Config: testAccGithubBranchProtectionConfig_usersCaseInsensitive(repoName), + Config: testAccGithubBranchProtectionConfig(repoName), Check: resource.ComposeTestCheckFunc( testAccCheckGithubProtectedBranchExists("github_branch_protection.master", repoName+":master", &protection), + testAccCheckGithubBranchProtectionRequiredStatusChecks(&protection, true, []string{"github/foo"}), testAccCheckGithubBranchProtectionRestrictions(&protection, []string{testUser}, []string{}), testAccCheckGithubBranchProtectionPullRequestReviews(&protection, true, []string{testUser}, []string{}, true), + resource.TestCheckResourceAttr("github_branch_protection.master", "repository", repoName), + resource.TestCheckResourceAttr("github_branch_protection.master", "branch", "master"), + resource.TestCheckResourceAttr("github_branch_protection.master", "enforce_admins", "true"), + resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.strict", "true"), + resource.TestCheckResourceAttr("github_branch_protection.master", "required_status_checks.0.contexts.#", "1"), + resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismiss_stale_reviews", "true"), resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_users.#", "1"), + resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.dismissal_teams.#", "0"), + resource.TestCheckResourceAttr("github_branch_protection.master", "required_pull_request_reviews.0.require_code_owner_reviews", "true"), resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.users.#", "1"), + resource.TestCheckResourceAttr("github_branch_protection.master", "restrictions.0.teams.#", "0"), ), }, { @@ -388,43 +376,6 @@ resource "github_branch_protection" "master" { `, repoName, repoName, testUser, testUser) } -func testAccGithubBranchProtectionConfig_usersCaseInsensitive(repoName string) string { - otherCase := []rune(testUser) - if unicode.IsUpper(otherCase[0]) { - otherCase[0] = unicode.ToLower(otherCase[0]) - } else { - otherCase[0] = unicode.ToUpper(otherCase[0]) - } - return fmt.Sprintf(` -resource "github_repository" "test" { - name = "%s" - description = "Terraform Acceptance Test %s" - auto_init = true -} - -resource "github_branch_protection" "master" { - repository = "${github_repository.test.name}" - branch = "master" - enforce_admins = true - - required_status_checks { - strict = true - contexts = ["github/foo"] - } - - required_pull_request_reviews { - dismiss_stale_reviews = true - dismissal_users = ["%s"] - require_code_owner_reviews = true - } - - restrictions { - users = ["%s"] - } -} -`, repoName, repoName, string(otherCase), string(otherCase)) -} - func testAccGithubBranchProtectionUpdateConfig(repoName string) string { return fmt.Sprintf(` resource "github_repository" "test" { From 08b732c17af6cdd8cfdc43ad91be817c24d19cf5 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Fri, 14 Jun 2019 13:12:23 -0500 Subject: [PATCH 04/15] fix imports --- github/resource_github_branch_protection.go | 1 - github/resource_github_branch_protection_test.go | 5 +++-- github/resource_github_membership.go | 5 +++-- github/resource_github_team_membership_test.go | 7 ++++--- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/github/resource_github_branch_protection.go b/github/resource_github_branch_protection.go index 8543119a00..7b5ca70f15 100644 --- a/github/resource_github_branch_protection.go +++ b/github/resource_github_branch_protection.go @@ -159,7 +159,6 @@ func resourceGithubBranchProtectionCreate(d *schema.ResourceData, meta interface branch, protectionRequest, ) - if err != nil { return err } diff --git a/github/resource_github_branch_protection_test.go b/github/resource_github_branch_protection_test.go index 9f39649c57..7bef788bb4 100644 --- a/github/resource_github_branch_protection_test.go +++ b/github/resource_github_branch_protection_test.go @@ -3,13 +3,14 @@ package github import ( "context" "fmt" + "sort" + "testing" + "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" "github.com/kylelemons/godebug/pretty" - "sort" - "testing" ) func TestAccGithubBranchProtection_basic(t *testing.T) { diff --git a/github/resource_github_membership.go b/github/resource_github_membership.go index a720566829..4955387ba3 100644 --- a/github/resource_github_membership.go +++ b/github/resource_github_membership.go @@ -2,10 +2,11 @@ package github import ( "context" - "github.com/google/go-github/v25/github" - "github.com/hashicorp/terraform/helper/schema" "log" "net/http" + + "github.com/google/go-github/v25/github" + "github.com/hashicorp/terraform/helper/schema" ) func resourceGithubMembership() *schema.Resource { diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go index 793bcd9314..f6b7025fb2 100644 --- a/github/resource_github_team_membership_test.go +++ b/github/resource_github_team_membership_test.go @@ -3,13 +3,14 @@ package github import ( "context" "fmt" + "strconv" + "testing" + "unicode" + "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" - "strconv" - "testing" - "unicode" ) func TestAccGithubTeamMembership_basic(t *testing.T) { From a5b71f64584f8704f60cd3b0e3222ea9de478dce Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Fri, 14 Jun 2019 14:20:47 -0500 Subject: [PATCH 05/15] remove changes to org block --- github/resource_github_team_membership_test.go | 2 +- github/resource_organization_block.go | 7 +++---- github/resource_organization_block_test.go | 14 -------------- 3 files changed, 4 insertions(+), 19 deletions(-) diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go index f6b7025fb2..7e9362b345 100644 --- a/github/resource_github_team_membership_test.go +++ b/github/resource_github_team_membership_test.go @@ -6,7 +6,7 @@ import ( "strconv" "testing" "unicode" - + "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" diff --git a/github/resource_organization_block.go b/github/resource_organization_block.go index 4e80477766..c86265a0df 100644 --- a/github/resource_organization_block.go +++ b/github/resource_organization_block.go @@ -20,10 +20,9 @@ func resourceOrganizationBlock() *schema.Resource { Schema: map[string]*schema.Schema{ "username": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - DiffSuppressFunc: caseInsensitive(), + Type: schema.TypeString, + Required: true, + ForceNew: true, }, "etag": { diff --git a/github/resource_organization_block_test.go b/github/resource_organization_block_test.go index de78d4eb16..8f292a4951 100644 --- a/github/resource_organization_block_test.go +++ b/github/resource_organization_block_test.go @@ -21,12 +21,6 @@ func TestAccOrganizationBlock_basic(t *testing.T) { testAccCheckOrganizationBlockExists("github_organization_block.test"), ), }, - { - Config: testAccOrganizationBlockConfig_caseInsensitive(), - Check: resource.ComposeTestCheckFunc( - testAccCheckOrganizationBlockExists("github_organization_block.test"), - ), - }, }, }) } @@ -91,14 +85,6 @@ func testAccCheckOrganizationBlockExists(n string) resource.TestCheckFunc { } } -func testAccOrganizationBlockConfig_caseInsensitive() string { - return ` -resource "github_organization_block" "test" { - username = "cGriggs01" -} -` -} - const testAccOrganizationBlockConfig = ` resource "github_organization_block" "test" { username = "cgriggs01" From e4375c33fa87498ad845ace36054e33a48327723 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Fri, 14 Jun 2019 16:12:03 -0500 Subject: [PATCH 06/15] update tests --- github/resource_github_membership_test.go | 12 ++++++++++++ ...rce_github_repository_collaborator_test.go | 19 +++++++++++++++++++ .../resource_github_team_membership_test.go | 19 ++++++++++++++++--- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/github/resource_github_membership_test.go b/github/resource_github_membership_test.go index cfc5693d17..320740cdd8 100644 --- a/github/resource_github_membership_test.go +++ b/github/resource_github_membership_test.go @@ -26,6 +26,18 @@ func TestAccGithubMembership_basic(t *testing.T) { testAccCheckGithubMembershipRoleState("github_membership.test_org_membership", &membership), ), }, + }, + }) +} + +func TestAccGithubMembership_caseInsensitive(t *testing.T) { + var membership github.Membership + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckGithubMembershipDestroy, + Steps: []resource.TestStep{ { Config: testAccGithubMembershipConfig_caseInsensitive(), Check: resource.ComposeTestCheckFunc( diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index 13f3e672c4..de53848273 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -42,6 +42,25 @@ func TestAccGithubRepositoryCollaborator_basic(t *testing.T) { }) } +func TestAccGithubRepositoryCollaborator_caseInsensitive(t *testing.T) { + resourceName := "github_repository_collaborator.test_repo_collaborator" + repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckGithubRepositoryCollaboratorDestroy, + Steps: []resource.TestStep{ + { + Config: testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName), + Check: resource.ComposeTestCheckFunc( + testAccCheckGithubRepositoryCollaboratorExists(resourceName), + ), + }, + }, + }) +} + func TestAccGithubRepositoryCollaborator_importBasic(t *testing.T) { repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5)) diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go index 7e9362b345..d322dd0cc2 100644 --- a/github/resource_github_team_membership_test.go +++ b/github/resource_github_team_membership_test.go @@ -30,16 +30,29 @@ func TestAccGithubTeamMembership_basic(t *testing.T) { ), }, { - Config: testAccGithubTeamMembershipConfig_caseInsensitive(randString, testCollaborator, "member"), + Config: testAccGithubTeamMembershipConfig(randString, testCollaborator, "maintainer"), Check: resource.ComposeTestCheckFunc( testAccCheckGithubTeamMembershipExists("github_team_membership.test_team_membership", &membership), + testAccCheckGithubTeamMembershipRoleState("github_team_membership.test_team_membership", "maintainer", &membership), ), }, + }, + }) +} + +func TestAccGithubTeamMembership_caseInsensitive(t *testing.T) { + var membership github.Membership + randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckGithubTeamMembershipDestroy, + Steps: []resource.TestStep{ { - Config: testAccGithubTeamMembershipConfig(randString, testCollaborator, "maintainer"), + Config: testAccGithubTeamMembershipConfig_caseInsensitive(randString, testCollaborator, "member"), Check: resource.ComposeTestCheckFunc( testAccCheckGithubTeamMembershipExists("github_team_membership.test_team_membership", &membership), - testAccCheckGithubTeamMembershipRoleState("github_team_membership.test_team_membership", "maintainer", &membership), ), }, }, From b56a29cbcb1164a3259c44060f4b24f8f47d6ada Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Mon, 17 Jun 2019 07:35:41 -0600 Subject: [PATCH 07/15] Fixing panic when testCollaborator isn't set --- github/resource_github_membership_test.go | 3 +++ github/resource_github_repository_collaborator_test.go | 3 +++ github/resource_github_team_membership_test.go | 3 +++ 3 files changed, 9 insertions(+) diff --git a/github/resource_github_membership_test.go b/github/resource_github_membership_test.go index 320740cdd8..55cd7fddcd 100644 --- a/github/resource_github_membership_test.go +++ b/github/resource_github_membership_test.go @@ -31,6 +31,9 @@ func TestAccGithubMembership_basic(t *testing.T) { } func TestAccGithubMembership_caseInsensitive(t *testing.T) { + if len(testCollaborator) == 0 { + t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is 0") + } var membership github.Membership resource.Test(t, resource.TestCase{ diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index de53848273..22f665b79b 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -43,6 +43,9 @@ func TestAccGithubRepositoryCollaborator_basic(t *testing.T) { } func TestAccGithubRepositoryCollaborator_caseInsensitive(t *testing.T) { + if len(testCollaborator) == 0 { + t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is 0") + } resourceName := "github_repository_collaborator.test_repo_collaborator" repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5)) diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go index d322dd0cc2..c0f43ccca9 100644 --- a/github/resource_github_team_membership_test.go +++ b/github/resource_github_team_membership_test.go @@ -41,6 +41,9 @@ func TestAccGithubTeamMembership_basic(t *testing.T) { } func TestAccGithubTeamMembership_caseInsensitive(t *testing.T) { + if len(testCollaborator) == 0 { + t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is 0") + } var membership github.Membership randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) From ff7d46d437c8168dce4689481d77b46020085f37 Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Mon, 17 Jun 2019 07:55:42 -0600 Subject: [PATCH 08/15] Remove unneccessary test check --- github/resource_github_repository_collaborator_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index 22f665b79b..1f84abee3b 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -32,12 +32,6 @@ func TestAccGithubRepositoryCollaborator_basic(t *testing.T) { resource.TestMatchResourceAttr(resourceName, "invitation_id", regexp.MustCompile(`^[0-9]+$`)), ), }, - { - Config: testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName), - Check: resource.ComposeTestCheckFunc( - testAccCheckGithubRepositoryCollaboratorExists(resourceName), - ), - }, }, }) } From 19b5f88d5ef09ba8e77618f1a56a03323e9f93e8 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Thu, 20 Jun 2019 14:39:00 -0500 Subject: [PATCH 09/15] fix repository collaborator test --- ...resource_github_repository_collaborator.go | 19 ++++---- ...rce_github_repository_collaborator_test.go | 48 +++++++++++++------ 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/github/resource_github_repository_collaborator.go b/github/resource_github_repository_collaborator.go index e28eb17867..874a8102d5 100644 --- a/github/resource_github_repository_collaborator.go +++ b/github/resource_github_repository_collaborator.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log" + "strings" "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/schema" @@ -84,7 +85,7 @@ func resourceGithubRepositoryCollaboratorRead(d *schema.ResourceData, meta inter ctx := context.WithValue(context.Background(), ctxId, d.Id()) // First, check if the user has been invited but has not yet accepted - invitation, err := findRepoInvitation(client, ctx, orgName, repoName, username) + invitation, username, err := findRepoInvitation(client, ctx, orgName, repoName, username) if err != nil { return err } else if invitation != nil { @@ -113,14 +114,14 @@ func resourceGithubRepositoryCollaboratorRead(d *schema.ResourceData, meta inter } for _, c := range collaborators { - if *c.Login == username { + if strings.ToLower(*c.Login) == strings.ToLower(username) { permissionName, err := getRepoPermission(c.Permissions) if err != nil { return err } d.Set("repository", repoName) - d.Set("username", username) + d.Set("username", *c.Login) d.Set("permission", permissionName) return nil } @@ -150,7 +151,7 @@ func resourceGithubRepositoryCollaboratorDelete(d *schema.ResourceData, meta int ctx := context.WithValue(context.Background(), ctxId, d.Id()) // Delete any pending invitations - invitation, err := findRepoInvitation(client, ctx, orgName, repoName, username) + invitation, username, err := findRepoInvitation(client, ctx, orgName, repoName, username) if err != nil { return err } else if invitation != nil { @@ -164,17 +165,17 @@ func resourceGithubRepositoryCollaboratorDelete(d *schema.ResourceData, meta int return err } -func findRepoInvitation(client *github.Client, ctx context.Context, owner, repo, collaborator string) (*github.RepositoryInvitation, error) { +func findRepoInvitation(client *github.Client, ctx context.Context, owner, repo, collaborator string) (*github.RepositoryInvitation, string, error) { opt := &github.ListOptions{PerPage: maxPerPage} for { invitations, resp, err := client.Repositories.ListInvitations(ctx, owner, repo, opt) if err != nil { - return nil, err + return nil, collaborator, err } for _, i := range invitations { - if *i.Invitee.Login == collaborator { - return i, nil + if strings.ToLower(*i.Invitee.Login) == strings.ToLower(collaborator) { + return i, *i.Invitee.Login, nil } } @@ -183,5 +184,5 @@ func findRepoInvitation(client *github.Client, ctx context.Context, owner, repo, } opt.Page = resp.NextPage } - return nil, nil + return nil, collaborator, nil } diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index 1f84abee3b..2dad278718 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -3,6 +3,8 @@ package github import ( "context" "fmt" + "github.com/hashicorp/terraform/helper/schema" + "os" "regexp" "testing" "unicode" @@ -40,18 +42,26 @@ func TestAccGithubRepositoryCollaborator_caseInsensitive(t *testing.T) { if len(testCollaborator) == 0 { t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is 0") } - resourceName := "github_repository_collaborator.test_repo_collaborator" + + inviteeToken := os.Getenv("GITHUB_TEST_COLLABORATOR_TOKEN") + if inviteeToken == "" { + t.Skip("GITHUB_TEST_COLLABORATOR_TOKEN was not provided, skipping test") + } + + resourceName := "github_repository_collaborator.test" repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5)) + var providers []*schema.Provider + resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckGithubRepositoryCollaboratorDestroy, + PreCheck: func() { testAccPreCheck(t) }, + ProviderFactories: testAccProviderFactories(&providers), + CheckDestroy: func(s *terraform.State) error { return nil }, Steps: []resource.TestStep{ { - Config: testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName), + Config: testAccGithubRepositoryCollaboratorConfig_caseInsensitive(inviteeToken, repoName), Check: resource.ComposeTestCheckFunc( - testAccCheckGithubRepositoryCollaboratorExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "username", testCollaborator), ), }, }, @@ -206,7 +216,7 @@ resource "github_repository" "test" { `, repoName, testCollaborator, expectedPermission) } -func testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName string) string { +func testAccGithubRepositoryCollaboratorConfig_caseInsensitive(inviteeToken, repoName string) string { otherCase := []rune(testCollaborator) if unicode.IsUpper(otherCase[0]) { otherCase[0] = unicode.ToLower(otherCase[0]) @@ -214,14 +224,24 @@ func testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName string) otherCase[0] = unicode.ToUpper(otherCase[0]) } return fmt.Sprintf(` +provider "github" { + alias = "invitee" + token = "%s" +} + resource "github_repository" "test" { - name = "%s" + name = "%s" } - resource "github_repository_collaborator" "test_repo_collaborator" { - repository = "${github_repository.test.name}" - username = "%s" - permission = "%s" - } -`, repoName, string(otherCase), expectedPermission) +resource "github_repository_collaborator" "test" { + repository = "${github_repository.test.name}" + username = "%s" + permission = "push" +} + +resource "github_user_invitation_accepter" "test" { + provider = "github.invitee" + invitation_id = "${github_repository_collaborator.test.invitation_id}" +} +`, inviteeToken, repoName, string(otherCase)) } From a68fc4cb8c493eb39b23e3dff03b54aa12aed9b4 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 24 Jun 2019 12:03:09 -0500 Subject: [PATCH 10/15] updates after review comments --- github/resource_github_membership_test.go | 2 +- ...resource_github_repository_collaborator.go | 16 +-- ...rce_github_repository_collaborator_test.go | 100 +++++++++++++----- .../resource_github_team_membership_test.go | 2 +- 4 files changed, 82 insertions(+), 38 deletions(-) diff --git a/github/resource_github_membership_test.go b/github/resource_github_membership_test.go index 55cd7fddcd..5a2d5b9996 100644 --- a/github/resource_github_membership_test.go +++ b/github/resource_github_membership_test.go @@ -31,7 +31,7 @@ func TestAccGithubMembership_basic(t *testing.T) { } func TestAccGithubMembership_caseInsensitive(t *testing.T) { - if len(testCollaborator) == 0 { + if testCollaborator == "" { t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is 0") } var membership github.Membership diff --git a/github/resource_github_repository_collaborator.go b/github/resource_github_repository_collaborator.go index 874a8102d5..1e0c13011d 100644 --- a/github/resource_github_repository_collaborator.go +++ b/github/resource_github_repository_collaborator.go @@ -85,10 +85,11 @@ func resourceGithubRepositoryCollaboratorRead(d *schema.ResourceData, meta inter ctx := context.WithValue(context.Background(), ctxId, d.Id()) // First, check if the user has been invited but has not yet accepted - invitation, username, err := findRepoInvitation(client, ctx, orgName, repoName, username) + invitation, err := findRepoInvitation(client, ctx, orgName, repoName, username) if err != nil { return err } else if invitation != nil { + username = *invitation.Invitee.Login permissionName, err := getInvitationPermission(invitation) if err != nil { return err @@ -121,7 +122,7 @@ func resourceGithubRepositoryCollaboratorRead(d *schema.ResourceData, meta inter } d.Set("repository", repoName) - d.Set("username", *c.Login) + d.Set("username", c.Login) d.Set("permission", permissionName) return nil } @@ -151,10 +152,11 @@ func resourceGithubRepositoryCollaboratorDelete(d *schema.ResourceData, meta int ctx := context.WithValue(context.Background(), ctxId, d.Id()) // Delete any pending invitations - invitation, username, err := findRepoInvitation(client, ctx, orgName, repoName, username) + invitation, err := findRepoInvitation(client, ctx, orgName, repoName, username) if err != nil { return err } else if invitation != nil { + username = *invitation.Invitee.Login _, err = client.Repositories.DeleteInvitation(ctx, orgName, repoName, *invitation.ID) return err } @@ -165,17 +167,17 @@ func resourceGithubRepositoryCollaboratorDelete(d *schema.ResourceData, meta int return err } -func findRepoInvitation(client *github.Client, ctx context.Context, owner, repo, collaborator string) (*github.RepositoryInvitation, string, error) { +func findRepoInvitation(client *github.Client, ctx context.Context, owner, repo, collaborator string) (*github.RepositoryInvitation, error) { opt := &github.ListOptions{PerPage: maxPerPage} for { invitations, resp, err := client.Repositories.ListInvitations(ctx, owner, repo, opt) if err != nil { - return nil, collaborator, err + return nil, err } for _, i := range invitations { if strings.ToLower(*i.Invitee.Login) == strings.ToLower(collaborator) { - return i, *i.Invitee.Login, nil + return i, nil } } @@ -184,5 +186,5 @@ func findRepoInvitation(client *github.Client, ctx context.Context, owner, repo, } opt.Page = resp.NextPage } - return nil, collaborator, nil + return nil, nil } diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index 2dad278718..606ba288d3 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -2,13 +2,14 @@ package github import ( "context" + "errors" "fmt" - "github.com/hashicorp/terraform/helper/schema" - "os" "regexp" + "strings" "testing" "unicode" + "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" @@ -39,31 +40,44 @@ func TestAccGithubRepositoryCollaborator_basic(t *testing.T) { } func TestAccGithubRepositoryCollaborator_caseInsensitive(t *testing.T) { - if len(testCollaborator) == 0 { + if testCollaborator == "" { t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is 0") } - inviteeToken := os.Getenv("GITHUB_TEST_COLLABORATOR_TOKEN") - if inviteeToken == "" { - t.Skip("GITHUB_TEST_COLLABORATOR_TOKEN was not provided, skipping test") - } - resourceName := "github_repository_collaborator.test" repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5)) - var providers []*schema.Provider + var origInvitation github.RepositoryInvitation + var otherInvitation github.RepositoryInvitation + + oc := []rune(testCollaborator) + if unicode.IsUpper(oc[0]) { + oc[0] = unicode.ToLower(oc[0]) + } else { + oc[0] = unicode.ToUpper(oc[0]) + } + otherCase := string(oc) resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ProviderFactories: testAccProviderFactories(&providers), - CheckDestroy: func(s *terraform.State) error { return nil }, + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: func(s *terraform.State) error { return nil }, Steps: []resource.TestStep{ { - Config: testAccGithubRepositoryCollaboratorConfig_caseInsensitive(inviteeToken, repoName), + Config: testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName, otherCase), Check: resource.ComposeTestCheckFunc( + testAccCheckGithubRepositoryCollaboratorInvited(repoName, otherCase, &otherInvitation), resource.TestCheckResourceAttr(resourceName, "username", testCollaborator), ), }, + { + Config: testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName, testCollaborator), + Check: resource.ComposeTestCheckFunc( + testAccCheckGithubRepositoryCollaboratorInvited(repoName, testCollaborator, &origInvitation), + resource.TestCheckResourceAttr(resourceName, "username", testCollaborator), + testAccGithubRepositoryCollaboratorTheSame(&origInvitation, &otherInvitation), + ), + }, }, }) } @@ -216,19 +230,8 @@ resource "github_repository" "test" { `, repoName, testCollaborator, expectedPermission) } -func testAccGithubRepositoryCollaboratorConfig_caseInsensitive(inviteeToken, repoName string) string { - otherCase := []rune(testCollaborator) - if unicode.IsUpper(otherCase[0]) { - otherCase[0] = unicode.ToLower(otherCase[0]) - } else { - otherCase[0] = unicode.ToUpper(otherCase[0]) - } +func testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName, collaborator string) string { return fmt.Sprintf(` -provider "github" { - alias = "invitee" - token = "%s" -} - resource "github_repository" "test" { name = "%s" } @@ -238,10 +241,49 @@ resource "github_repository_collaborator" "test" { username = "%s" permission = "push" } +`, repoName, collaborator) +} + +func testAccCheckGithubRepositoryCollaboratorInvited(repoName, username string, invitation *github.RepositoryInvitation) resource.TestCheckFunc { + return func(s *terraform.State) error { + opt := &github.ListOptions{PerPage: maxPerPage} -resource "github_user_invitation_accepter" "test" { - provider = "github.invitee" - invitation_id = "${github_repository_collaborator.test.invitation_id}" + client := testAccProvider.Meta().(*Organization).client + org := testAccProvider.Meta().(*Organization).name + + for { + invitations, resp, err := client.Repositories.ListInvitations(context.TODO(), org, repoName, opt) + if err != nil { + return errors.New(err.Error()) + } + + if len(invitations) > 1 { + return errors.New(fmt.Sprintf("multiple invitations have been sent for repository %s", repoName)) + } + + for _, i := range invitations { + if strings.ToLower(*i.Invitee.Login) == strings.ToLower(username) { + invitation = i + return nil + } + } + + if resp.NextPage == 0 { + break + } + opt.Page = resp.NextPage + } + + return errors.New(fmt.Sprintf("no invitation found for %s", username)) + } } -`, inviteeToken, repoName, string(otherCase)) + +func testAccGithubRepositoryCollaboratorTheSame(orig, other *github.RepositoryInvitation) resource.TestCheckFunc { + return func(s *terraform.State) error { + if orig.ID != other.ID { + return errors.New("collaborators are different") + } + + return nil + } } diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go index c0f43ccca9..9aca115db1 100644 --- a/github/resource_github_team_membership_test.go +++ b/github/resource_github_team_membership_test.go @@ -41,7 +41,7 @@ func TestAccGithubTeamMembership_basic(t *testing.T) { } func TestAccGithubTeamMembership_caseInsensitive(t *testing.T) { - if len(testCollaborator) == 0 { + if testCollaborator == "" { t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is 0") } var membership github.Membership From 793befc86a77a60dc5e80ec2cc363f454eaf0353 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 24 Jun 2019 13:44:16 -0500 Subject: [PATCH 11/15] updates after review comments --- github/resource_github_membership_test.go | 64 +++++++++---------- ...rce_github_repository_collaborator_test.go | 61 ++++-------------- .../resource_github_team_membership_test.go | 63 +++++++----------- 3 files changed, 67 insertions(+), 121 deletions(-) diff --git a/github/resource_github_membership_test.go b/github/resource_github_membership_test.go index 5a2d5b9996..452f3b5950 100644 --- a/github/resource_github_membership_test.go +++ b/github/resource_github_membership_test.go @@ -2,6 +2,7 @@ package github import ( "context" + "errors" "fmt" "testing" "unicode" @@ -12,7 +13,21 @@ import ( ) func TestAccGithubMembership_basic(t *testing.T) { + if testCollaborator == "" { + t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is not set") + } + var membership github.Membership + var otherMembership github.Membership + + oc := []rune(testCollaborator) + if unicode.IsUpper(oc[0]) { + oc[0] = unicode.ToLower(oc[0]) + } else { + oc[0] = unicode.ToUpper(oc[0]) + } + + otherCase := string(oc) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -20,31 +35,17 @@ func TestAccGithubMembership_basic(t *testing.T) { CheckDestroy: testAccCheckGithubMembershipDestroy, Steps: []resource.TestStep{ { - Config: testAccGithubMembershipConfig, + Config: testAccGithubMembershipConfig(testCollaborator), Check: resource.ComposeTestCheckFunc( testAccCheckGithubMembershipExists("github_membership.test_org_membership", &membership), testAccCheckGithubMembershipRoleState("github_membership.test_org_membership", &membership), ), }, - }, - }) -} - -func TestAccGithubMembership_caseInsensitive(t *testing.T) { - if testCollaborator == "" { - t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is 0") - } - var membership github.Membership - - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckGithubMembershipDestroy, - Steps: []resource.TestStep{ { - Config: testAccGithubMembershipConfig_caseInsensitive(), + Config: testAccGithubMembershipConfig(otherCase), Check: resource.ComposeTestCheckFunc( - testAccCheckGithubMembershipExists("github_membership.test_org_membership", &membership), + testAccCheckGithubMembershipExists("github_membership.test_org_membership", &otherMembership), + testAccGithubMembershipTheSame(&membership, &otherMembership), ), }, }, @@ -58,7 +59,7 @@ func TestAccGithubMembership_importBasic(t *testing.T) { CheckDestroy: testAccCheckGithubMembershipDestroy, Steps: []resource.TestStep{ { - Config: testAccGithubMembershipConfig, + Config: testAccGithubMembershipConfig(testCollaborator), }, { ResourceName: "github_membership.test_org_membership", @@ -156,24 +157,21 @@ func testAccCheckGithubMembershipRoleState(n string, membership *github.Membersh } } -var testAccGithubMembershipConfig string = fmt.Sprintf(` +func testAccGithubMembershipConfig(username string) string { + return fmt.Sprintf(` resource "github_membership" "test_org_membership" { username = "%s" role = "member" } -`, testCollaborator) +`, username) +} -func testAccGithubMembershipConfig_caseInsensitive() string { - otherCase := []rune(testCollaborator) - if unicode.IsUpper(otherCase[0]) { - otherCase[0] = unicode.ToLower(otherCase[0]) - } else { - otherCase[0] = unicode.ToUpper(otherCase[0]) +func testAccGithubMembershipTheSame(orig, other *github.Membership) resource.TestCheckFunc { + return func(s *terraform.State) error { + if *orig.URL != *other.URL { + return errors.New("users are different") + } + + return nil } - return fmt.Sprintf(` - resource "github_membership" "test_org_membership" { - username = "%s" - role = "member" - } -`, string(otherCase)) } diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index 606ba288d3..87ad26d67f 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -18,33 +18,11 @@ import ( const expectedPermission string = "admin" func TestAccGithubRepositoryCollaborator_basic(t *testing.T) { - resourceName := "github_repository_collaborator.test_repo_collaborator" - repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5)) - - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckGithubRepositoryCollaboratorDestroy, - Steps: []resource.TestStep{ - { - Config: testAccGithubRepositoryCollaboratorConfig(repoName), - Check: resource.ComposeTestCheckFunc( - testAccCheckGithubRepositoryCollaboratorExists(resourceName), - testAccCheckGithubRepositoryCollaboratorPermission(resourceName), - resource.TestCheckResourceAttr(resourceName, "permission", expectedPermission), - resource.TestMatchResourceAttr(resourceName, "invitation_id", regexp.MustCompile(`^[0-9]+$`)), - ), - }, - }, - }) -} - -func TestAccGithubRepositoryCollaborator_caseInsensitive(t *testing.T) { if testCollaborator == "" { - t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is 0") + t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is not set") } - resourceName := "github_repository_collaborator.test" + resourceName := "github_repository_collaborator.test_repo_collaborator" repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5)) var origInvitation github.RepositoryInvitation @@ -61,19 +39,22 @@ func TestAccGithubRepositoryCollaborator_caseInsensitive(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, - CheckDestroy: func(s *terraform.State) error { return nil }, + CheckDestroy: testAccCheckGithubRepositoryCollaboratorDestroy, Steps: []resource.TestStep{ { - Config: testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName, otherCase), + Config: testAccGithubRepositoryCollaboratorConfig(repoName, testCollaborator), Check: resource.ComposeTestCheckFunc( - testAccCheckGithubRepositoryCollaboratorInvited(repoName, otherCase, &otherInvitation), - resource.TestCheckResourceAttr(resourceName, "username", testCollaborator), + testAccCheckGithubRepositoryCollaboratorExists(resourceName), + testAccCheckGithubRepositoryCollaboratorInvited(repoName, testCollaborator, &origInvitation), + testAccCheckGithubRepositoryCollaboratorPermission(resourceName), + resource.TestCheckResourceAttr(resourceName, "permission", expectedPermission), + resource.TestMatchResourceAttr(resourceName, "invitation_id", regexp.MustCompile(`^[0-9]+$`)), ), }, { - Config: testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName, testCollaborator), + Config: testAccGithubRepositoryCollaboratorConfig(repoName, otherCase), Check: resource.ComposeTestCheckFunc( - testAccCheckGithubRepositoryCollaboratorInvited(repoName, testCollaborator, &origInvitation), + testAccCheckGithubRepositoryCollaboratorInvited(repoName, otherCase, &otherInvitation), resource.TestCheckResourceAttr(resourceName, "username", testCollaborator), testAccGithubRepositoryCollaboratorTheSame(&origInvitation, &otherInvitation), ), @@ -91,7 +72,7 @@ func TestAccGithubRepositoryCollaborator_importBasic(t *testing.T) { CheckDestroy: testAccCheckGithubRepositoryCollaboratorDestroy, Steps: []resource.TestStep{ { - Config: testAccGithubRepositoryCollaboratorConfig(repoName), + Config: testAccGithubRepositoryCollaboratorConfig(repoName, testCollaborator), }, { ResourceName: "github_repository_collaborator.test_repo_collaborator", @@ -216,7 +197,7 @@ func testAccCheckGithubRepositoryCollaboratorPermission(n string) resource.TestC } } -func testAccGithubRepositoryCollaboratorConfig(repoName string) string { +func testAccGithubRepositoryCollaboratorConfig(repoName, username string) string { return fmt.Sprintf(` resource "github_repository" "test" { name = "%s" @@ -227,21 +208,7 @@ resource "github_repository" "test" { username = "%s" permission = "%s" } -`, repoName, testCollaborator, expectedPermission) -} - -func testAccGithubRepositoryCollaboratorConfig_caseInsensitive(repoName, collaborator string) string { - return fmt.Sprintf(` -resource "github_repository" "test" { - name = "%s" -} - -resource "github_repository_collaborator" "test" { - repository = "${github_repository.test.name}" - username = "%s" - permission = "push" -} -`, repoName, collaborator) +`, repoName, username, expectedPermission) } func testAccCheckGithubRepositoryCollaboratorInvited(repoName, username string, invitation *github.RepositoryInvitation) resource.TestCheckFunc { diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go index 9aca115db1..b443f7fe2e 100644 --- a/github/resource_github_team_membership_test.go +++ b/github/resource_github_team_membership_test.go @@ -2,6 +2,7 @@ package github import ( "context" + "errors" "fmt" "strconv" "testing" @@ -15,8 +16,19 @@ import ( func TestAccGithubTeamMembership_basic(t *testing.T) { var membership github.Membership + var otherMembership github.Membership + randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + oc := []rune(testCollaborator) + if unicode.IsUpper(oc[0]) { + oc[0] = unicode.ToLower(oc[0]) + } else { + oc[0] = unicode.ToUpper(oc[0]) + } + + otherCase := string(oc) + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -36,26 +48,11 @@ func TestAccGithubTeamMembership_basic(t *testing.T) { testAccCheckGithubTeamMembershipRoleState("github_team_membership.test_team_membership", "maintainer", &membership), ), }, - }, - }) -} - -func TestAccGithubTeamMembership_caseInsensitive(t *testing.T) { - if testCollaborator == "" { - t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is 0") - } - var membership github.Membership - randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) - - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckGithubTeamMembershipDestroy, - Steps: []resource.TestStep{ { - Config: testAccGithubTeamMembershipConfig_caseInsensitive(randString, testCollaborator, "member"), + Config: testAccGithubTeamMembershipConfig(randString, otherCase, "maintainer"), Check: resource.ComposeTestCheckFunc( - testAccCheckGithubTeamMembershipExists("github_team_membership.test_team_membership", &membership), + testAccCheckGithubTeamMembershipExists("github_team_membership.test_team_membership", &otherMembership), + testAccGithubTeamMembershipTheSame(&membership, &otherMembership), ), }, }, @@ -208,28 +205,12 @@ resource "github_team_membership" "test_team_membership" { `, username, randString, username, role) } -func testAccGithubTeamMembershipConfig_caseInsensitive(randString, username, role string) string { - otherCase := []rune(testCollaborator) - if unicode.IsUpper(otherCase[0]) { - otherCase[0] = unicode.ToLower(otherCase[0]) - } else { - otherCase[0] = unicode.ToUpper(otherCase[0]) - } - return fmt.Sprintf(` -resource "github_membership" "test_org_membership" { - username = "%s" - role = "member" -} - -resource "github_team" "test_team" { - name = "tf-acc-test-team-membership-%s" - description = "Terraform acc test group" -} +func testAccGithubTeamMembershipTheSame(orig, other *github.Membership) resource.TestCheckFunc { + return func(s *terraform.State) error { + if *orig.URL != *other.URL { + return errors.New("users are different") + } -resource "github_team_membership" "test_team_membership" { - team_id = "${github_team.test_team.id}" - username = "%s" - role = "%s" -} -`, string(otherCase), randString, string(otherCase), role) + return nil + } } From 7e8fa4076623d61545df374fd5bb2e04faf76520 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 24 Jun 2019 13:55:55 -0500 Subject: [PATCH 12/15] indent --- github/resource_github_repository_collaborator_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index 87ad26d67f..493b44c50e 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -203,11 +203,11 @@ resource "github_repository" "test" { name = "%s" } - resource "github_repository_collaborator" "test_repo_collaborator" { +resource "github_repository_collaborator" "test_repo_collaborator" { repository = "${github_repository.test.name}" username = "%s" permission = "%s" - } +} `, repoName, username, expectedPermission) } From 37fed314d204e4c65b655128c2a2a6aea3818eed Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 24 Jun 2019 14:13:59 -0500 Subject: [PATCH 13/15] adding test collaborator check to test --- github/resource_github_team_membership_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go index 52a21b6341..c9763031e9 100644 --- a/github/resource_github_team_membership_test.go +++ b/github/resource_github_team_membership_test.go @@ -15,6 +15,10 @@ import ( ) func TestAccGithubTeamMembership_basic(t *testing.T) { + if testCollaborator == "" { + t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is not set") + } + var membership github.Membership var otherMembership github.Membership From 34e594c6e925ac6c23d2ac15a2d222af9a208cc9 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 24 Jun 2019 15:07:49 -0500 Subject: [PATCH 14/15] removing 'length of' from skip message --- github/resource_github_membership_test.go | 2 +- github/resource_github_repository_collaborator_test.go | 2 +- github/resource_github_team_membership_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/github/resource_github_membership_test.go b/github/resource_github_membership_test.go index 452f3b5950..f403c3c0d4 100644 --- a/github/resource_github_membership_test.go +++ b/github/resource_github_membership_test.go @@ -14,7 +14,7 @@ import ( func TestAccGithubMembership_basic(t *testing.T) { if testCollaborator == "" { - t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is not set") + t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` is not set") } var membership github.Membership diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index f55737cba5..dfaa3b96de 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -19,7 +19,7 @@ const expectedPermission string = "admin" func TestAccGithubRepositoryCollaborator_basic(t *testing.T) { if testCollaborator == "" { - t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is not set") + t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` is not set") } resourceName := "github_repository_collaborator.test_repo_collaborator" diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go index c9763031e9..8029c0182b 100644 --- a/github/resource_github_team_membership_test.go +++ b/github/resource_github_team_membership_test.go @@ -16,7 +16,7 @@ import ( func TestAccGithubTeamMembership_basic(t *testing.T) { if testCollaborator == "" { - t.Skip("Skipping because length of `GITHUB_TEST_COLLABORATOR` is not set") + t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` is not set") } var membership github.Membership From d995f5458d2f98b34c77ee3f74516759a01c0ac4 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 24 Jun 2019 16:20:10 -0500 Subject: [PATCH 15/15] update tests for cases for first char is not a letter --- github/resource_github_membership_test.go | 36 +++++++++++---- ...rce_github_repository_collaborator_test.go | 41 ++++++++++++----- .../resource_github_team_membership_test.go | 44 ++++++++++++++----- github/util_test.go | 19 ++++++++ 4 files changed, 108 insertions(+), 32 deletions(-) diff --git a/github/resource_github_membership_test.go b/github/resource_github_membership_test.go index f403c3c0d4..211f1978c6 100644 --- a/github/resource_github_membership_test.go +++ b/github/resource_github_membership_test.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "testing" - "unicode" "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/resource" @@ -18,16 +17,36 @@ func TestAccGithubMembership_basic(t *testing.T) { } var membership github.Membership - var otherMembership github.Membership - oc := []rune(testCollaborator) - if unicode.IsUpper(oc[0]) { - oc[0] = unicode.ToLower(oc[0]) - } else { - oc[0] = unicode.ToUpper(oc[0]) + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckGithubMembershipDestroy, + Steps: []resource.TestStep{ + { + Config: testAccGithubMembershipConfig(testCollaborator), + Check: resource.ComposeTestCheckFunc( + testAccCheckGithubMembershipExists("github_membership.test_org_membership", &membership), + testAccCheckGithubMembershipRoleState("github_membership.test_org_membership", &membership), + ), + }, + }, + }) +} + +func TestAccGithubMembership_caseInsensitive(t *testing.T) { + if testCollaborator == "" { + t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` is not set") } - otherCase := string(oc) + var membership github.Membership + var otherMembership github.Membership + + otherCase := flipUsernameCase(testCollaborator) + + if testCollaborator == otherCase { + t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` has no letters to flip case") + } resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -38,7 +57,6 @@ func TestAccGithubMembership_basic(t *testing.T) { Config: testAccGithubMembershipConfig(testCollaborator), Check: resource.ComposeTestCheckFunc( testAccCheckGithubMembershipExists("github_membership.test_org_membership", &membership), - testAccCheckGithubMembershipRoleState("github_membership.test_org_membership", &membership), ), }, { diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index dfaa3b96de..8245e931ec 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -7,7 +7,6 @@ import ( "regexp" "strings" "testing" - "unicode" "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/acctest" @@ -25,16 +24,40 @@ func TestAccGithubRepositoryCollaborator_basic(t *testing.T) { resourceName := "github_repository_collaborator.test_repo_collaborator" repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5)) + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckGithubRepositoryCollaboratorDestroy, + Steps: []resource.TestStep{ + { + Config: testAccGithubRepositoryCollaboratorConfig(repoName, testCollaborator), + Check: resource.ComposeTestCheckFunc( + testAccCheckGithubRepositoryCollaboratorExists(resourceName), + testAccCheckGithubRepositoryCollaboratorPermission(resourceName), + resource.TestCheckResourceAttr(resourceName, "permission", expectedPermission), + resource.TestMatchResourceAttr(resourceName, "invitation_id", regexp.MustCompile(`^[0-9]+$`)), + ), + }, + }, + }) +} + +func TestAccGithubRepositoryCollaborator_caseInsensitive(t *testing.T) { + if testCollaborator == "" { + t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` is not set") + } + + resourceName := "github_repository_collaborator.test_repo_collaborator" + repoName := fmt.Sprintf("tf-acc-test-collab-%s", acctest.RandString(5)) + var origInvitation github.RepositoryInvitation var otherInvitation github.RepositoryInvitation - oc := []rune(testCollaborator) - if unicode.IsUpper(oc[0]) { - oc[0] = unicode.ToLower(oc[0]) - } else { - oc[0] = unicode.ToUpper(oc[0]) + otherCase := flipUsernameCase(testCollaborator) + + if testCollaborator == otherCase { + t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` has no letters to flip case") } - otherCase := string(oc) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -44,11 +67,7 @@ func TestAccGithubRepositoryCollaborator_basic(t *testing.T) { { Config: testAccGithubRepositoryCollaboratorConfig(repoName, testCollaborator), Check: resource.ComposeTestCheckFunc( - testAccCheckGithubRepositoryCollaboratorExists(resourceName), testAccCheckGithubRepositoryCollaboratorInvited(repoName, testCollaborator, &origInvitation), - testAccCheckGithubRepositoryCollaboratorPermission(resourceName), - resource.TestCheckResourceAttr(resourceName, "permission", expectedPermission), - resource.TestMatchResourceAttr(resourceName, "invitation_id", regexp.MustCompile(`^[0-9]+$`)), ), }, { diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go index 8029c0182b..9c6ba879d0 100644 --- a/github/resource_github_team_membership_test.go +++ b/github/resource_github_team_membership_test.go @@ -6,7 +6,6 @@ import ( "fmt" "strconv" "testing" - "unicode" "github.com/google/go-github/v25/github" "github.com/hashicorp/terraform/helper/acctest" @@ -20,19 +19,9 @@ func TestAccGithubTeamMembership_basic(t *testing.T) { } var membership github.Membership - var otherMembership github.Membership randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) - oc := []rune(testCollaborator) - if unicode.IsUpper(oc[0]) { - oc[0] = unicode.ToLower(oc[0]) - } else { - oc[0] = unicode.ToUpper(oc[0]) - } - - otherCase := string(oc) - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -52,8 +41,39 @@ func TestAccGithubTeamMembership_basic(t *testing.T) { testAccCheckGithubTeamMembershipRoleState("github_team_membership.test_team_membership", "maintainer", &membership), ), }, + }, + }) +} + +func TestAccGithubTeamMembership_caseInsensitive(t *testing.T) { + if testCollaborator == "" { + t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` is not set") + } + + var membership github.Membership + var otherMembership github.Membership + + randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + + otherCase := flipUsernameCase(testCollaborator) + + if testCollaborator == otherCase { + t.Skip("Skipping because `GITHUB_TEST_COLLABORATOR` has no letters to flip case") + } + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckGithubTeamMembershipDestroy, + Steps: []resource.TestStep{ + { + Config: testAccGithubTeamMembershipConfig(randString, testCollaborator, "member"), + Check: resource.ComposeTestCheckFunc( + testAccCheckGithubTeamMembershipExists("github_team_membership.test_team_membership", &membership), + ), + }, { - Config: testAccGithubTeamMembershipConfig(randString, otherCase, "maintainer"), + Config: testAccGithubTeamMembershipConfig(randString, otherCase, "member"), Check: resource.ComposeTestCheckFunc( testAccCheckGithubTeamMembershipExists("github_team_membership.test_team_membership", &otherMembership), testAccGithubTeamMembershipTheSame(&membership, &otherMembership), diff --git a/github/util_test.go b/github/util_test.go index 00b29900d7..1f0c33fc27 100644 --- a/github/util_test.go +++ b/github/util_test.go @@ -2,6 +2,7 @@ package github import ( "testing" + "unicode" ) func TestAccGithubUtilRole_validation(t *testing.T) { @@ -56,3 +57,21 @@ func TestAccGithubUtilTwoPartID(t *testing.T) { t.Fatalf("Expected parsed part two bar, actual: %s", parsedPartTwo) } } + +func flipUsernameCase(username string) string { + oc := []rune(username) + + for i, ch := range oc { + if unicode.IsLetter(ch) { + + if unicode.IsUpper(ch) { + oc[i] = unicode.ToLower(ch) + } else { + oc[i] = unicode.ToUpper(ch) + } + break + } + + } + return string(oc) +}