-
Notifications
You must be signed in to change notification settings - Fork 164
[1LP][RFR] Add test_infrastructure_hosts_tagging #10182
Conversation
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.
Thanks for automating this test @prichard77
some comments have a glance.
caseimportance: high | ||
initialEstimate: 1/6h | ||
""" | ||
host = appliance.collections.hosts.all()[0] |
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.
It may raise IndexError
if not single host available for testing.
Its rare case but we can handle this and try to skip test if possible.
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.
I believe we have been assuming that there will be at lest one host. There always is with the providers we use for host testing. I created a fixture previously that checked for min hosts when we needed more than one. But it hasn't been used to check for just one. There are many tests in test_host.py that do this. If we really want to check I can add one, but it's not been the convention.
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.
Yup, I agreed that's why added rare case scenario like removal of host.
Purpose or Intent
version1.2.3.
PRT Run
{{ pytest: cfme/tests/infrastructure/test_host.py::test_infrastructure_hosts_tagging }}