rds_instance_info/tests: add unit-tests #1132
Conversation
ansibullbot
added
community_review
module
goneri
force-pushed
the
rds_instance_info-tests-add-unit-tests_21027
branch
2 times, most recently
from
df946b9 to
bfa0ad1
Compare
goneri
force-pushed
the
rds_instance_info-tests-add-unit-tests_21027
branch
from
bfa0ad1 to
df84c28
Compare
| def instance_info(module, conn): | ||
| instance_name = module.params.get('db_instance_identifier') | ||
| filters = module.params.get('filters') | ||
| class RdsInstanceInfoFailure(Exception): |
There was a problem hiding this comment.
I do not really understand why we should use this custom exception class.
There was a problem hiding this comment.
When it comes to unit testing, it's much easier to test a function which simply raises an error a custom Exception than mocking up a "module" and handling the fail_json / fail_json_aws calls
There was a problem hiding this comment.
Ok, so we're going to uniform that for the other modules as well (I mean those we're willing to cover by unit).
There was a problem hiding this comment.
Compare the complexity of
https://github.com/ansible-collections/amazon.aws/blob/main/tests/unit/plugins/modules/ec2_instance/test_determine_iam_role.py#L50
to
pytest.raises(SomeException):
do_something_which_fails()
The down side is that you need to think a little about how to add any extra state to the exception so that it can be made available to the eventual
except SomeException:
module.fail_json_aws(....)
There was a problem hiding this comment.
I would leave the "what we're going to do" conversation to your broader team. I think there's a place for both. The exception based methodology is much more friendly to a unit-test heavy environment. As I understand it, it's also a little more 'pythonic'
The current "module.fail_json_aws" heavy methodology is much easier when you're working in an integration test heavy environment...
|
Pull request merge failed: Resource not accessible by integration, You may need to manually rebase your PR and retry. |
Extend the unit-test coverage of the `rds_instance_info` module. - break up `instance_info()` - use exception to handle the API failures
goneri
force-pushed
the
rds_instance_info-tests-add-unit-tests_21027
branch
from
e9a5551 to
f664507
Compare
|
regate |
|
regate |
|
regate |
|
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. |
Extend the unit-test coverage of the
rds_instance_infomodule.instance_info()modulewhen we can use an exception to handle the API failures