[1LP][RFR] CLI - Testing restoring DB using NFS - negative

[dgaikwad]

Purpose or Intent

Adding tests Testing restoring DB using NFS via appliance console by providing negative input.

PRT Run

{{pytest: ./cfme/tests/cli/test_appliance_console.py::test_appliance_console_network_conf_negative ./cfme/tests/cli/test_appliance_console.py::test_appliance_console_static_ip_negative ./cfme/tests/cli/test_appliance_console.py::test_appliance_console_key_fetch_negative ./cfme/tests/cli/test_appliance_console.py::test_appliance_console_restore_db_network_negative ./cfme/tests/cli/test_appliance_console.py::test_appliance_console_key_fetch_negative -v }}

[john-dupuy]

Two small questions. Thanks for all the automation work @dgaikwad!

[john-dupuy]

LGTM, thanks for this PR

[jawatts]

This should just be result, we don't want to print the entire stout everytime

[dgaikwad]

We are interested in CLI behavior for entire commands not only for the last command which we executed from command list.

[dgaikwad]

We are just printing stdout once after all commands execution is done.

[jawatts]

@dgaikwad After seeing your explanation, I brought up this up in the reviewers call and we are suggesting the following updates:

1. Line 60, change this from debug to info. We should be logging the result of each command on each iteration
2. Remove output as an arg and the if statement on line 61. Code on line 62 and 62 should always be run.
3. Line 37, we should be creating a dict here instead, the key being the command, and the value being the result. Then log that dict on line 62.

I hope that makes sense!

[dgaikwad]

@jawatts Thank you for the first two suggestion but in third suggestion, if we make dict instead of list then here we are going to override result.
Example:
commands = ("ap", "", "5", "2", "1", "2", "2")
In above example we will not able to store result separately for all 2 and 1 command and we will not get which command executed first due to its dict type.

[jawatts]

Two comments

[jawatts]

@dgaikwad I think there is one more case here: https://github.com/ManageIQ/integration_tests/blob/master/cfme/tests/cli/test_appliance_console.py#L1368

Perhaps your PR isn't on the latest commit?

[dgaikwad]

@jawatts Based on your last comment I have done the changes