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

Ignore/Fix golangci-lint v1.27.0 and later reports for unused with t.Skip() test functions #14240

Closed
bflad opened this issue Jul 18, 2020 · 2 comments · Fixed by #14241
Closed
Assignees
Labels
linter Pertains to changes to or issues with the various linters. provider Pertains to the provider itself, rather than any interaction with AWS. technical-debt Addresses areas of the codebase that need refactoring or redesign. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.

Comments

@bflad
Copy link
Contributor

bflad commented Jul 18, 2020

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

New unused reports in golangci-lint v1.27.0 and later are currently preventing its upgrade. We would generally prefer to keep these skipped tests and test configurations in the codebase for future remediation, so the preference in this case is figuring out how to disable the t.Skip() behavior with unused or worst case commenting the affected code. Current reports:

$ golangci-lint run --enable unused ./aws/...
aws/resource_aws_datasync_task_test.go:958:6: func `testAccAWSDataSyncTaskConfigTags2` is unused (unused)
aws/resource_aws_msk_cluster_test.go:757:6: func `testAccMskClusterConfigClientAuthenticationTlsCertificateAuthorityArns` is unused (unused)
aws/resource_aws_msk_cluster_test.go:828:6: func `testAccMskClusterConfigConfigurationInfoRevision2` is unused (unused)
aws/resource_aws_ec2_capacity_reservation_test.go:704:6: func `testAccEc2CapacityReservationConfig_tenancy` is unused (unused)
aws/resource_aws_db_instance_test.go:6502:6: func `testAccAWSDBInstanceConfig_SnapshotIdentifier_Tags_Unset` is unused (unused)
aws/resource_aws_organizations_account_test.go:247:6: func `testAccAwsOrganizationsAccountConfigParentId2` is unused (unused)
aws/resource_aws_organizations_account_test.go:225:6: func `testAccAwsOrganizationsAccountConfigParentId1` is unused (unused)
aws/resource_aws_cloudformation_stack_set_test.go:742:6: func `testAccAWSCloudFormationStackSetTemplateBodyParametersDefault1` is unused (unused)
aws/resource_aws_cloudformation_stack_set_test.go:958:6: func `testAccAWSCloudFormationStackSetConfigParametersDefault0` is unused (unused)
aws/resource_aws_media_convert_queue_test.go:341:6: func `testAccMediaConvertQueueConfig_ReservedQueue` is unused (unused)
aws/resource_aws_acmpca_certificate_authority_test.go:518:6: func `testAccAwsAcmpcaCertificateAuthorityConfig_Enabled` is unused (unused)
aws/resource_aws_ec2_fleet_test.go:1528:6: func `testAccAWSEc2FleetConfig_LaunchTemplateConfig_Override_MaxPrice` is unused (unused)
aws/resource_aws_organizations_account_test.go:284:6: func `testAccAwsOrganizationsAccountConfigTags2` is unused (unused)
aws/resource_aws_cloudformation_stack_set_test.go:767:6: func `testAccAWSCloudFormationStackSetTemplateBodyParametersNoEcho1` is unused (unused)
aws/resource_aws_cloudformation_stack_set_test.go:998:6: func `testAccAWSCloudFormationStackSetConfigParametersNoEcho1` is unused (unused)
aws/resource_aws_db_instance_test.go:5413:6: func `testAccAWSDBInstanceConfig_ReplicateSourceDb_DeletionProtection` is unused (unused)
aws/resource_aws_msk_cluster_test.go:797:6: func `testAccMskClusterConfigConfigurationInfoRevision1` is unused (unused)
aws/resource_aws_organizations_account_test.go:188:6: func `testAccCheckAwsOrganizationsAccountExists` is unused (unused)
aws/resource_aws_storagegateway_cached_iscsi_volume_test.go:416:6: func `testAccAWSStorageGatewayCachedIscsiVolumeConfig_SourceVolumeArn` is unused (unused)
aws/resource_aws_cloudformation_stack_set_test.go:976:6: func `testAccAWSCloudFormationStackSetConfigParametersDefault1` is unused (unused)
aws/resource_aws_organizations_account_test.go:157:6: func `testAccCheckAwsOrganizationsAccountDestroy` is unused (unused)
aws/resource_aws_spot_fleet_request_test.go:2265:6: func `testAccAWSSpotFleetRequestLaunchSpecificationWithInstanceStoreAmi` is unused (unused)
aws/resource_aws_datasync_task_test.go:944:6: func `testAccAWSDataSyncTaskConfigTags1` is unused (unused)
aws/resource_aws_organizations_account_test.go:216:6: func `testAccAwsOrganizationsAccountConfig` is unused (unused)
aws/resource_aws_ses_template_test.go:193:6: func `testAccCheckAwsSesTemplateResourceConfigBasic2` is unused (unused)
aws/resource_aws_organizations_account_test.go:269:6: func `testAccAwsOrganizationsAccountConfigTags1` is unused (unused)
aws/resource_aws_ses_template_test.go:204:6: func `testAccCheckAwsSesTemplateResourceConfigBasic3` is unused (unused)
aws/resource_aws_codebuild_project_test.go:3929:6: func `testAccAWSCodebuildProjectConfig_SecondaryArtifacts_Name` is unused (unused)

