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.

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

provider: Fix updated govet composites and nilness checks #7993

Merged
merged 3 commits into from
Mar 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ linters:
- gosimple
- ineffassign
- misspell
- staticcheck
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Temporary for now to fix out of memory issue. We can consider adding a separate TravisCI matrix job, running staticcheck by itself, or waiting for staticcheck 2019.1.1 support in golangci-lint:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the memory usage difference locally for me:

# golangci-lint on master
6227824640  maximum resident set size
# golangci-lint 1.15.0
5142007808  maximum resident set size

We should also watch: dominikh/go-tools#419

Copy link

Choose a reason for hiding this comment

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

Hi! golangci/golangci-lint#445 was merged. Also, I've added the section about memory usage into README. You can try to set GOGC=30 for example to trade memory for CPU.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks so much @jirfag 😄 This should be a huge help for us.

- structcheck
- unconvert
- unused
Expand Down
31 changes: 15 additions & 16 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,34 +1,33 @@
dist: trusty
dist: xenial
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was updated to see if it helped with the memory issue since TravisCI does not start services by default on the newer OS. It didn't make the memory issue disappear, but the update is helpful anyways.

sudo: required
services:
- docker
language: go
go:
- "1.12.x"
env:
GOFLAGS=-mod=vendor
- "1.12.x"
env: GOFLAGS=-mod=vendor

git:
depth: 1

install:
# This script is used by the Travis build to install a cookie for
# go.googlesource.com so rate limits are higher when using `go get` to fetch
# packages that live there.
# See: https://github.com/golang/go/issues/12933
- bash scripts/gogetcookie.sh
- make tools
# This script is used by the Travis build to install a cookie for
# go.googlesource.com so rate limits are higher when using `go get` to fetch
# packages that live there.
# See: https://github.com/golang/go/issues/12933
- bash scripts/gogetcookie.sh
- make tools

script:
- make lint
- make test
- make website-lint
- make website-test
- make lint
- make test
- make website-lint
- make website-test

