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

[1LP][RFR] Vm name enhancement - to append assignee to VM name #9186

Merged
merged 6 commits into from
Jan 14, 2020

Conversation

kedark3
Copy link
Contributor

@kedark3 kedark3 commented Aug 11, 2019

Purpose or Intent

  • Enhancement for feature vm.create_on_provider() method to take request fixture, fetch assignee from polarion docblock and postfix assignee to VM name

Tested all affected files on jenkins 511z/job/kk-console-chrome/69/ where I replaced all full_template_vm usage with new create_vm_fixture and it passed correctly. Please review.

PRT Run

  • {{ pytest: cfme/utils/tests/test_vm_name_assignee.py cfme/utils/tests/test_vm_name_assignee_func_scope.py --use-provider vsphere67-nested -v }}

@kedark3 kedark3 force-pushed the vm_name_enhancement branch from 639689f to 245f364 Compare August 11, 2019 23:18
@kedark3
Copy link
Contributor Author

kedark3 commented Aug 11, 2019

@jawatts @mshriver this is my initial thought around how to get VM names postfixed with asignee. Tested it locally and it worked fine. Let me know how can we improve it. Thanks.
HTML5 console tests are failing but you can see cfme.log to see vm name, look for test-html5-

@kedark3 kedark3 changed the title [WIPTEST]Vm name enhancement - to postfix assignee to VM name [RFR]Vm name enhancement - to postfix assignee to VM name Aug 14, 2019
cfme/common/vm.py Outdated Show resolved Hide resolved
@john-dupuy john-dupuy changed the title [RFR]Vm name enhancement - to postfix assignee to VM name [WIPTEST] Vm name enhancement - to postfix assignee to VM name Aug 15, 2019
@kedark3 kedark3 force-pushed the vm_name_enhancement branch from 245f364 to 1fb20d3 Compare August 15, 2019 18:05
@kedark3 kedark3 changed the title [WIPTEST] Vm name enhancement - to postfix assignee to VM name [RFR] Vm name enhancement - to postfix assignee to VM name Aug 15, 2019
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.

Some more comments and questions, thanks for making use of polarion_docstrings!

cfme/common/vm.py Outdated Show resolved Hide resolved
cfme/common/vm.py Outdated Show resolved Hide resolved
cfme/common/vm.py Outdated Show resolved Hide resolved
cfme/tests/cloud_infra_common/test_html5_vm_console.py Outdated Show resolved Hide resolved
try:
from polarion_docstrings.parser import DocstringParser
assignee = DocstringParser(request.function.__doc__).parse()["assignee"].value
except TypeError:
Copy link
Contributor

Choose a reason for hiding this comment

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

we can remove this is we use the .get()

@kedark3
Copy link
Contributor Author

kedark3 commented Aug 15, 2019

@john-dupuy I had chat with @mshriver he had some thoughts around moving it to Fixtures layer, he will comment on the PR shortly and I will re-work it then :) Thanks for review.

@john-dupuy john-dupuy changed the title [RFR] Vm name enhancement - to postfix assignee to VM name [WIPTEST] Vm name enhancement - to postfix assignee to VM name Aug 16, 2019
@kedark3 kedark3 changed the title [WIPTEST] Vm name enhancement - to postfix assignee to VM name [WIPTEST] Vm name enhancement - to append assignee to VM name Aug 22, 2019
vm = deploy_template(self.provider.key, self.name, self.template_name, **kwargs)
try:
from polarion_docstrings.parser import DocstringParser
assignee = DocstringParser(request.function.__doc__).parse()["assignee"].value
Copy link
Member

Choose a reason for hiding this comment

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

This metadata can be defined at levels other than the function. There are new methods which merge the meta from all levels, in a given file. I'm using them in #9225

@kedark3 kedark3 force-pushed the vm_name_enhancement branch 3 times, most recently from 4cb2e93 to 9dcbb02 Compare August 22, 2019 19:13
@@ -11,7 +12,15 @@

def _create_vm(request, template, provider, vm_name):
collection = provider.appliance.provider_based_collection(provider)
vm_obj = collection.instantiate(vm_name, provider, template_name=template.name)
try:
assignee = DocstringParser(request.function.__doc__).parse()["assignee"].value
Copy link
Member

Choose a reason for hiding this comment

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