References

@bflad bflad added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. technical-debt Addresses areas of the codebase that need refactoring or redesign. provider Pertains to the provider itself, rather than any interaction with AWS. labels Jul 18, 2020
bflad added a commit that referenced this issue Jul 18, 2020
Reference: #14240

Output from acceptance testing:

```console
$ golangci-lint run --disable-all --enable unused ./aws/...
$ TF_ACC=1 go test ./aws -v -count 1 -timeout 120m -parallel 20 -run='(TestAccAWSDataSyncTask_Tags|TestAccAWSEc2CapacityReservation_tenancy|TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns|TestAccAWSMskCluster_ConfigurationInfo_Revision|TestAccAWSDBInstance_SnapshotIdentifier_Tags_Unset|TestAccAwsAcmpcaCertificateAuthority_Enabled|TestAccAWSCloudFormationStackSet_Parameters_NoEcho|TestAccAWSCodeBuildProject_SecondaryArtifacts_Name|TestAccAWSSpotFleetRequest_WithInstanceStoreAmi|TestAccAWSStorageGatewayCachedIscsiVolume_SourceVolumeArn|TestAccAWSDBInstance_ReplicateSourceDb_DeletionProtection|TestAccAWSSesTemplate_Update|TestAccAWSCloudFormationStackSet_Parameters_Default)$'
=== RUN   TestAccAwsAcmpcaCertificateAuthority_Enabled
    TestAccAwsAcmpcaCertificateAuthority_Enabled: provider_test.go:30: We need to fully sign the certificate authority CSR from another CA in order to test this functionality, which requires another resource
--- SKIP: TestAccAwsAcmpcaCertificateAuthority_Enabled (0.00s)
=== RUN   TestAccAWSCloudFormationStackSet_Parameters_Default
    TestAccAWSCloudFormationStackSet_Parameters_Default: provider_test.go:30: this resource does not currently ignore unconfigured CloudFormation template parameters with the Default property
--- SKIP: TestAccAWSCloudFormationStackSet_Parameters_Default (0.00s)
=== RUN   TestAccAWSCloudFormationStackSet_Parameters_NoEcho
    TestAccAWSCloudFormationStackSet_Parameters_NoEcho: provider_test.go:30: this resource does not currently ignore CloudFormation template parameters with the NoEcho property
--- SKIP: TestAccAWSCloudFormationStackSet_Parameters_NoEcho (0.00s)
=== RUN   TestAccAWSCodeBuildProject_SecondaryArtifacts_Name
    TestAccAWSCodeBuildProject_SecondaryArtifacts_Name: provider_test.go:30: Currently no solution to allow updates on name attribute
--- SKIP: TestAccAWSCodeBuildProject_SecondaryArtifacts_Name (0.00s)
=== RUN   TestAccAWSDataSyncTask_Tags
    TestAccAWSDataSyncTask_Tags: provider_test.go:30: Tagging on creation is inconsistent
--- SKIP: TestAccAWSDataSyncTask_Tags (0.00s)
=== RUN   TestAccAWSDBInstance_ReplicateSourceDb_DeletionProtection
    TestAccAWSDBInstance_ReplicateSourceDb_DeletionProtection: provider_test.go:30: CreateDBInstanceReadReplica API currently ignores DeletionProtection=true with SourceDBInstanceIdentifier set
--- SKIP: TestAccAWSDBInstance_ReplicateSourceDb_DeletionProtection (0.00s)
=== RUN   TestAccAWSDBInstance_SnapshotIdentifier_Tags_Unset
    TestAccAWSDBInstance_SnapshotIdentifier_Tags_Unset: provider_test.go:30: To be fixed: #5959
--- SKIP: TestAccAWSDBInstance_SnapshotIdentifier_Tags_Unset (0.00s)
=== RUN   TestAccAWSEc2CapacityReservation_tenancy
    TestAccAWSEc2CapacityReservation_tenancy: provider_test.go:30: EC2 Capacity Reservations do not currently support dedicated tenancy.
--- SKIP: TestAccAWSEc2CapacityReservation_tenancy (0.00s)
=== RUN   TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns
    TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns: provider_test.go:30: Requires the aws_acmpca_certificate_authority resource to support importing the root CA certificate
--- SKIP: TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns (0.00s)
=== RUN   TestAccAWSMskCluster_ConfigurationInfo_Revision
    TestAccAWSMskCluster_ConfigurationInfo_Revision: provider_test.go:30: aws_msk_cluster is correctly calling UpdateClusterConfiguration however API is always returning 429 and 500 errors
--- SKIP: TestAccAWSMskCluster_ConfigurationInfo_Revision (0.00s)
=== RUN   TestAccAWSSesTemplate_Update
    TestAccAWSSesTemplate_Update: provider_test.go:30: Skip due to SES.UpdateTemplate eventual consistency issues
--- SKIP: TestAccAWSSesTemplate_Update (0.00s)
=== RUN   TestAccAWSSpotFleetRequest_WithInstanceStoreAmi
    TestAccAWSSpotFleetRequest_WithInstanceStoreAmi: provider_test.go:30: Test fails due to test harness constraints
--- SKIP: TestAccAWSSpotFleetRequest_WithInstanceStoreAmi (0.00s)
=== RUN   TestAccAWSStorageGatewayCachedIscsiVolume_SourceVolumeArn
    TestAccAWSStorageGatewayCachedIscsiVolume_SourceVolumeArn: provider_test.go:30: This test can cause Storage Gateway 2.0.10.0 to enter an irrecoverable state during volume deletion.
--- SKIP: TestAccAWSStorageGatewayCachedIscsiVolume_SourceVolumeArn (0.00s)
PASS
ok    github.com/terraform-providers/terraform-provider-aws/aws 0.931s
```
@bflad bflad self-assigned this Jul 18, 2020
@bflad
Copy link
Contributor Author