branches:
only:
- master
- master
matrix:
fast_finish: true
allow_failures:
- go: tip
- go: tip
13 changes: 10 additions & 3 deletions aws/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,16 @@ import (
func TestGetSupportedEC2Platforms(t *testing.T) {
ec2Endpoints := []*awsbase.MockEndpoint{
{
Request: &awsbase.MockRequest{"POST", "/", "Action=DescribeAccountAttributes&" +
"AttributeName.1=supported-platforms&Version=2016-11-15"},
Response: &awsbase.MockResponse{200, test_ec2_describeAccountAttributes_response, "text/xml"},
Request: &awsbase.MockRequest{
Method: "POST",
Uri: "/",
Body: "Action=DescribeAccountAttributes&AttributeName.1=supported-platforms&Version=2016-11-15",
},
Response: &awsbase.MockResponse{
StatusCode: 200,
Body: test_ec2_describeAccountAttributes_response,
ContentType: "text/xml",
},
},
}
closeFunc, sess, err := awsbase.GetMockedAwsApiSession("EC2", ec2Endpoints)
Expand Down
7 changes: 2 additions & 5 deletions aws/resource_aws_db_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -1544,11 +1544,8 @@ func resourceAwsDbInstanceRetrieve(id string, conn *rds.RDS) (*rds.DBInstance, e
return nil, fmt.Errorf("Error retrieving DB Instances: %s", err)
}

if len(resp.DBInstances) != 1 ||
*resp.DBInstances[0].DBInstanceIdentifier != id {
if err != nil {
return nil, nil
}
if len(resp.DBInstances) != 1 || resp.DBInstances[0] == nil || aws.StringValue(resp.DBInstances[0].DBInstanceIdentifier) != id {
return nil, nil
}

return resp.DBInstances[0], nil
Expand Down
7 changes: 2 additions & 5 deletions aws/resource_aws_docdb_cluster_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,11 +456,8 @@ func resourceAwsDocDBInstanceRetrieve(id string, conn *docdb.DocDB) (*docdb.DBIn
return nil, fmt.Errorf("Error retrieving DB Instances: %s", err)
}

if len(resp.DBInstances) != 1 ||
*resp.DBInstances[0].DBInstanceIdentifier != id {
if err != nil {
return nil, nil
}
if len(resp.DBInstances) != 1 || resp.DBInstances[0] == nil || aws.StringValue(resp.DBInstances[0].DBInstanceIdentifier) != id {
return nil, nil
}

return resp.DBInstances[0], nil
Expand Down
4 changes: 0 additions & 4 deletions aws/resource_aws_egress_only_internet_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@ func resourceAwsEgressOnlyInternetGatewayCreate(d *schema.ResourceData, meta int

d.SetId(*resp.EgressOnlyInternetGateway.EgressOnlyInternetGatewayId)

if err != nil {
return fmt.Errorf("%s", err)
}

return resourceAwsEgressOnlyInternetGatewayRead(d, meta)
}

Expand Down
17 changes: 8 additions & 9 deletions aws/resource_aws_emr_security_configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,20 @@ func testAccCheckEmrSecurityConfigurationDestroy(s *terraform.State) error {
resp, err := conn.DescribeSecurityConfiguration(&emr.DescribeSecurityConfigurationInput{
Name: aws.String(rs.Primary.ID),
})
if err == nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Invert logic here to match preferred style across codebase of error checking first.

if resp.Name != nil && *resp.Name == rs.Primary.ID {
// assume this means the resource still exists
return fmt.Errorf("Error: EMR Security Configuration still exists: %s", *resp.Name)
}

if isAWSErr(err, "InvalidRequestException", "does not exist") {
return nil
}

// Verify the error is what we want
if err != nil {
if isAWSErr(err, "InvalidRequestException", "does not exist") {
return nil
}
return err
}

if resp != nil && aws.StringValue(resp.Name) == rs.Primary.ID {
return fmt.Errorf("Error: EMR Security Configuration still exists: %s", aws.StringValue(resp.Name))
}

return nil
}

return nil
Expand Down
17 changes: 5 additions & 12 deletions aws/resource_aws_gamelift_game_session_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ func testSweepGameliftGameSessionQueue(region string) error {

out, err := conn.DescribeGameSessionQueues(&gamelift.DescribeGameSessionQueuesInput{})

if testSweepSkipSweepError(err) {
log.Printf("[WARN] Skipping Gamelife Queue sweep for %s: %s", region, err)
return nil
}

if err != nil {
if testSweepSkipSweepError(err) {
log.Printf("[WARN] Skipping Gamelife Queue sweep for %s: %s", region, err)
return nil
}
return fmt.Errorf("error listing Gamelift Session Queue: %s", err)
}

Expand All @@ -57,14 +58,6 @@ func testSweepGameliftGameSessionQueue(region string) error {
}
}

if err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing duplicate logic.

if testSweepSkipSweepError(err) {
log.Printf("[WARN] Skipping Gamelift Session Queue sweep for %s: %s", region, err)
return nil
}
return fmt.Errorf("error listing Gamelift Session Queue: %s", err)
}

return nil
}

Expand Down
9 changes: 5 additions & 4 deletions aws/resource_aws_organizations_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,15 @@ func testAccCheckAwsOrganizationsAccountDestroy(s *terraform.State) error {

resp, err := conn.DescribeAccount(params)

if isAWSErr(err, organizations.ErrCodeAccountNotFoundException, "") {
return nil
}

if err != nil {
if isAWSErr(err, organizations.ErrCodeAccountNotFoundException, "") {
return nil
}
return err
}

if resp == nil && resp.Account != nil {
if resp != nil && resp.Account != nil {
return fmt.Errorf("Bad: Account still exists: %q", rs.Primary.ID)
}
}
Expand Down
9 changes: 5 additions & 4 deletions aws/resource_aws_organizations_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,15 @@ func testAccCheckAwsOrganizationsPolicyDestroy(s *terraform.State) error {

resp, err := conn.DescribePolicy(input)

if isAWSErr(err, organizations.ErrCodePolicyNotFoundException, "") {
return nil
}

if err != nil {
if isAWSErr(err, organizations.ErrCodePolicyNotFoundException, "") {
return nil
}
return err
}

if resp == nil && resp.Policy != nil {
if resp != nil && resp.Policy != nil {
return fmt.Errorf("Policy %q still exists", rs.Primary.ID)
}
}
Expand Down
8 changes: 2 additions & 6 deletions aws/resource_aws_vpn_connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,13 +222,9 @@ func testAccAWSVpnConnectionDisappears(connection *ec2.VpnConnection) resource.T
_, err := conn.DeleteVpnConnection(&ec2.DeleteVpnConnectionInput{
VpnConnectionId: connection.VpnConnectionId,
})

if err != nil {
if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "InvalidVpnConnectionID.NotFound" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The acceptance test should fail if this error is returned here.

return nil
}
if err != nil {
return err
}
return err
}

return resource.Retry(40*time.Minute, func() *resource.RetryError {
Expand Down
13 changes: 1 addition & 12 deletions aws/resource_aws_vpn_gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,18 +374,7 @@ func testAccAWSVpnGatewayDisappears(gateway *ec2.VpnGateway) resource.TestCheckF
VpcId: gateway.VpcAttachments[0].VpcId,
})
if err != nil {
ec2err, ok := err.(awserr.Error)
if ok {
if ec2err.Code() == "InvalidVpnGatewayID.NotFound" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The acceptance test should fail if either of these errors are returned here.

return nil
} else if ec2err.Code() == "InvalidVpnGatewayAttachment.NotFound" {
return nil
}
}

if err != nil {
return err
}
return err
}

opts := &ec2.DeleteVpnGatewayInput{
Expand Down