[1LP][RFR] Add support for cancel and nav_away to Host.update() and test_infrastructure_hosts_crud #10090

prichard77 · 2020-05-04T20:56:58Z

Purpose or Intent

Adding tests for host edit to support cancel of edit and also navigating away before saving updates. The navigating away case will cover customer BZ 1634794.

PRT Run

{{ pytest: --use-provider complete --long-running cfme/tests/infrastructure/test_host.py::test_infrastructure_hosts_crud }}

prichard77 · 2020-05-04T23:55:59Z

I am still making small updates to this and testing, but I'd like to start the review.

john-dupuy

Thanks for this PR @prichard77, I have some comments and suggestions, please take a look!

cfme/common/host_views.py

cfme/infrastructure/host.py

cfme/tests/infrastructure/test_host.py

prichard77 · 2020-05-05T14:13:50Z

FYI, This PR includes commits from PR#9961. At the moment it contains all of the commits for PR#9961.

cfme/infrastructure/host.py

cfme/tests/infrastructure/test_host.py

jarovo · 2020-05-05T13:48:13Z

cfme/tests/infrastructure/test_host.py

+            else:
+                raise
+        if crud_action not in ['cancel', 'nav_away_changes', 'nav_away_no_changes']:
+            assert host.custom_ident == new_custom_id  # is this actually doing anything here


Perhaps the host.custom_ident comes from REST API? Doesn't seem like though when I take a look on the code around the Host

It's a UI item. It can be seen on the details page and edit pages.

cfme/tests/infrastructure/test_host.py

cfme/infrastructure/host.py

prichard77 · 2020-05-05T15:33:27Z

I am setting this back to WIPTEST as I have a few things to work out based upon comments, and I want reviewers to focus on other PRs for the moment.

prichard77 · 2020-05-07T16:27:13Z

I am updating some comments in host.update that I forget to update, but otherwise changed usage of action string to kwargs. I'll be pushing changes to comments shortly, but would still like review comments.

cfme/infrastructure/host.py

cfme/tests/infrastructure/test_host.py

…crud for clarity and easier understanding

…eption and default to from_details

prichard77 · 2020-05-13T21:56:09Z

I updated so that edit from details would be the default behavior as it was originally.

prichard77 · 2020-05-14T04:45:21Z

rerunning PRT. I am not sure why it's failing as it is passing when I run locally.

mshriver

The approach of building the behavior for the test into Host.update has been rejected, please see individual comments. Thanks @prichard77

… actions

…in cancel calls in test_infrastructure_hosts_crud

cfme/infrastructure/host.py

cfme/tests/infrastructure/test_host.py

mshriver

Two more really small changes requested, otherwise the test is looking great now!

… default value

prichard77 changed the title ~~[WIP] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud~~ [WIPTEST] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud May 4, 2020

dajoRH added lint-ok WIP-testing labels May 4, 2020

prichard77 changed the title ~~[WIPTEST] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud~~ [RFR] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud May 4, 2020

dajoRH removed the WIP-testing label May 5, 2020

john-dupuy suggested changes May 5, 2020

View reviewed changes

mshriver reviewed May 5, 2020

View reviewed changes

cfme/infrastructure/host.py Outdated Show resolved Hide resolved

jarovo reviewed May 5, 2020

View reviewed changes

mshriver reviewed May 5, 2020

View reviewed changes

cfme/infrastructure/host.py Outdated Show resolved Hide resolved

mshriver reviewed May 5, 2020

View reviewed changes

cfme/infrastructure/host.py Outdated Show resolved Hide resolved

prichard77 changed the title ~~[RFR] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud~~ [WIPTEST] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud May 5, 2020

dajoRH added the WIP-testing label May 5, 2020

dajoRH changed the title ~~[WIPTEST] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud~~ [WIP] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud May 6, 2020

dajoRH added needs-rebase WIP and removed WIP-testing labels May 6, 2020

prichard77 force-pushed the newIncMultiEdit branch from 329f5ca to c10f76e Compare May 7, 2020 14:36

dajoRH removed the needs-rebase label May 7, 2020

prichard77 changed the title ~~[WIP] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud~~ [WIPTEST] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud May 7, 2020

dajoRH added WIP-testing and removed WIP labels May 7, 2020

prichard77 changed the title ~~[WIPTEST] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud~~ [RFR] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud May 7, 2020

dajoRH removed the WIP-testing label May 7, 2020