bflad commented Jul 20, 2020

Took the wrapped function workaround approach suggested upstream in #14241.

bflad added a commit that referenced this issue Jul 29, 2020
…age (#14241)

Reference: #14240

Output from acceptance testing:

```console
$ golangci-lint run --disable-all --enable unused ./aws/...
$ TF_ACC=1 go test ./aws -v -count 1 -timeout 120m -parallel 20 -run='(TestAccAWSDataSyncTask_Tags|TestAccAWSEc2CapacityReservation_tenancy|TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns|TestAccAWSMskCluster_ConfigurationInfo_Revision|TestAccAWSDBInstance_SnapshotIdentifier_Tags_Unset|TestAccAwsAcmpcaCertificateAuthority_Enabled|TestAccAWSCloudFormationStackSet_Parameters_NoEcho|TestAccAWSCodeBuildProject_SecondaryArtifacts_Name|TestAccAWSSpotFleetRequest_WithInstanceStoreAmi|TestAccAWSStorageGatewayCachedIscsiVolume_SourceVolumeArn|TestAccAWSDBInstance_ReplicateSourceDb_DeletionProtection|TestAccAWSSesTemplate_Update|TestAccAWSCloudFormationStackSet_Parameters_Default)$'
=== RUN   TestAccAwsAcmpcaCertificateAuthority_Enabled
    TestAccAwsAcmpcaCertificateAuthority_Enabled: provider_test.go:30: We need to fully sign the certificate authority CSR from another CA in order to test this functionality, which requires another resource
--- SKIP: TestAccAwsAcmpcaCertificateAuthority_Enabled (0.00s)
=== RUN   TestAccAWSCloudFormationStackSet_Parameters_Default
    TestAccAWSCloudFormationStackSet_Parameters_Default: provider_test.go:30: this resource does not currently ignore unconfigured CloudFormation template parameters with the Default property
--- SKIP: TestAccAWSCloudFormationStackSet_Parameters_Default (0.00s)
=== RUN   TestAccAWSCloudFormationStackSet_Parameters_NoEcho
    TestAccAWSCloudFormationStackSet_Parameters_NoEcho: provider_test.go:30: this resource does not currently ignore CloudFormation template parameters with the NoEcho property
--- SKIP: TestAccAWSCloudFormationStackSet_Parameters_NoEcho (0.00s)
=== RUN   TestAccAWSCodeBuildProject_SecondaryArtifacts_Name
    TestAccAWSCodeBuildProject_SecondaryArtifacts_Name: provider_test.go:30: Currently no solution to allow updates on name attribute
--- SKIP: TestAccAWSCodeBuildProject_SecondaryArtifacts_Name (0.00s)
=== RUN   TestAccAWSDataSyncTask_Tags
    TestAccAWSDataSyncTask_Tags: provider_test.go:30: Tagging on creation is inconsistent
--- SKIP: TestAccAWSDataSyncTask_Tags (0.00s)
=== RUN   TestAccAWSDBInstance_ReplicateSourceDb_DeletionProtection
    TestAccAWSDBInstance_ReplicateSourceDb_DeletionProtection: provider_test.go:30: CreateDBInstanceReadReplica API currently ignores DeletionProtection=true with SourceDBInstanceIdentifier set
--- SKIP: TestAccAWSDBInstance_ReplicateSourceDb_DeletionProtection (0.00s)
=== RUN   TestAccAWSDBInstance_SnapshotIdentifier_Tags_Unset
    TestAccAWSDBInstance_SnapshotIdentifier_Tags_Unset: provider_test.go:30: To be fixed: #5959
--- SKIP: TestAccAWSDBInstance_SnapshotIdentifier_Tags_Unset (0.00s)
=== RUN   TestAccAWSEc2CapacityReservation_tenancy
    TestAccAWSEc2CapacityReservation_tenancy: provider_test.go:30: EC2 Capacity Reservations do not currently support dedicated tenancy.
--- SKIP: TestAccAWSEc2CapacityReservation_tenancy (0.00s)
=== RUN   TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns
    TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns: provider_test.go:30: Requires the aws_acmpca_certificate_authority resource to support importing the root CA certificate
--- SKIP: TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns (0.00s)
=== RUN   TestAccAWSMskCluster_ConfigurationInfo_Revision
    TestAccAWSMskCluster_ConfigurationInfo_Revision: provider_test.go:30: aws_msk_cluster is correctly calling UpdateClusterConfiguration however API is always returning 429 and 500 errors
--- SKIP: TestAccAWSMskCluster_ConfigurationInfo_Revision (0.00s)
=== RUN   TestAccAWSSesTemplate_Update
    TestAccAWSSesTemplate_Update: provider_test.go:30: Skip due to SES.UpdateTemplate eventual consistency issues
--- SKIP: TestAccAWSSesTemplate_Update (0.00s)
=== RUN   TestAccAWSSpotFleetRequest_WithInstanceStoreAmi
    TestAccAWSSpotFleetRequest_WithInstanceStoreAmi: provider_test.go:30: Test fails due to test harness constraints
--- SKIP: TestAccAWSSpotFleetRequest_WithInstanceStoreAmi (0.00s)
=== RUN   TestAccAWSStorageGatewayCachedIscsiVolume_SourceVolumeArn
    TestAccAWSStorageGatewayCachedIscsiVolume_SourceVolumeArn: provider_test.go:30: This test can cause Storage Gateway 2.0.10.0 to enter an irrecoverable state during volume deletion.
--- SKIP: TestAccAWSStorageGatewayCachedIscsiVolume_SourceVolumeArn (0.00s)
PASS
ok    github.com/terraform-providers/terraform-provider-aws/aws 0.931s
```
@ghost
Copy link

ghost commented Aug 28, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Aug 28, 2020
@breathingdust breathingdust added the linter Pertains to changes to or issues with the various linters. label Oct 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
linter Pertains to changes to or issues with the various linters. provider Pertains to the provider itself, rather than any interaction with AWS. technical-debt Addresses areas of the codebase that need refactoring or redesign. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
2 participants