Skip to content

Commit

Permalink
Merge pull request #6338 from terraform-providers/td-unparam
Browse files Browse the repository at this point in the history
tests/provider: Fix unparam Linting Issues and Enable in TravisCI Testing
  • Loading branch information
bflad authored Nov 4, 2018
2 parents 0bd8c1a + f3ef761 commit 09e8156
Show file tree
Hide file tree
Showing 61 changed files with 230 additions and 241 deletions.
1 change: 1 addition & 0 deletions .gometalinter.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"misspell",
"structcheck",
"unconvert",
"unparam",
"unused",
"varcheck",
"vet"
Expand Down
8 changes: 4 additions & 4 deletions aws/autoscaling_tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ func setAutoscalingTags(conn *autoscaling.AutoScaling, d *schema.ResourceData) e

if d.HasChange("tag") || d.HasChange("tags") {
oraw, nraw := d.GetChange("tag")
o := setToMapByKey(oraw.(*schema.Set), "key")
n := setToMapByKey(nraw.(*schema.Set), "key")
o := setToMapByKey(oraw.(*schema.Set))
n := setToMapByKey(nraw.(*schema.Set))

old, err := autoscalingTagsFromMap(o, resourceID)
if err != nil {
Expand Down Expand Up @@ -262,11 +262,11 @@ func autoscalingTagDescriptionsToSlice(ts []*autoscaling.TagDescription) []map[s
return tags
}

func setToMapByKey(s *schema.Set, key string) map[string]interface{} {
func setToMapByKey(s *schema.Set) map[string]interface{} {
result := make(map[string]interface{})
for _, rawData := range s.List() {
data := rawData.(map[string]interface{})
result[data[key].(string)] = data
result[data["key"].(string)] = data
}

return result
Expand Down
9 changes: 6 additions & 3 deletions aws/awserr.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@ func isAWSErr(err error, code string, message string) bool {
return false
}

// Returns true if the error matches all these conditions:
// IsAWSErrExtended returns true if the error matches all conditions
// * err is of type awserr.Error
// * Error.Code() matches code
// * Error.Message() contains message
// * Error.OrigErr() contains origErrMessage
func isAWSErrExtended(err error, code string, message string, origErrMessage string) bool {
// Note: This function will be moved out of the aws package in the future.
func IsAWSErrExtended(err error, code string, message string, origErrMessage string) bool {
if !isAWSErr(err, code, message) {
return false
}
Expand All @@ -48,7 +49,9 @@ func retryOnAwsCode(code string, f func() (interface{}, error)) (interface{}, er
return resp, err
}

func retryOnAwsCodes(codes []string, f func() (interface{}, error)) (interface{}, error) {
// RetryOnAwsCodes retries AWS error codes for one minute
// Note: This function will be moved out of the aws package in the future.
func RetryOnAwsCodes(codes []string, f func() (interface{}, error)) (interface{}, error) {
var resp interface{}
err := resource.Retry(1*time.Minute, func() *resource.RetryError {
var err error
Expand Down
4 changes: 2 additions & 2 deletions aws/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,13 +380,13 @@ func (c *Config) Client() (interface{}, error) {
}
// RequestError: send request failed
// caused by: Post https://FQDN/: dial tcp: lookup FQDN: no such host
if isAWSErrExtended(r.Error, "RequestError", "send request failed", "no such host") {
if IsAWSErrExtended(r.Error, "RequestError", "send request failed", "no such host") {
log.Printf("[WARN] Disabling retries after next request due to networking issue")
r.Retryable = aws.Bool(false)
}
// RequestError: send request failed
// caused by: Post https://FQDN/: dial tcp IPADDRESS:443: connect: connection refused
if isAWSErrExtended(r.Error, "RequestError", "send request failed", "connection refused") {
if IsAWSErrExtended(r.Error, "RequestError", "send request failed", "connection refused") {
log.Printf("[WARN] Disabling retries after next request due to networking issue")
r.Retryable = aws.Bool(false)
}
Expand Down
4 changes: 3 additions & 1 deletion aws/ecs_task_definition_equivalency.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ import (
"github.com/mitchellh/copystructure"
)

func ecsContainerDefinitionsAreEquivalent(def1, def2 string, isAWSVPC bool) (bool, error) {
// EcsContainerDefinitionsAreEquivalent determines equality between two ECS container definition JSON strings
// Note: This function will be moved out of the aws package in the future.
func EcsContainerDefinitionsAreEquivalent(def1, def2 string, isAWSVPC bool) (bool, error) {
var obj1 containerDefinitions
err := json.Unmarshal([]byte(def1), &obj1)
if err != nil {
Expand Down
12 changes: 6 additions & 6 deletions aws/ecs_task_definition_equivalency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func TestAwsEcsContainerDefinitionsAreEquivalent_basic(t *testing.T) {
}
]`

equal, err := ecsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, false)
equal, err := EcsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, false)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -125,7 +125,7 @@ func TestAwsEcsContainerDefinitionsAreEquivalent_portMappings(t *testing.T) {
}
]`

equal, err := ecsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, false)
equal, err := EcsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, false)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -167,15 +167,15 @@ func TestAwsEcsContainerDefinitionsAreEquivalent_portMappingsIgnoreHostPort(t *t
err error
)

equal, err = ecsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, false)
equal, err = EcsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, false)
if err != nil {
t.Fatal(err)
}
if equal {
t.Fatal("Expected definitions to differ.")
}

equal, err = ecsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, true)
equal, err = EcsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, true)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -428,7 +428,7 @@ func TestAwsEcsContainerDefinitionsAreEquivalent_arrays(t *testing.T) {
]
`

equal, err := ecsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, false)
equal, err := EcsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, false)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -466,7 +466,7 @@ func TestAwsEcsContainerDefinitionsAreEquivalent_negative(t *testing.T) {
}
]`

equal, err := ecsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, false)
equal, err := EcsContainerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, false)
if err != nil {
t.Fatal(err)
}
Expand Down
5 changes: 3 additions & 2 deletions aws/resource_aws_api_gateway_stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func resourceAwsApiGatewayStageUpdate(d *schema.ResourceData, meta interface{})
o, n := d.GetChange("variables")
oldV := o.(map[string]interface{})
newV := n.(map[string]interface{})
operations = append(operations, diffVariablesOps("/variables/", oldV, newV)...)
operations = append(operations, diffVariablesOps(oldV, newV)...)
}
if d.HasChange("access_log_settings") {
accessLogSettings := d.Get("access_log_settings").([]interface{})
Expand Down Expand Up @@ -411,8 +411,9 @@ func resourceAwsApiGatewayStageUpdate(d *schema.ResourceData, meta interface{})
return resourceAwsApiGatewayStageRead(d, meta)
}

func diffVariablesOps(prefix string, oldVars, newVars map[string]interface{}) []*apigateway.PatchOperation {
func diffVariablesOps(oldVars, newVars map[string]interface{}) []*apigateway.PatchOperation {
ops := make([]*apigateway.PatchOperation, 0)
prefix := "/variables/"

for k := range oldVars {
if _, ok := newVars[k]; !ok {
Expand Down
8 changes: 4 additions & 4 deletions aws/resource_aws_athena_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func resourceAwsAthenaDatabaseCreate(d *schema.ResourceData, meta interface{}) e
return err
}

if err := executeAndExpectNoRowsWhenCreate(*resp.QueryExecutionId, d, conn); err != nil {
if err := executeAndExpectNoRowsWhenCreate(*resp.QueryExecutionId, conn); err != nil {
return err
}
d.SetId(d.Get("name").(string))
Expand Down Expand Up @@ -169,13 +169,13 @@ func resourceAwsAthenaDatabaseDelete(d *schema.ResourceData, meta interface{}) e
return err
}

if err := executeAndExpectNoRowsWhenDrop(*resp.QueryExecutionId, d, conn); err != nil {
if err := executeAndExpectNoRowsWhenDrop(*resp.QueryExecutionId, conn); err != nil {
return err
}
return nil
}

func executeAndExpectNoRowsWhenCreate(qeid string, d *schema.ResourceData, conn *athena.Athena) error {
func executeAndExpectNoRowsWhenCreate(qeid string, conn *athena.Athena) error {
rs, err := queryExecutionResult(qeid, conn)
if err != nil {
return err
Expand All @@ -201,7 +201,7 @@ func executeAndExpectMatchingRow(qeid string, dbName string, conn *athena.Athena
return fmt.Errorf("Athena not found database: %s, query result: %s", dbName, flattenAthenaResultSet(rs))
}

func executeAndExpectNoRowsWhenDrop(qeid string, d *schema.ResourceData, conn *athena.Athena) error {
func executeAndExpectNoRowsWhenDrop(qeid string, conn *athena.Athena) error {
rs, err := queryExecutionResult(qeid, conn)
if err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions aws/resource_aws_autoscaling_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ func resourceAwsAutoscalingGroupCreate(d *schema.ResourceData, meta interface{})
if v, ok := d.GetOk("tag"); ok {
var err error
createOpts.Tags, err = autoscalingTagsFromMap(
setToMapByKey(v.(*schema.Set), "key"), resourceID)
setToMapByKey(v.(*schema.Set)), resourceID)
if err != nil {
return err
}
Expand Down Expand Up @@ -566,7 +566,7 @@ func resourceAwsAutoscalingGroupRead(d *schema.ResourceData, meta interface{}) e
var v interface{}

if v, tagOk = d.GetOk("tag"); tagOk {
tags := setToMapByKey(v.(*schema.Set), "key")
tags := setToMapByKey(v.(*schema.Set))
for _, t := range g.Tags {
if _, ok := tags[*t.Key]; ok {
tagList = append(tagList, t)
Expand Down
12 changes: 6 additions & 6 deletions aws/resource_aws_batch_job_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,13 @@ func resourceAwsBatchJobQueueDelete(d *schema.ResourceData, meta interface{}) er
name := d.Get("name").(string)

log.Printf("[DEBUG] Disabling Batch Job Queue %s", name)
err := disableBatchJobQueue(name, 10*time.Minute, conn)
err := disableBatchJobQueue(name, conn)
if err != nil {
return fmt.Errorf("error disabling Batch Job Queue (%s): %s", name, err)
}

log.Printf("[DEBUG] Deleting Batch Job Queue %s", name)
err = deleteBatchJobQueue(name, 10*time.Minute, conn)
err = deleteBatchJobQueue(name, conn)
if err != nil {
return fmt.Errorf("error deleting Batch Job Queue (%s): %s", name, err)
}
Expand All @@ -162,7 +162,7 @@ func createComputeEnvironmentOrder(order []interface{}) (envs []*batch.ComputeEn
return
}

func deleteBatchJobQueue(jobQueue string, timeout time.Duration, conn *batch.Batch) error {
func deleteBatchJobQueue(jobQueue string, conn *batch.Batch) error {
_, err := conn.DeleteJobQueue(&batch.DeleteJobQueueInput{
JobQueue: aws.String(jobQueue),
})
Expand All @@ -174,7 +174,7 @@ func deleteBatchJobQueue(jobQueue string, timeout time.Duration, conn *batch.Bat
Pending: []string{batch.JQStateDisabled, batch.JQStatusDeleting},
Target: []string{batch.JQStatusDeleted},
Refresh: batchJobQueueRefreshStatusFunc(conn, jobQueue),
Timeout: timeout,
Timeout: 10 * time.Minute,
Delay: 10 * time.Second,
MinTimeout: 3 * time.Second,
}
Expand All @@ -183,7 +183,7 @@ func deleteBatchJobQueue(jobQueue string, timeout time.Duration, conn *batch.Bat
return err
}

func disableBatchJobQueue(jobQueue string, timeout time.Duration, conn *batch.Batch) error {
func disableBatchJobQueue(jobQueue string, conn *batch.Batch) error {
_, err := conn.UpdateJobQueue(&batch.UpdateJobQueueInput{
JobQueue: aws.String(jobQueue),
State: aws.String(batch.JQStateDisabled),
Expand All @@ -196,7 +196,7 @@ func disableBatchJobQueue(jobQueue string, timeout time.Duration, conn *batch.Ba
Pending: []string{batch.JQStatusUpdating},
Target: []string{batch.JQStatusValid},
Refresh: batchJobQueueRefreshStatusFunc(conn, jobQueue),
Timeout: timeout,
Timeout: 10 * time.Minute,
Delay: 10 * time.Second,
MinTimeout: 3 * time.Second,
}
Expand Down
9 changes: 4 additions & 5 deletions aws/resource_aws_batch_job_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"strconv"
"strings"
"testing"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/batch"
Expand Down Expand Up @@ -56,14 +55,14 @@ func testSweepBatchJobQueues(region string) error {
}

log.Printf("[INFO] Disabling Batch Job Queue: %s", *name)
err := disableBatchJobQueue(*name, 10*time.Minute, conn)
err := disableBatchJobQueue(*name, conn)
if err != nil {
log.Printf("[ERROR] Failed to disable Batch Job Queue %s: %s", *name, err)
continue
}

log.Printf("[INFO] Deleting Batch Job Queue: %s", *name)
err = deleteBatchJobQueue(*name, 10*time.Minute, conn)
err = deleteBatchJobQueue(*name, conn)
if err != nil {
log.Printf("[ERROR] Failed to delete Batch Job Queue %s: %s", *name, err)
}
Expand Down Expand Up @@ -218,12 +217,12 @@ func testAccCheckBatchJobQueueDisappears(jobQueue *batch.JobQueueDetail) resourc
conn := testAccProvider.Meta().(*AWSClient).batchconn
name := aws.StringValue(jobQueue.JobQueueName)

err := disableBatchJobQueue(name, 10*time.Minute, conn)
err := disableBatchJobQueue(name, conn)
if err != nil {
return fmt.Errorf("error disabling Batch Job Queue (%s): %s", name, err)
}

return deleteBatchJobQueue(name, 10*time.Minute, conn)
return deleteBatchJobQueue(name, conn)
}
}

Expand Down
8 changes: 4 additions & 4 deletions aws/resource_aws_cloudwatch_event_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func resourceAwsCloudWatchEventRule() *schema.Resource {
"event_pattern": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validateEventPatternValue(2048),
ValidateFunc: validateEventPatternValue(),
StateFunc: func(v interface{}) string {
json, _ := structure.NormalizeJsonString(v.(string))
return json
Expand Down Expand Up @@ -281,7 +281,7 @@ func getStringStateFromBoolean(isEnabled bool) string {
return "DISABLED"
}

func validateEventPatternValue(length int) schema.SchemaValidateFunc {
func validateEventPatternValue() schema.SchemaValidateFunc {
return func(v interface{}, k string) (ws []string, errors []error) {
json, err := structure.NormalizeJsonString(v)
if err != nil {
Expand All @@ -294,9 +294,9 @@ func validateEventPatternValue(length int) schema.SchemaValidateFunc {
}

// Check whether the normalized JSON is within the given length.
if len(json) > length {
if len(json) > 2048 {
errors = append(errors, fmt.Errorf(
"%q cannot be longer than %d characters: %q", k, length, json))
"%q cannot be longer than %d characters: %q", k, 2048, json))
}
return
}
Expand Down
17 changes: 5 additions & 12 deletions aws/resource_aws_cloudwatch_event_rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,56 +269,49 @@ func testAccCheckAWSCloudWatchEventRuleDestroy(s *terraform.State) error {

func TestResourceAWSCloudWatchEventRule_validateEventPatternValue(t *testing.T) {
type testCases struct {
Length int
Value string
ErrCount int
}

invalidCases := []testCases{
{
Length: 8,
Value: acctest.RandString(16),
Value: acctest.RandString(2049),
ErrCount: 1,
},
{
Length: 123,
Value: `{"abc":}`,
Value: `not-json`,
ErrCount: 1,
},
{
Length: 1,
Value: `{"abc":["1","2"]}`,
Value: fmt.Sprintf("{%q:[1, 2]}", acctest.RandString(2049)),
ErrCount: 1,
},
}

for _, tc := range invalidCases {
_, errors := validateEventPatternValue(tc.Length)(tc.Value, "event_pattern")
_, errors := validateEventPatternValue()(tc.Value, "event_pattern")
if len(errors) != tc.ErrCount {
t.Fatalf("Expected %q to trigger a validation error.", tc.Value)
}
}

validCases := []testCases{
{
Length: 0,
Value: ``,
ErrCount: 0,
},
{
Length: 2,
Value: `{}`,
ErrCount: 0,
},
{
Length: 18,
Value: `{"abc":["1","2"]}`,
ErrCount: 0,
},
}

for _, tc := range validCases {
_, errors := validateEventPatternValue(tc.Length)(tc.Value, "event_pattern")
_, errors := validateEventPatternValue()(tc.Value, "event_pattern")
if len(errors) != tc.ErrCount {
t.Fatalf("Expected %q not to trigger a validation error.", tc.Value)
}
Expand Down
Loading

0 comments on commit 09e8156

Please sign in to comment.