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

Resource Acceptance Testing Missing CheckDestroy #8958

Closed
36 tasks done
bflad opened this issue Jun 12, 2019 · 2 comments · Fixed by #9159 or #9237
Closed
36 tasks done

Resource Acceptance Testing Missing CheckDestroy #8958

bflad opened this issue Jun 12, 2019 · 2 comments · Fixed by #9159 or #9237
Labels
technical-debt Addresses areas of the codebase that need refactoring or redesign. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Milestone
v2.20.0

Comments

@bflad
Copy link
Contributor

bflad commented Jun 12, 2019 •
edited
Loading

Description

TestCase CheckDestroy functions are used to verify that test infrastructure has been removed at the end of an acceptance test (e.g. the Delete function of the resource works correctly) by calling the API to attempt to read the deleted infrastructure. More information can be found at: https://www.terraform.io/docs/extend/testing/acceptance-tests/testcase.html#checkdestroy

We would like to enable the tfproviderlint AT001 check for verifying this check is included during pull request reviews, however there is some existing testing missing this functionality that needs to be addressed or explicitly ignored:

  • aws/provider_test.go:407:36: AT001: missing CheckDestroy
  • aws/provider_test.go:435:36: AT001: missing CheckDestroy
  • aws/resource_aws_api_gateway_vpc_link_test.go:94:36: AT001: missing CheckDestroy
  • aws/resource_aws_autoscaling_attachment_test.go:18:36: AT001: missing CheckDestroy
  • aws/resource_aws_autoscaling_attachment_test.go:60:36: AT001: missing CheckDestroy
  • aws/resource_aws_db_snapshot_test.go:17:36: AT001: missing CheckDestroy
  • aws/resource_aws_db_snapshot_test.go:34:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_snapshot_copy_test.go:17:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_snapshot_copy_test.go:34:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_snapshot_copy_test.go:63:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_snapshot_copy_test.go:80:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_volume_test.go:73:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_volume_test.go:104:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_volume_test.go:136:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_volume_test.go:168:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_volume_test.go:200:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_volume_test.go:235:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_volume_test.go:261:36: AT001: missing CheckDestroy
  • aws/resource_aws_ebs_volume_test.go:284:36: AT001: missing CheckDestroy
  • aws/resource_aws_iam_instance_profile_test.go:44:36: AT001: missing CheckDestroy
  • aws/resource_aws_iam_instance_profile_test.go:62:36: AT001: missing CheckDestroy
  • aws/resource_aws_iam_instance_profile_test.go:78:36: AT001: missing CheckDestroy
  • aws/resource_aws_inspector_resource_group_test.go:12:36: AT001: missing CheckDestroy
  • aws/resource_aws_kms_ciphertext_test.go:10:36: AT001: missing CheckDestroy
  • aws/resource_aws_kms_ciphertext_test.go:29:36: AT001: missing CheckDestroy
  • aws/resource_aws_kms_ciphertext_test.go:48:36: AT001: missing CheckDestroy
  • aws/resource_aws_lambda_event_source_mapping_test.go:271:36: AT001: missing CheckDestroy
  • aws/resource_aws_lb_target_group_test.go:900:36: AT001: missing CheckDestroy
  • aws/resource_aws_macie_member_account_association_test.go:37:36: AT001: missing CheckDestroy
  • aws/resource_aws_ses_domain_dkim_test.go:21:36: AT001: missing CheckDestroy
  • aws/resource_aws_snapshot_create_volume_permission_test.go:15:36: AT001: missing CheckDestroy
  • aws/resource_aws_storagegateway_cache_test.go:75:36: AT001: missing CheckDestroy
  • aws/resource_aws_storagegateway_cache_test.go:102:36: AT001: missing CheckDestroy
  • aws/resource_aws_storagegateway_upload_buffer_test.go:75:36: AT001: missing CheckDestroy
  • aws/resource_aws_storagegateway_working_storage_test.go:75:36: AT001: missing CheckDestroy
  • aws/resource_aws_vpc_dhcp_options_test.go:17:36: AT001: missing CheckDestroy

For a limited number of the above, bypassing the linter check either via //lintignore:AT001 or CheckDestroy: nil, is acceptable as there is no API check to perform.

Definition of Done

  • tfproviderlint is updated to v0.4.0 or later which will ignore data source files (upcoming release)
  • In GNUmakefile, the tfproviderlint command under the lint target has the -AT001 flag enabled and TravisCI testing passes
@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. labels Jun 12, 2019
This was referenced Jun 24, 2019
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@bflad bflad reopened this Jun 27, 2019
@bflad
Copy link
Contributor Author

bflad commented Jun 27, 2019

This linter check is now passing in master and ready to be enabled for pull requests!

bflad added a commit that referenced this issue Jul 4, 2019
@bflad
tests/provider: Enable tfproviderlint AT001 check
39cdad2 
Reference: #8958

Pull requests will now be checked for resource TestCase CheckDestroy implementation.
@bflad bflad mentioned this issue Jul 4, 2019
Merged
@bflad bflad added this to the v2.20.0 milestone Jul 11, 2019
@ghost
Copy link

ghost commented Nov 2, 2019

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 Nov 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
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
Milestone
v2.20.0
1 participant
@bflad