john-dupuy approved these changes May 7, 2020

View reviewed changes

john-dupuy changed the title ~~[RFR] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud~~ [1LP][RFR] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud May 7, 2020

tpapaioa suggested changes May 8, 2020

View reviewed changes

Add comments to and rearrange structure of test_infrastructure_hosts_…

65d4c46

…crud for clarity and easier understanding

prichard77 changed the title ~~[1LP][WIPTEST] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud~~ [1LP][RFR] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud May 13, 2020

dajoRH added lint-ok and removed needs-lint WIP-testing labels May 13, 2020

prichard77 added 2 commits May 13, 2020 17:29

Add more comments and update docblock for clarity

0f4ae4d

Update kwargs and associated logic to make from_hosts the one off exc…

401ee6a

…eption and default to from_details

prichard77 requested a review from mshriver May 13, 2020 21:57

mshriver changed the title ~~[1LP][RFR] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud~~ [1LP][WIPTEST] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud May 14, 2020

mshriver suggested changes May 14, 2020

View reviewed changes

dajoRH added the WIP-testing label May 14, 2020

prichard77 added 2 commits May 14, 2020 12:09

Remove nav_away and changes kwargs from Host.update()

5ecc395

Rewrite test_infrastructure_hosts_crud to not use parameterization of…

00ca912

… actions

prichard77 changed the title ~~[1LP][WIPTEST] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud~~ [1LP][RFR] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud May 14, 2020

dajoRH removed the WIP-testing label May 14, 2020

Set from_details default to True in Host.update() and set it to True …

155afde

…in cancel calls in test_infrastructure_hosts_crud

prichard77 requested a review from mshriver May 14, 2020 17:33

mshriver reviewed May 15, 2020

View reviewed changes

cfme/infrastructure/host.py Outdated Show resolved Hide resolved

mshriver reviewed May 15, 2020

View reviewed changes

cfme/infrastructure/host.py Outdated Show resolved Hide resolved

mshriver reviewed May 15, 2020

View reviewed changes

cfme/tests/infrastructure/test_host.py Outdated Show resolved Hide resolved

mshriver suggested changes May 15, 2020

View reviewed changes

prichard77 added 2 commits May 15, 2020 10:15

Update from_details logic and remove cancel kwarg assignements to the…

16f7747

… default value

Combine multi lines into one where applicable

de75db5

prichard77 requested a review from mshriver May 15, 2020 14:32

mshriver merged commit c63b987 into ManageIQ:master May 19, 2020

jawatts added fix-framework new-test-or-feature labels May 22, 2020

prichard77 changed the title ~~[1LP][RFR] Add support for create and nav_away to Host.update() and test_infrastructure_hosts_crud~~ [1LP][RFR] Add support for cancel and nav_away to Host.update() and test_infrastructure_hosts_crud May 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1LP][RFR] Add support for cancel and nav_away to Host.update() and test_infrastructure_hosts_crud #10090

[1LP][RFR] Add support for cancel and nav_away to Host.update() and test_infrastructure_hosts_crud #10090

prichard77 commented May 4, 2020 •

edited

Loading

prichard77 commented May 4, 2020

john-dupuy left a comment

prichard77 commented May 5, 2020

jarovo May 5, 2020

prichard77 May 12, 2020

prichard77 commented May 5, 2020

prichard77 commented May 7, 2020

prichard77 commented May 13, 2020

prichard77 commented May 14, 2020

mshriver left a comment

mshriver left a comment

[1LP][RFR] Add support for cancel and nav_away to Host.update() and test_infrastructure_hosts_crud #10090

[1LP][RFR] Add support for cancel and nav_away to Host.update() and test_infrastructure_hosts_crud #10090

Conversation

prichard77 commented May 4, 2020 • edited Loading

Purpose or Intent

PRT Run

prichard77 commented May 4, 2020

john-dupuy left a comment

Choose a reason for hiding this comment

prichard77 commented May 5, 2020

jarovo May 5, 2020

Choose a reason for hiding this comment

prichard77 May 12, 2020

Choose a reason for hiding this comment

prichard77 commented May 5, 2020

prichard77 commented May 7, 2020

prichard77 commented May 13, 2020

prichard77 commented May 14, 2020

mshriver left a comment

Choose a reason for hiding this comment

mshriver left a comment

Choose a reason for hiding this comment

prichard77 commented May 4, 2020 •

edited

Loading