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

[1LP][RFR] New Test: Testing associated vm storages using automate method #9806

Merged
merged 1 commit into from
Jan 16, 2020

Conversation

ganeshhubale
Copy link
Member

@ganeshhubale ganeshhubale commented Dec 30, 2019

Purpose or Intent

  • Testing associated vm storages using automate method

PRT Run

{{ pytest: cfme/tests/automate/test_common_methods.py -k 'test_list_of_diff_vm_storages_via_rails' --use-provider complete --use-template-cache -qsvvv }}

    - Testing associated vm storages using automate method
@ganeshhubale ganeshhubale added the test-automation To be applied on PR's which are automating existing manual cases label Dec 30, 2019
@ganeshhubale ganeshhubale changed the title [WIPTEST] New Test: Testing associated vm storages using automate method [RFR] New Test: Testing associated vm storages using automate method Jan 3, 2020
@dajoRH dajoRH removed the WIP-testing label Jan 3, 2020
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.

Awesome test case @ganeshhubale, looks good to me!

@john-dupuy john-dupuy changed the title [RFR] New Test: Testing associated vm storages using automate method [1LP][RFR] New Test: Testing associated vm storages using automate method Jan 6, 2020
4. Returns available storages
"""
list_storages = dedent(
f'vmware = $evm.vmdb("ems").find_by_name("{provider.name}")\n'
Copy link
Contributor

Choose a reason for hiding this comment

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

As we are using dedent, why not use full script?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree @digitronik

  • But I am not able to format this line using fstring or .format(vm = vmware.vms.select {|v| v.name == "{testing_vm.name}"}.first) because of extra {}.
  • Hence I modified this script in this way. Please let me know if you have a solution for formatting this script.

Copy link
Member

Choose a reason for hiding this comment

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

This can be done in either with a double { character.

In [8]: storages = (f'vmware = $evm.vmdb("ems").find_by_name("{provider_name}")\n' 
   ...: f'vm = vmware.vms.select {{|v| v.name == "{test_vm_name}"}}.first\n' 
   ...: )                                                                                                                     

In [9]: storages                                                                                                              
Out[9]: 'vmware = $evm.vmdb("ems").find_by_name("test_provider_name")\nvm = vmware.vms.select {|v| v.name == "test_virtual_machine"}.first\n'

The single braced expressions are evaluated, the double braced characters result in a single brace in the resulting string.

https://docs.python.org/3.7/reference/lexical_analysis.html#formatted-string-literals

First paragraph below the code block.

Copy link
Member

Choose a reason for hiding this comment

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

That said, the formatting change can be a followup, I won't block merging on this. The use of dedent and multiple lines makes it clear what is being evaluated and what is not.

Copy link
Member

@mshriver mshriver left a comment

Choose a reason for hiding this comment

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

Looks good to me - holding merge on PRT results. Last run is false-positive, the cases skipped because of provisioning timeouts.

@mshriver mshriver merged commit cba25fa into ManageIQ:master Jan 16, 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.

5 participants