With the most recent versions of the polarion tools (PR #9225 includes update) this metadata (composited from module/class/function) is available from the session.

Currently that session attribute is only populated if in collect only mode, I'm looking to change that in pytest-polarion-collect PR 5 or a followup.

This works for now for testing this functionality and designing the fixture structure, so I don't think it blocks working the rest of this PR. But ASAP this should be changed to use the pre-parsed (and composited) metadata.

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 have access to items and fspath through the session object. I do see nodeid too but that returns blank. I don't see what we can use to fetch the metadata as you mentioned in the comment above. I agree that once it becomes available we can use it.

n [15]: s.items                                                                                                                                                                                                                               
Out[15]: [<Function 'test_vm_name_postfix[virtualcenter-6.7-full_template]'>]

In [16]: s.fspath                                                                                                                                                                                                                              
Out[16]: local('/home/kkulkarn/git/integration_tests')

In [17]: s.nodeid                                                                                                                                                                                                                              
Out[17]: ''

except TypeError:
assignee = ""
if assignee:
vm_name = vm_name + "-" + assignee
Copy link
Member

Choose a reason for hiding this comment

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

I think the logic can be simplified a bit here to avoid adding these strings in a clunky way, and do it more consistently.

Consider using 'unassigned' or something similar when its missing.

@kedark3 kedark3 force-pushed the vm_name_enhancement branch 3 times, most recently from 8c0eeea to 0ab1b52 Compare August 22, 2019 20:34
@kedark3 kedark3 force-pushed the vm_name_enhancement branch from 0ab1b52 to f5c9ef8 Compare August 29, 2019 21:17
@kedark3 kedark3 force-pushed the vm_name_enhancement branch from be1ccbb to ab664fa Compare December 11, 2019 16:49
@kedark3 kedark3 force-pushed the vm_name_enhancement branch from ab664fa to a9e368c Compare January 10, 2020 14:00
@kedark3 kedark3 changed the title [1LP][WIP] Vm name enhancement - to append assignee to VM name [1LP][RFR] Vm name enhancement - to append assignee to VM name Jan 10, 2020
@dajoRH dajoRH added lint-ok and removed needs-lint labels Jan 10, 2020
@kedark3 kedark3 force-pushed the vm_name_enhancement branch from a9e368c to 494c338 Compare January 10, 2020 14:10
@dajoRH dajoRH removed the WIP label Jan 10, 2020
@kedark3 kedark3 force-pushed the vm_name_enhancement branch 2 times, most recently from 494c338 to f2c6b73 Compare January 10, 2020 18:06
@kedark3
Copy link
Contributor Author

kedark3 commented Jan 10, 2020

@jawatts @mshriver Travis failures are now resolved. PR is passing locally when ran against Wharf. PRT should pass too. 🤞 Kindly review.

@dajoRH dajoRH changed the title [1LP][RFR] Vm name enhancement - to append assignee to VM name [1LP][WIP] Vm name enhancement - to append assignee to VM name Jan 13, 2020
@kedark3 kedark3 force-pushed the vm_name_enhancement branch from f2c6b73 to ce67052 Compare January 13, 2020 16:31
@kedark3 kedark3 changed the title [1LP][WIP] Vm name enhancement - to append assignee to VM name [1LP][RFR] Vm name enhancement - to append assignee to VM name Jan 13, 2020
@dajoRH
Copy link
Contributor

dajoRH commented Jan 13, 2020

I detected some fixture changes in commit ce67052

Show fixtures

The global fixture create_vm_modscope is used in the following files:

  • cfme/tests/ansible/test_embedded_ansible_actions.py
    • policy_for_testing
  • cfme/tests/ansible/test_embedded_ansible_automate.py
    • alert_profile
    • test_embedded_ansible_custom_button_localhost
    • test_embedded_ansible_custom_button_target_machine
    • test_embedded_ansible_custom_button_specific_hosts
    • test_alert_run_ansible_playbook

The global fixture create_vm is used in the following files:

  • cfme/tests/automate/test_service_automate.py
    • test_retire_vm_now
  • cfme/tests/control/test_alerts.py
    • wait_candu
    • ssh
    • test_alert_rtp
    • test_alert_timeline_cpu
    • test_alert_snmp
  • cfme/tests/infrastructure/test_tenant_quota.py
    • test_quota_not_fails_after_vm_reconfigure_disk_remove
  • cfme/tests/services/test_dialog_element_in_catalog.py
    • test_service_dialog_expression_method
  • cfme/tests/webui/test_general_ui.py
    • test_vm_right_size_recommendation_back_button

The local fixture policy_for_testing is used in the following files:

  • cfme/tests/ansible/test_embedded_ansible_actions.py

The local fixture alert_profile is used in the following files:

  • cfme/tests/ansible/test_embedded_ansible_automate.py
    • test_alert_run_ansible_playbook

The local fixture wait_candu is used in the following files:

  • cfme/tests/control/test_alerts.py

The local fixture ssh is used in the following files:

  • cfme/tests/control/test_alerts.py

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

@dajoRH dajoRH removed the WIP label Jan 13, 2020
@jawatts jawatts merged commit 0ad7f68 into ManageIQ:master Jan 14, 2020
spusateri pushed a commit to spusateri/integration_tests that referenced this pull request Jan 27, 2020
…eIQ#9186)

* Extending create_on_provider method to postfix assignee to vm name

* Updating the approach for getting assignee for vm name using pytest_polarion_collect

* Updating minor things

* Adding more tests for function scope

* changing full_template_vm(_modscope) calls to create_vm(_modscope)

* Fixing Travis
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants