Skip to content
This repository has been archived by the owner on Apr 7, 2022. It is now read-only.

[1LP][RFR] Add test_infrastructure_hosts_tagging #10182

Merged
merged 4 commits into from
Jun 19, 2020

Conversation

prichard77
Copy link
Contributor

@prichard77 prichard77 commented Jun 9, 2020

Purpose or Intent

  • Adding tests for host tagging to replace manual test, test_host_tagging.
    version1.2.3.

PRT Run

{{ pytest: cfme/tests/infrastructure/test_host.py::test_infrastructure_hosts_tagging }}

@prichard77 prichard77 changed the title [WIP] Add test_infrastructure_hosts_tagging [WIPTEST] Add test_infrastructure_hosts_tagging Jun 9, 2020
@prichard77 prichard77 changed the title [WIPTEST] Add test_infrastructure_hosts_tagging [RFR] Add test_infrastructure_hosts_tagging Jun 9, 2020
@john-dupuy john-dupuy changed the title [RFR] Add test_infrastructure_hosts_tagging [1LP][RFR] Add test_infrastructure_hosts_tagging Jun 16, 2020
@john-dupuy john-dupuy added test-automation To be applied on PR's which are automating existing manual cases and removed new-test-or-feature labels Jun 16, 2020
Copy link
Contributor

@digitronik digitronik left a 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]
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

cfme/tests/infrastructure/test_host.py Show resolved Hide resolved
@digitronik digitronik self-assigned this Jun 18, 2020
@digitronik digitronik changed the title [1LP][RFR] Add test_infrastructure_hosts_tagging [1LP][WIP] Add test_infrastructure_hosts_tagging Jun 18, 2020
@prichard77 prichard77 changed the title [1LP][WIP] Add test_infrastructure_hosts_tagging [1LP][RFR] Add test_infrastructure_hosts_tagging Jun 18, 2020
@digitronik digitronik merged commit c65205d into ManageIQ:master Jun 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
test-automation To be applied on PR's which are automating existing manual cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants