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

[1LP][RFR] Fix tests #9978

Merged
merged 2 commits into from
Mar 25, 2020
Merged

[1LP][RFR] Fix tests #9978

merged 2 commits into from
Mar 25, 2020

Conversation

valaparthvi
Copy link
Contributor

@valaparthvi valaparthvi commented Mar 11, 2020 •
edited by ganeshhubale
Loading

Purpose or Intent

  • Fixing
    1. Fix test_canned_corresponds.py tests by adding 2 new fixtures setup_provider_temp_appliance and setup_provider_temp_appliance_modscope to make sure provider is added on the right appliance while using temp appliances in the test.
    2. Fix test_default_filters_reset and test_reset_report_menus failing because of updates with wt.pf FlashMessages
    3. Block test_vm_retirement_rest.py with BZ(1805119)
    4. Fix test_customization_paginator by fixing the AddDialogView::is_displayed. Also add same fix to EditDialogView::is_displayed.
    5. Fix test_vm_volume_free_space_less_than_20_percent by restoring the deleted template used by the test.
    6. Fix TestProvidersRESTAPI by waiting for the data before testing further.

PRT Run

{{ pytest: cfme/tests/intelligence/reports/test_canned_corresponds.py cfme/tests/configure/test_default_filters.py::test_default_filters_reset cfme/tests/infrastructure/test_providers.py::test_infra_discovery_screen cfme/tests/infrastructure/test_vm_retirement_rest.py cfme/tests/intelligence/test_download_report.py cfme/tests/webui/test_general_ui.py::test_provider_documentation cfme/tests/intelligence/reports/test_reports.py::test_vm_volume_free_space_less_than_20_percent cfme/tests/cloud/test_providers.py::test_display_network_topology cfme/tests/intelligence/reports/test_menus.py::test_reset_report_menus cfme/tests/cloud/test_providers.py::TestProvidersRESTAPI cfme/tests/automate/test_customization_paginator.py --long-running }}

@valaparthvi valaparthvi force-pushed the test_fixes branch 2 times, most recently from 674717c to 8478d27 Compare March 12, 2020 11:22
@valaparthvi valaparthvi changed the title [WIPTEST] Fix tests [RFR] Fix tests Mar 12, 2020
ganeshhubale
ganeshhubale approved these changes Mar 13, 2020
View reviewed changes
Copy link
Member

@ganeshhubale ganeshhubale 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 this PR @valaparthvi
LGTM :)

john-dupuy
john-dupuy approved these changes Mar 13, 2020
View reviewed changes
Copy link
Contributor

@john-dupuy john-dupuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice solution to the provider on a temp appliance problem, I like it. 👍

@john-dupuy john-dupuy changed the title [RFR] Fix tests [1LP][RFR] Fix tests Mar 13, 2020
@dajoRH dajoRH changed the title [1LP][RFR] Fix tests [1LP][WIP] Fix tests Mar 13, 2020
@valaparthvi valaparthvi changed the title [1LP][WIP] Fix tests [1LP][WIPTEST] Fix tests Mar 13, 2020
@valaparthvi valaparthvi changed the title [1LP][WIPTEST] Fix tests [1LP][RFR] Fix tests Mar 19, 2020
digitronik
digitronik approved these changes Mar 24, 2020
View reviewed changes
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.

Nice PR 👍 Thanks for fixes.

cfme/tests/intelligence/reports/test_reports.py
@@ -181,7 +181,7 @@ def setup_vm(configure_fleecing, appliance, provider):
vm = appliance.collections.infra_vms.instantiate(
name=random_vm_name(context="report", max_length=20),
provider=provider,
template_name="env-rhel7-20-percent-full-disk-tpl",
template_name="env-rhel7-20-percent-full-disk-pvala-tpl",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should come from data yaml than hardcodding. We can include this under template session for provider. I'm marking this optional as just modifying but please raise separate PR for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This template won't be used by any other test case unless you want to provision a VM which has less than 20% disk space, which is why I directly used the template name even though it's weird to look at it. But if you insist, we can move it to data yaml.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup better to take it from yaml... It will help in template maintenance as well.

cfme/fixtures/provider.py
@@ -104,6 +104,8 @@ def enable_provider_regions(provider):
def _setup_provider_verbose(request, provider, appliance=None):
if appliance is None:
appliance = store.current_appliance
else:
provider.appliance = appliance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please add comment for this?

@digitronik digitronik self-assigned this Mar 24, 2020
@dajoRH
Copy link
Contributor

dajoRH commented Mar 24, 2020

I detected some fixture changes in commit f5d065a

The global fixture setup_provider_temp_appliance was changed, but I didn't find where it's used.
The global fixture setup_provider_temp_appliance_modscope was changed, but I didn't find where it's used.
The local fixture setup_vm is used in the following files:

  • cfme/tests/intelligence/reports/test_reports.py

Please, consider creating a PRT run to make sure your fixture changes do not break existing usage 😃

@digitronik digitronik merged commit cbf9179 into ManageIQ:master Mar 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@digitronik digitronik digitronik approved these changes

@john-dupuy john-dupuy john-dupuy approved these changes

@ganeshhubale ganeshhubale ganeshhubale approved these changes

Assignees

@digitronik digitronik

Labels
enhancement fix-locator-or-text fix-test lint-ok
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@valaparthvi @dajoRH @digitronik @ganeshhubale @john-dupuy