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

[RFR] Add locals to test_infrastructure_hosts_navigation_after_download and ... #10117

Merged
merged 1 commit into from
Jul 1, 2020

Conversation

prichard77
Copy link
Contributor

@prichard77 prichard77 commented May 14, 2020

Purpose or Intent

  • Updating tests to use locals and possibly putting functionality into functions and other updates.

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

@prichard77
Copy link
Contributor Author

prichard77 commented Jun 24, 2020

The commit here now only contains adding the usage of locals. I have a few additional updates I'd like to discuss. These deal with putting the navigation and button clicking for download (hosts report from the host view) into a method of HostCollection. I have done this and tested and it works fine. The one caveat being how we want to deal with the "Print or Export" functionality.

        view = navigate_to(self.hosts, "All")
        view.toolbar.download.item_select(report_format.value)
        if report_format == DownloadOptions.PDF:
            handle_extra_tabs(view)
        return view

This allows for the nav to be included in the download:
hosts_view = locals()[hosts_collection].collections.hosts.download_report(report_format)

If I simple move the code out of the test and into the method as it is (with support for "Print or Export") the test would function as it does now and would be valid for verifying the customer BZ has been fixed. The only thing is that we'd have a download method that a) includes an action that while on the same button isn't actually a download. b) doesn't actually perform a print or export when selected c) adding support for print/export is not trivial and has not been denoted as a priority.

One solution would be to remove PDF from the DownloadOptions Enum. Then we would only support CSV and Text. These are the only True downloads, and are supported with existing code. This would (without me adding it back in explicitly) remove the case for the PDF export from the 2 tests that use it, however the BZ for witch it was written is reproduced both in Text and CSV which we would still have. And this is only used within 2 tests that are specifically testing the customer BZ and not the button. This makes more sense to me, but want some input.

Or I could just use as is.

@prichard77 prichard77 changed the title [WIP]Add locals to test_infrastructure_hosts_navigation_after_download and… [WIPTEST] Add locals to test_infrastructure_hosts_navigation_after_download and… Jun 24, 2020
@prichard77 prichard77 changed the title [WIPTEST] Add locals to test_infrastructure_hosts_navigation_after_download and… [WIPTEST] Add locals to test_infrastructure_hosts_navigation_after_download Jun 24, 2020
@prichard77 prichard77 changed the title [WIPTEST] Add locals to test_infrastructure_hosts_navigation_after_download [WIPTEST] Add locals to test_infrastructure_hosts_navigation_after_download and ... Jun 24, 2020
@prichard77 prichard77 changed the title [WIPTEST] Add locals to test_infrastructure_hosts_navigation_after_download and ... [RFR] Add locals to test_infrastructure_hosts_navigation_after_download and ... Jun 24, 2020
@john-dupuy john-dupuy removed the WIP label Jun 26, 2020
@mshriver mshriver merged commit ba57475 into ManageIQ:master Jul 1, 2020
@mshriver mshriver added the test-automation To be applied on PR's which are automating existing manual cases label Jul 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lint-ok 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.

None yet

4 participants