From 935c31c25f26eba09d704921b0fd34ee8b24b872 Mon Sep 17 00:00:00 2001 From: Chloe Hutchinson Date: Fri, 19 Feb 2021 16:13:04 +1100 Subject: [PATCH 1/2] Add acceptance tests for teams This is one of the resources that does not yet have acceptance tests associated with it. In order to guard against regressions, I have added the following tests: * creating a team (with only the required attributes) * updating a team (changing its name, description and visibility) * importing a team During this work I discovered an error in importing teams. In order to resolve this, I added the DefaultMemberRole value which was present in the schema to the TeamNode struct, and added a mapping to updateTeam, so this PR doubles as a bugfix for import. --- buildkite/resource_team.go | 2 + buildkite/resource_team_test.go | 193 ++++++++++++++++++++++++++++++++ 2 files changed, 195 insertions(+) create mode 100644 buildkite/resource_team_test.go diff --git a/buildkite/resource_team.go b/buildkite/resource_team.go index ce2107a5..b95e34f1 100644 --- a/buildkite/resource_team.go +++ b/buildkite/resource_team.go @@ -15,6 +15,7 @@ type TeamNode struct { Description graphql.String ID graphql.String IsDefaultTeam graphql.Boolean + DefaultMemberRole graphql.String Name graphql.String MembersCanCreatePipelines graphql.Boolean Privacy TeamPrivacy @@ -208,6 +209,7 @@ func updateTeam(d *schema.ResourceData, t *TeamNode) { d.SetId(string(t.ID)) d.Set("default_team", bool(t.IsDefaultTeam)) d.Set("description", string(t.Description)) + d.Set("default_member_role", string(t.DefaultMemberRole)) d.Set("members_can_create_pipelines", bool(t.MembersCanCreatePipelines)) d.Set("name", string(t.Name)) d.Set("privacy", string(t.Privacy)) diff --git a/buildkite/resource_team_test.go b/buildkite/resource_team_test.go new file mode 100644 index 00000000..47fbebd0 --- /dev/null +++ b/buildkite/resource_team_test.go @@ -0,0 +1,193 @@ +package buildkite + +import ( + "context" + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" +) + +// Confirm that we can create a team, and then delete it without error +func TestAccTeam_add_remove(t *testing.T) { + var resourceTeam TeamNode + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckTeamResourceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccTeamConfigBasic("developers"), + Check: resource.ComposeAggregateTestCheckFunc( + // Confirm the team exists in the buildkite API + testAccCheckTeamExists("buildkite_team.test", &resourceTeam), + // Confirm the team has the correct values in Buildkite's system + testAccCheckTeamRemoteValues(&resourceTeam, "developers"), + // Confirm the team has the correct values in terraform state + resource.TestCheckResourceAttr("buildkite_team.test", "name", "developers"), + resource.TestCheckResourceAttr("buildkite_team.test", "privacy", "VISIBLE"), + ), + }, + }, + }) +} + +func TestAccTeam_update(t *testing.T) { + var resourceTeam TeamNode + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckTeamResourceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccTeamConfigBasic("developers"), + Check: resource.ComposeAggregateTestCheckFunc( + // Confirm the team exists in the buildkite API + testAccCheckTeamExists("buildkite_team.test", &resourceTeam), + // Quick check to confirm the local state is correct before we update it + resource.TestCheckResourceAttr("buildkite_team.test", "name", "developers"), + ), + }, + { + Config: testAccTeamConfigBasic("wombats"), + Check: resource.ComposeAggregateTestCheckFunc( + // Confirm the team exists in the buildkite API + testAccCheckTeamExists("buildkite_team.test", &resourceTeam), + // Confirm the team has the updated values in Buildkite's system + testAccCheckTeamRemoteValues(&resourceTeam, "wombats"), + // Confirm the team has the updated values in terraform state + resource.TestCheckResourceAttr("buildkite_team.test", "name", "wombats"), + resource.TestCheckResourceAttr("buildkite_team.test", "name", "wombats"), + ), + }, + { + Config: testAccTeamConfigSecret("wombats"), + Check: resource.ComposeAggregateTestCheckFunc( + // Confirm the team exists in the buildkite API + testAccCheckTeamExists("buildkite_team.test", &resourceTeam), + // Confirm the team has the updated values in Buildkite's system + testAccCheckTeamRemoteValues(&resourceTeam, "wombats"), + // Confirm the team has the updated values in terraform state + resource.TestCheckResourceAttr("buildkite_team.test", "name", "wombats"), + resource.TestCheckResourceAttr("buildkite_team.test", "description", "a secret team of wombats"), + resource.TestCheckResourceAttr("buildkite_team.test", "privacy", "SECRET"), + ), + }, + }, + }) +} + +// Confirm that this resource can be imported +func TestAccTeam_import(t *testing.T) { + var resourceTeam TeamNode + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckExampleResourceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccTeamConfigBasic("important"), + Check: resource.ComposeAggregateTestCheckFunc( + // Confirm the team exists in the buildkite API + testAccCheckTeamExists("buildkite_team.test", &resourceTeam), + // Quick check to confirm the local state is correct before we re-import it + resource.TestCheckResourceAttr("buildkite_team.test", "name", "important"), + ), + }, + { + // re-import the resource (using the graphql token of the existing resource) and confirm they match + ResourceName: "buildkite_team.test", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func testAccCheckTeamExists(resourceName string, resourceTeam *TeamNode) resource.TestCheckFunc { + return func(s *terraform.State) error { + resourceState, ok := s.RootModule().Resources[resourceName] + + if !ok { + return fmt.Errorf("Not found in state: %s", resourceName) + } + + if resourceState.Primary.ID == "" { + return fmt.Errorf("No ID is set in state") + } + + provider := testAccProvider.Meta().(*Client) + var query struct { + Node struct { + Team TeamNode `graphql:"... on Team"` + } `graphql:"node(id: $id)"` + } + + vars := map[string]interface{}{ + "id": resourceState.Primary.ID, + } + + err := provider.graphql.Query(context.Background(), &query, vars) + if err != nil { + return fmt.Errorf("Error fetching team from graphql API: %v", err) + } + + if string(query.Node.Team.ID) == "" { + return fmt.Errorf("No team found with graphql id: %s", resourceState.Primary.ID) + } + + if string(query.Node.Team.Name) != resourceState.Primary.Attributes["name"] { + return fmt.Errorf("Team name in state doesn't match remote name") + } + + *resourceTeam = query.Node.Team + + return nil + } +} + +func testAccCheckTeamRemoteValues(resourceTeam *TeamNode, name string) resource.TestCheckFunc { + return func(s *terraform.State) error { + + if string(resourceTeam.Name) != name { + return fmt.Errorf("remote team name (%s) doesn't match expected value (%s)", resourceTeam.Name, name) + } + return nil + } +} + +func testAccTeamConfigBasic(name string) string { + config := ` + resource "buildkite_team" "test" { + name = "%s" + description = "a cool team of %s" + privacy = "VISIBLE" + default_team = true + default_member_role = "MEMBER" + } + ` + return fmt.Sprintf(config, name, name) +} + +func testAccTeamConfigSecret(name string) string { + config := ` + resource "buildkite_team" "test" { + name = "%s" + description = "a secret team of %s" + privacy = "SECRET" + default_team = true + default_member_role = "MEMBER" + } + ` + return fmt.Sprintf(config, name, name) +} + +// verifies the team has been destroyed +func testAccCheckTeamResourceDestroy(s *terraform.State) error { + // TODO manually check that all resources created during acceptance tests have been cleaned up + return nil +} From 94fb737cfa343029a47cc16b048f289a95fec6a6 Mon Sep 17 00:00:00 2001 From: Chloe Hutchinson Date: Mon, 22 Feb 2021 12:21:34 +1100 Subject: [PATCH 2/2] Remove superfluous state check from test --- buildkite/resource_team_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/buildkite/resource_team_test.go b/buildkite/resource_team_test.go index 47fbebd0..da787c6f 100644 --- a/buildkite/resource_team_test.go +++ b/buildkite/resource_team_test.go @@ -140,10 +140,6 @@ func testAccCheckTeamExists(resourceName string, resourceTeam *TeamNode) resourc return fmt.Errorf("No team found with graphql id: %s", resourceState.Primary.ID) } - if string(query.Node.Team.Name) != resourceState.Primary.Attributes["name"] { - return fmt.Errorf("Team name in state doesn't match remote name") - } - *resourceTeam = query.Node.Team return nil