ec2_eni_info/tests: add unit-tests #1236
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ansibullbot
added
community_review
module
goneri
reviewed
Nov 2, 2022
plugins/modules/ec2_eni_info.py
Outdated
| def list_eni(connection, module): | ||
| def build_request_args(eni_id, filters): | ||
| request_args = { | ||
| 'NetworkInterfaceIds': [eni_id] if eni_id else '', |
There was a problem hiding this comment.
I'm surprise you default on an empty string if eni_id is False. I would expect an empty list.
goneri
requested changes
Nov 3, 2022
| { | ||
| "AvailabilityZone": "us-east-2b", | ||
| "Description": "", | ||
| "Groups": [ |
There was a problem hiding this comment.
A lot of these keys are not relevant to this test. I think it's better to just remove them, this way:
- you keep the test simple
- you avoid the situation where after a couple of years, your structure comes with a key that is deprecated
| ] | ||
| } | ||
|
|
||
| request_args = ec2_eni_info.build_request_args() |
There was a problem hiding this comment.
The build_request_arg() is superfluous since it already has a dedeicated test. Just use a static structure.
| ) | ||
| assert len(camel_network_interfaces) == 2 | ||
|
|
||
| assert camel_network_interfaces[0].get('id', None) == 'eni-1234567890' |
There was a problem hiding this comment.
you don't need the .get('id', None) here, just do ['id']. Also the None is not necessary because it's already the default value returned by .get() if a key is missing.
| assert camel_network_interfaces[1].get('name', None) == 'my-test-eni-name' | ||
|
|
||
|
|
||
| @patch(module_name + ".AnsibleAWSModule") |
There was a problem hiding this comment.
To be honest, I don't think this test is really useful because main() can be totally broken, and the test will be still successful.
goneri
approved these changes
Nov 6, 2022
|
regate |
|
regate |
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
markuman
pushed a commit
to markuman/amazon.aws
that referenced
this pull request
Nov 14, 2022
…sage (ansible-collections#1236) route53_info: Add snake_cased return key,values and a deprecation message Depends-On: ansible/ansible-zuul-jobs#1564 SUMMARY Add snake_case return values and a deprecation message for existing CamelCase return values. Route53_info currently returns CamelCase values, to have uniformity along all *_info modules in terms of return values, it should return snake_case values instead. Proposed change should make addition of snake_case return values and the deprecation message provides time for users to upgrade their playbooks to avoid breaking existing playbooks due to the proposed change. ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION This PR is relation to the initiative for having updated developer guidelines for *_info modules specifically related to guidelines for deprecating return values. Reviewed-by: Mark Chappell <None> Reviewed-by: Joseph Torcasso <None> Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@3df423a
markuman
pushed a commit
to markuman/amazon.aws
that referenced
this pull request
Nov 14, 2022
route53_info: Add RETURN block SUMMARY Currently route53_info is mising a return block. This is a follow up on ansible-collections#1236 ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info Reviewed-by: Joseph Torcasso <None> Reviewed-by: Mike Graves <[email protected]> Reviewed-by: Mandar Kulkarni <[email protected]> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@6e7f150
markuman
pushed a commit
to markuman/amazon.aws
that referenced
this pull request
Nov 14, 2022
…nsible-collections#1322) route53_info: Add snake_cased return key,values to remaining methods SUMMARY Following up on ansible-collections#1236 Found more places where route53_info module does not return a snake_case output. Added snake_case output to checker_ip_range_details , reusable_delegation_set_details, and get_health_check methods. ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info Reviewed-by: Joseph Torcasso <None> Reviewed-by: Mark Chappell <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@d7f3862
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
…sage (ansible-collections#1236) route53_info: Add snake_cased return key,values and a deprecation message Depends-On: ansible/ansible-zuul-jobs#1564 SUMMARY Add snake_case return values and a deprecation message for existing CamelCase return values. Route53_info currently returns CamelCase values, to have uniformity along all *_info modules in terms of return values, it should return snake_case values instead. Proposed change should make addition of snake_case return values and the deprecation message provides time for users to upgrade their playbooks to avoid breaking existing playbooks due to the proposed change. ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION This PR is relation to the initiative for having updated developer guidelines for *_info modules specifically related to guidelines for deprecating return values. Reviewed-by: Mark Chappell <None> Reviewed-by: Joseph Torcasso <None> Reviewed-by: Alina Buzachis <None>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
route53_info: Add RETURN block SUMMARY Currently route53_info is mising a return block. This is a follow up on ansible-collections#1236 ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info Reviewed-by: Joseph Torcasso <None> Reviewed-by: Mike Graves <[email protected]> Reviewed-by: Mandar Kulkarni <[email protected]>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
…nsible-collections#1322) route53_info: Add snake_cased return key,values to remaining methods SUMMARY Following up on ansible-collections#1236 Found more places where route53_info module does not return a snake_case output. Added snake_case output to checker_ip_range_details , reusable_delegation_set_details, and get_health_check methods. ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info Reviewed-by: Joseph Torcasso <None> Reviewed-by: Mark Chappell <None>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
…sage (ansible-collections#1236) route53_info: Add snake_cased return key,values and a deprecation message Depends-On: ansible/ansible-zuul-jobs#1564 SUMMARY Add snake_case return values and a deprecation message for existing CamelCase return values. Route53_info currently returns CamelCase values, to have uniformity along all *_info modules in terms of return values, it should return snake_case values instead. Proposed change should make addition of snake_case return values and the deprecation message provides time for users to upgrade their playbooks to avoid breaking existing playbooks due to the proposed change. ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION This PR is relation to the initiative for having updated developer guidelines for *_info modules specifically related to guidelines for deprecating return values. Reviewed-by: Mark Chappell <None> Reviewed-by: Joseph Torcasso <None> Reviewed-by: Alina Buzachis <None>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
route53_info: Add RETURN block SUMMARY Currently route53_info is mising a return block. This is a follow up on ansible-collections#1236 ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info Reviewed-by: Joseph Torcasso <None> Reviewed-by: Mike Graves <[email protected]> Reviewed-by: Mandar Kulkarni <[email protected]>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
…nsible-collections#1322) route53_info: Add snake_cased return key,values to remaining methods SUMMARY Following up on ansible-collections#1236 Found more places where route53_info module does not return a snake_case output. Added snake_case output to checker_ip_range_details , reusable_delegation_set_details, and get_health_check methods. ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info Reviewed-by: Joseph Torcasso <None> Reviewed-by: Mark Chappell <None>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Oct 24, 2023
…sage (ansible-collections#1236) route53_info: Add snake_cased return key,values and a deprecation message Depends-On: ansible/ansible-zuul-jobs#1564 SUMMARY Add snake_case return values and a deprecation message for existing CamelCase return values. Route53_info currently returns CamelCase values, to have uniformity along all *_info modules in terms of return values, it should return snake_case values instead. Proposed change should make addition of snake_case return values and the deprecation message provides time for users to upgrade their playbooks to avoid breaking existing playbooks due to the proposed change. ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION This PR is relation to the initiative for having updated developer guidelines for *_info modules specifically related to guidelines for deprecating return values. Reviewed-by: Mark Chappell <None> Reviewed-by: Joseph Torcasso <None> Reviewed-by: Alina Buzachis <None>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Oct 24, 2023
route53_info: Add RETURN block SUMMARY Currently route53_info is mising a return block. This is a follow up on ansible-collections#1236 ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info Reviewed-by: Joseph Torcasso <None> Reviewed-by: Mike Graves <[email protected]> Reviewed-by: Mandar Kulkarni <[email protected]>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Oct 24, 2023
…nsible-collections#1322) route53_info: Add snake_cased return key,values to remaining methods SUMMARY Following up on ansible-collections#1236 Found more places where route53_info module does not return a snake_case output. Added snake_case output to checker_ip_range_details , reusable_delegation_set_details, and get_health_check methods. ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info Reviewed-by: Joseph Torcasso <None> Reviewed-by: Mark Chappell <None>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
COMPONENT NAME
ec2_eni_info