-
Notifications
You must be signed in to change notification settings - Fork 346
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
Unit test cleanup #961
Unit test cleanup #961
Conversation
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 42s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
84d7e57
to
8c41b67
Compare
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 43s |
Build succeeded (gate pipeline). ✔️ ansible-galaxy-importer SUCCESS in 5m 04s |
Backport to stable-4: 💚 backport PR created✅ Backport PR branch: Backported as #962 🤖 @patchback |
Unit test cleanup SUMMARY Speaking to @mattclay, pytest based unit tests are generally considered preferred over unittest based unit tests. For the sake of having "good" examples in amazon.aws, migrates unittest based tests over to pytest Additionally: Moves tests about to reflect module_utils Cleans up the boto3/botocore test skipping uses "pytest.raises" rather than try/except blocks Cleans up unused variables Cleans up unused imports fixes s3_object unit test (was trying to import from the old location, redirects don't handle this) ISSUE TYPE Feature Pull Request COMPONENT NAME tests/units ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> (cherry picked from commit 898ade5)
[PR #961/898ade59 backport][stable-4] Unit test cleanup This is a backport of PR #961 as merged into main (898ade5). SUMMARY Speaking to mattclay, pytest based unit tests are generally considered preferred over unittest based unit tests. For the sake of having "good" examples in amazon.aws, migrates unittest based tests over to pytest Additionally: Moves tests about to reflect module_utils Cleans up the boto3/botocore test skipping uses "pytest.raises" rather than try/except blocks Cleans up unused variables Cleans up unused imports fixes s3_object unit test (was trying to import from the old location, redirects don't handle this) ISSUE TYPE Feature Pull Request COMPONENT NAME tests/units ADDITIONAL INFORMATION Reviewed-by: Mark Chappell <None>
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections/amazon.aws#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None>
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@8b4afa9
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@8b4afa9
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@8b4afa9
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@8b4afa9
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@8b4afa9
…ible-collections#961) IAM Role Removal Does Not Require Removal of Permission Boundary SUMMARY Removes unnecessary removal of permission boundary from a role when deleting a role. Unlike inline policies, permission boundaries do not need to be removed from an IAM role before deleting the IAM role. This behavior causes issues when a permission boundary is inherited that prevents removal of the permission boundary. Fixes ansible-collections#959 ISSUE TYPE Bugfix Pull Request COMPONENT NAME iam_role Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@e670b34
…ible-collections#961) IAM Role Removal Does Not Require Removal of Permission Boundary SUMMARY Removes unnecessary removal of permission boundary from a role when deleting a role. Unlike inline policies, permission boundaries do not need to be removed from an IAM role before deleting the IAM role. This behavior causes issues when a permission boundary is inherited that prevents removal of the permission boundary. Fixes ansible-collections#959 ISSUE TYPE Bugfix Pull Request COMPONENT NAME iam_role Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@e670b34
…ible-collections#961) IAM Role Removal Does Not Require Removal of Permission Boundary SUMMARY Removes unnecessary removal of permission boundary from a role when deleting a role. Unlike inline policies, permission boundaries do not need to be removed from an IAM role before deleting the IAM role. This behavior causes issues when a permission boundary is inherited that prevents removal of the permission boundary. Fixes ansible-collections#959 ISSUE TYPE Bugfix Pull Request COMPONENT NAME iam_role Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None>
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None>
…ible-collections#961) IAM Role Removal Does Not Require Removal of Permission Boundary SUMMARY Removes unnecessary removal of permission boundary from a role when deleting a role. Unlike inline policies, permission boundaries do not need to be removed from an IAM role before deleting the IAM role. This behavior causes issues when a permission boundary is inherited that prevents removal of the permission boundary. Fixes ansible-collections#959 ISSUE TYPE Bugfix Pull Request COMPONENT NAME iam_role Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None>
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None>
…ible-collections#961) IAM Role Removal Does Not Require Removal of Permission Boundary SUMMARY Removes unnecessary removal of permission boundary from a role when deleting a role. Unlike inline policies, permission boundaries do not need to be removed from an IAM role before deleting the IAM role. This behavior causes issues when a permission boundary is inherited that prevents removal of the permission boundary. Fixes ansible-collections#959 ISSUE TYPE Bugfix Pull Request COMPONENT NAME iam_role Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@e670b34
…ible-collections#961) IAM Role Removal Does Not Require Removal of Permission Boundary SUMMARY Removes unnecessary removal of permission boundary from a role when deleting a role. Unlike inline policies, permission boundaries do not need to be removed from an IAM role before deleting the IAM role. This behavior causes issues when a permission boundary is inherited that prevents removal of the permission boundary. Fixes ansible-collections#959 ISSUE TYPE Bugfix Pull Request COMPONENT NAME iam_role Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@e670b34
…ible-collections#961) IAM Role Removal Does Not Require Removal of Permission Boundary SUMMARY Removes unnecessary removal of permission boundary from a role when deleting a role. Unlike inline policies, permission boundaries do not need to be removed from an IAM role before deleting the IAM role. This behavior causes issues when a permission boundary is inherited that prevents removal of the permission boundary. Fixes ansible-collections#959 ISSUE TYPE Bugfix Pull Request COMPONENT NAME iam_role Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@e670b34
…ible-collections#961) IAM Role Removal Does Not Require Removal of Permission Boundary SUMMARY Removes unnecessary removal of permission boundary from a role when deleting a role. Unlike inline policies, permission boundaries do not need to be removed from an IAM role before deleting the IAM role. This behavior causes issues when a permission boundary is inherited that prevents removal of the permission boundary. Fixes ansible-collections#959 ISSUE TYPE Bugfix Pull Request COMPONENT NAME iam_role Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@e670b34
…ible-collections#961) IAM Role Removal Does Not Require Removal of Permission Boundary SUMMARY Removes unnecessary removal of permission boundary from a role when deleting a role. Unlike inline policies, permission boundaries do not need to be removed from an IAM role before deleting the IAM role. This behavior causes issues when a permission boundary is inherited that prevents removal of the permission boundary. Fixes ansible-collections#959 ISSUE TYPE Bugfix Pull Request COMPONENT NAME iam_role Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Chappell <None>
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@8b4afa9
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@8b4afa9
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@8b4afa9
Minor linting fixups - 2022-08-12 SUMMARY Various linting and unit test fixups unused variables overly broad Exception catching (highlighted some broken tests) removes direct use of unittest in favour of pytest (see also ansible-collections#961) cleans up skipping of tests when botocore/boto3 aren't installed passes error message from VPNConnectionException into its super to make testing easier, should never be directly exposed to the user Removes tests for 3 modules which now have integration tests, they're either recording based (fragile)or test things which are separately tested in the integration tests. lambda s3_bucket_notifications route53_zone ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_vpc_vpn.py tests/unit ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@8b4afa9
SUMMARY
Speaking to @mattclay,
pytest
based unit tests are generally considered preferred overunittest
based unit tests. For the sake of having "good" examples in amazon.aws, migrates unittest based tests over to pytestAdditionally:
ISSUE TYPE
COMPONENT NAME
tests/units
ADDITIONAL INFORMATION