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

Adds acceptance tests for datasources for HVN, Consul and Vault clusters #135

Merged
merged 4 commits into from
Jun 10, 2021
Merged

Adds acceptance tests for datasources for HVN, Consul and Vault clusters #135

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c4d704d
Adds acceptance tests for data sources
lackeyjb Jun 2, 2021
268deea
Move datasource tests to resource tests to decrease test time
lackeyjb Jun 7, 2021
191c55f
Add comments to explain steps in acc tests with datasources added
lackeyjb Jun 8, 2021
b3bce89
Relocate comments
lackeyjb Jun 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 72 additions & 65 deletions contributing/writing-tests.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,56 +110,56 @@ When executing the test, the following steps are taken for each `TestStep`:
`hcp_hvn` is required. This results in configuration which
looks like this:

```hcl
resource "hcp_hvn" "test" {
hvn_id = "test-hvn"
cloud_provider = "aws"
region = "us-west-2"
}
resource "hcp_consul_cluster" "test" {
cluster_id = "test-consul-cluster"
hvn_id = hcp_hvn.test.hvn_id
tier = "development"
}
```
```hcl
resource "hcp_hvn" "test" {
hvn_id = "test-hvn"
cloud_provider = "aws"
region = "us-west-2"
}

resource "hcp_consul_cluster" "test" {
cluster_id = "test-consul-cluster"
hvn_id = hcp_hvn.test.hvn_id
tier = "development"
}
```

1. Assertions are run using the provider API. These use the provider API
directly rather than asserting against the resource state. For example, to
verify that the `hcp_consul_cluster` described above was created
successfully, a test function like this is used:

```go
func testAccCheckConsulClusterExists(name string) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[name]
if !ok {
return fmt.Errorf("not found: %s", name)
}
```go
func testAccCheckConsulClusterExists(name string) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[name]
if !ok {
return fmt.Errorf("not found: %s", name)
}

id := rs.Primary.ID
if id == "" {
return fmt.Errorf("no ID is set")
}
id := rs.Primary.ID
if id == "" {
return fmt.Errorf("no ID is set")
}

client := testAccProvider.Meta().(*clients.Client)
client := testAccProvider.Meta().(*clients.Client)

link, err := buildLinkFromURL(id, ConsulClusterResourceType, client.Config.OrganizationID)
if err != nil {
return fmt.Errorf("unable to build link for %q: %v", id, err)
}
link, err := buildLinkFromURL(id, ConsulClusterResourceType, client.Config.OrganizationID)
if err != nil {
return fmt.Errorf("unable to build link for %q: %v", id, err)
}

clusterID := link.ID
loc := link.Location
clusterID := link.ID
loc := link.Location

if _, err := clients.GetConsulClusterByID(context.Background(), client, loc, clusterID); err != nil {
return fmt.Errorf("unable to read Consul cluster %q: %v", id, err)
}
if _, err := clients.GetConsulClusterByID(context.Background(), client, loc, clusterID); err != nil {
return fmt.Errorf("unable to read Consul cluster %q: %v", id, err)
}

return nil
}
}
```
return nil
}
}
```

Notice that the only information used from the Terraform state is the ID of
the resource - though in this case it is necessary to split the ID into
Expand All @@ -168,9 +168,9 @@ When executing the test, the following steps are taken for each `TestStep`:
expected value if possible. The testing framework provides helper functions
for several common types of check - for example:

```go
resource.TestCheckResourceAttr(resourceName, "cluster_id", "test-consul-cluster"),
```
```go
resource.TestCheckResourceAttr(resourceName, "cluster_id", "test-consul-cluster"),
```

1. The resources created by the test are destroyed. This step happens
automatically, and is the equivalent of calling `terraform destroy`.
Expand All @@ -180,34 +180,41 @@ When executing the test, the following steps are taken for each `TestStep`:
"dangling resources". The code to ensure that the `hcp_consul_cluster` shown
above is removed looks like this:

```go
func testAccCheckConsulClusterDestroy(s *terraform.State) error {
client := testAccProvider.Meta().(*clients.Client)
```go
func testAccCheckConsulClusterDestroy(s *terraform.State) error {
client := testAccProvider.Meta().(*clients.Client)

for _, rs := range s.RootModule().Resources {
switch rs.Type {
case "hcp_consul_cluster":
id := rs.Primary.ID
for _, rs := range s.RootModule().Resources {
switch rs.Type {
case "hcp_consul_cluster":
id := rs.Primary.ID

link, err := buildLinkFromURL(id, ConsulClusterResourceType, client.Config.OrganizationID)
if err != nil {
return fmt.Errorf("unable to build link for %q: %v", id, err)
}
link, err := buildLinkFromURL(id, ConsulClusterResourceType, client.Config.OrganizationID)
if err != nil {
return fmt.Errorf("unable to build link for %q: %v", id, err)
}

clusterID := link.ID
loc := link.Location
clusterID := link.ID
loc := link.Location

_, err = clients.GetConsulClusterByID(context.Background(), client, loc, clusterID)
if err == nil || !clients.IsResponseCodeNotFound(err) {
return fmt.Errorf("didn't get a 404 when reading destroyed Consul cluster %q: %v", id, err)
}
_, err = clients.GetConsulClusterByID(context.Background(), client, loc, clusterID)
if err == nil || !clients.IsResponseCodeNotFound(err) {
return fmt.Errorf("didn't get a 404 when reading destroyed Consul cluster %q: %v", id, err)
}

default:
continue
}
}
return nil
}
```
default:
continue
}
}
return nil
}
```

These functions usually test only for the resource directly under test.

## Test Time and Consolidation

Because of the increased length of time it takes to run acceptance tests, efforts
should be made to not create multiples of the same resource for testing purposes. For
example, datasource tests have been consolidated into their corresponding resource
tests so that resources may be reused.
47 changes: 42 additions & 5 deletions internal/provider/resource_consul_cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// This includes tests against both the resource and the corresponding datasource
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for adding these! last thing: can you bump this down to above L33? Same for the others? I don't usually see comments above the package in go so I think this'll be easy to miss.

// to shorten testing time.
package provider

import (
Expand All @@ -10,29 +12,34 @@ import (
"github.com/hashicorp/terraform-provider-hcp/internal/clients"
)

var (
testAccConsulClusterConfig = fmt.Sprintf(`
var testAccConsulClusterConfig = `
resource "hcp_hvn" "test" {
hvn_id = "test-hvn"
cloud_provider = "aws"
region = "us-west-2"
}

resource "hcp_consul_cluster" "test" {
cluster_id = "test-consul-cluster"
hvn_id = hcp_hvn.test.hvn_id
tier = "development"
}`)
)
}

data "hcp_consul_cluster" "test" {
cluster_id = hcp_consul_cluster.test.cluster_id
}
`

func TestAccConsulCluster(t *testing.T) {
resourceName := "hcp_consul_cluster.test"
dataSourceName := "data.hcp_consul_cluster.test"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t, false) },
ProviderFactories: providerFactories,
CheckDestroy: testAccCheckConsulClusterDestroy,
Steps: []resource.TestStep{
// Tests create
{
Config: testConfig(testAccConsulClusterConfig),
Check: resource.ComposeTestCheckFunc(
Expand Down Expand Up @@ -62,6 +69,7 @@ func TestAccConsulCluster(t *testing.T) {
resource.TestCheckResourceAttrSet(resourceName, "size"),
),
},
// Tests import
{
ResourceName: resourceName,
ImportState: true,
Expand All @@ -76,6 +84,7 @@ func TestAccConsulCluster(t *testing.T) {
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"consul_root_token_accessor_id", "consul_root_token_secret_id"},
},
// Tests read
{
Config: testConfig(testAccConsulClusterConfig),
Check: resource.ComposeTestCheckFunc(
Expand Down Expand Up @@ -105,6 +114,34 @@ func TestAccConsulCluster(t *testing.T) {
resource.TestCheckResourceAttrSet(resourceName, "size"),
),
},
// Tests datasource
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Two things on each of these tests:

  1. Since we have a few test steps now, it'd be great to add some guidance comments at the beginning of each step. I.e. first step testsCreate, second step tests Import, third step tests Updates, and fourth step tests the datasource. It might even be worth an intro comment at the top explaining that the datasource test is incorporated in the resource test.
  2. To have more separation between the test cases, I'm thinking it'd be worth having two TF configs, one with just the resource and the other adding the data source (which we'd use just in this last step).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I split the configs up the tests errored out, so I ended up just adding comments to each step in the tests

Config: testConfig(testAccConsulClusterConfig),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(resourceName, "cluster_id", dataSourceName, "cluster_id"),
resource.TestCheckResourceAttrPair(resourceName, "project_id", dataSourceName, "project_id"),
resource.TestCheckResourceAttrPair(resourceName, "organization_id", dataSourceName, "organization_id"),
resource.TestCheckResourceAttrPair(resourceName, "hvn_id", dataSourceName, "hvn_id"),
resource.TestCheckResourceAttrPair(resourceName, "cloud_provider", dataSourceName, "cloud_provider"),
resource.TestCheckResourceAttrPair(resourceName, "region", dataSourceName, "region"),
resource.TestCheckResourceAttrPair(resourceName, "public_endpoint", dataSourceName, "public_endpoint"),
resource.TestCheckResourceAttrPair(resourceName, "datacenter", dataSourceName, "datacenter"),
resource.TestCheckResourceAttrPair(resourceName, "connect_enabled", dataSourceName, "connect_enabled"),
resource.TestCheckResourceAttrPair(resourceName, "consul_automatic_upgrades", dataSourceName, "consul_automatic_upgrades"),
resource.TestCheckResourceAttrPair(resourceName, "consul_snapshot_interval", dataSourceName, "consul_snapshot_interval"),
resource.TestCheckResourceAttrPair(resourceName, "consul_snapshot_retention", dataSourceName, "consul_snapshot_retention"),
resource.TestCheckResourceAttrPair(resourceName, "consul_config_file", dataSourceName, "consul_config_file"),
resource.TestCheckResourceAttrPair(resourceName, "consul_ca_file", dataSourceName, "consul_ca_file"),
resource.TestCheckResourceAttrPair(resourceName, "consul_version", dataSourceName, "consul_version"),
resource.TestCheckResourceAttrPair(resourceName, "consul_public_endpoint_url", dataSourceName, "consul_public_endpoint_url"),
resource.TestCheckResourceAttrPair(resourceName, "consul_private_endpoint_url", dataSourceName, "consul_private_endpoint_url"),
resource.TestCheckResourceAttrPair(resourceName, "scale", dataSourceName, "scale"),
resource.TestCheckResourceAttrPair(resourceName, "tier", dataSourceName, "tier"),
resource.TestCheckResourceAttrPair(resourceName, "size", dataSourceName, "size"),
resource.TestCheckResourceAttrPair(resourceName, "self_link", dataSourceName, "self_link"),
resource.TestCheckResourceAttrPair(resourceName, "primary_link", dataSourceName, "primary_link"),
),
},
},
})
}
Expand Down
32 changes: 28 additions & 4 deletions internal/provider/resource_hvn_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// This includes tests against both the resource and the corresponding datasource
// to shorten testing time.
package provider

import (
Expand All @@ -12,23 +14,28 @@ import (
"github.com/hashicorp/terraform-provider-hcp/internal/clients"
)

var (
testAccHvnConfig = fmt.Sprintf(`
var testAccHvnConfig = `
resource "hcp_hvn" "test" {
hvn_id = "test-hvn"
cloud_provider = "aws"
region = "us-west-2"
}`)
)
}

data "hcp_hvn" "test" {
hvn_id = hcp_hvn.test.hvn_id
}
`

func TestAccHvn(t *testing.T) {
resourceName := "hcp_hvn.test"
dataSourceName := "data.hcp_hvn.test"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t, false) },
ProviderFactories: providerFactories,
CheckDestroy: testAccCheckHvnDestroy,
Steps: []resource.TestStep{
// Tests create
{
Config: testConfig(testAccHvnConfig),
Check: resource.ComposeTestCheckFunc(
Expand All @@ -44,6 +51,7 @@ func TestAccHvn(t *testing.T) {
testLink(resourceName, "self_link", "test-hvn", HvnResourceType, resourceName),
),
},
// Tests import
{
ResourceName: resourceName,
ImportState: true,
Expand All @@ -57,6 +65,7 @@ func TestAccHvn(t *testing.T) {
},
ImportStateVerify: true,
},
// Tests read
{
Config: testConfig(testAccHvnConfig),
Check: resource.ComposeTestCheckFunc(
Expand All @@ -72,6 +81,21 @@ func TestAccHvn(t *testing.T) {
testLink(resourceName, "self_link", "test-hvn", HvnResourceType, resourceName),
),
},
// Tests datasource
{
Config: testConfig(testAccHvnConfig),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(resourceName, "hvn_id", dataSourceName, "hvn_id"),
resource.TestCheckResourceAttrPair(resourceName, "cloud_provider", dataSourceName, "cloud_provider"),
resource.TestCheckResourceAttrPair(resourceName, "region", dataSourceName, "region"),
resource.TestCheckResourceAttrPair(resourceName, "cidr_block", dataSourceName, "cidr_block"),
resource.TestCheckResourceAttrPair(resourceName, "organization_id", dataSourceName, "organization_id"),
resource.TestCheckResourceAttrPair(resourceName, "project_id", dataSourceName, "project_id"),
resource.TestCheckResourceAttrPair(resourceName, "provider_account_id", dataSourceName, "provider_account_id"),
resource.TestCheckResourceAttrPair(resourceName, "created_at", dataSourceName, "created_at"),
resource.TestCheckResourceAttrPair(resourceName, "self_link", dataSourceName, "self_link"),
),
},
},
})
}
Expand Down
Loading