[RFR] Added Pod.pods_per_ready_status + Fix smart management test #4922

gshefer · 2017-06-28T12:53:55Z

{{pytest: cfme/tests/containers/test_reload_button_provider.py cfme/tests/containers/test_properties.py cfme/tests/containers/test_reports.py cfme/tests/containers/test_smart_management.py -v --use-provider cm-env2}}

Created Pod.pods_per_ready_status in order to be able to share this usage between test_pods_conditions and test_report_pods_per_ready_status
Fixed test_smart_management which didn't wait enough time to the tags to be populated.

jarovo · 2017-07-04T12:39:48Z

cfme/tests/containers/test_properties.py

+
+        for status in cfme_pod_condition:
+            soft_assert(ose_pod_condition[status] == cfme_pod_condition[status],
+                        'Expected "{}" status for pod {} is {}. Found {} instead.'


The English grammar is bad in the error message and it doesn't match the assert condition. Why it is not saying something like

The Pod {} status mismatch: It is "{}" in openshift while cfme sees "{}".

Not mentioning that it doesn't make much sense to me why one needs to first group the pods by it status to compare them then. It smells like the Polarion prescription is bad, but I cannot check it in the moment.

No problem I'll fix that.

jarovo · 2017-07-04T12:44:55Z

cfme/tests/containers/test_smart_management.py

+        return wait_for(lambda: getattr(obj_inst.summary.smart_management,
+                                        'my_company_tags', []), fail_condition=[],
+                        num_sec=30, delay=5, fail_func=obj_inst.summary.reload).out.pop()
+    except TimedOutError:


Why did you choose to silence the exception?

Returning none will cause problems at lines 89 and 98

jarovo

"General fixes" is too general for patch title.

psav · 2017-07-05T12:21:05Z

cfme/tests/containers/test_properties.py

@@ -132,8 +134,7 @@
 ]


-@pytest.mark.parametrize('test_item', TEST_ITEMS,
-                         ids=[ti.args[1].pretty_id() for ti in TEST_ITEMS])
+@pytest.mark.parametrize('test_item', TEST_ITEMS)


Why remove this?

causes problem when there is a bugzilla marker, it's too complicating the way to call that (sometimes args[1].pretty_id, sometimes args[0].args[1].pretty_id). I can create some function that treat this but i think it's too much for this purpose. what you think?

I need to have something that is either a string, or has a .name attribute for a parameter else it will screw up future reporting.

psav · 2017-07-05T12:21:36Z

cfme/tests/containers/test_properties.py

@@ -143,7 +144,6 @@ def test_properties(provider, test_item, soft_assert):

    for instance in instances:

-        navigate_to(instance, 'Details')


We don't need to navigate anymore?

No, The instance.summary below is already do it for us.

psav · 2017-07-05T12:22:20Z

cfme/tests/containers/test_reload_button_provider.py

    """

-    navigate_to(provider, 'Details')
-    tb.select('Reload Current Display')
    provider.validate_stats(ui=True)


Is the refresh done as part of this func call?

https://github.com/ManageIQ/integration_tests/blob/master/cfme/common/provider.py#L454

psav · 2017-07-05T12:23:39Z

cfme/tests/containers/test_reports.py

@@ -91,24 +76,24 @@ def test_container_reports_base_on_options(soft_assert):
        soft_assert(option, 'Could not find option "{}" for base report on.'.format(base_on))


-@pytest.mark.meta(blockers=[BZ(1435958, forced_streams=["5.8"])])
+@pytest.mark.meta(blockers=[BZ(1467059, forced_streams=["5.8"])])


If the bug applies to all the tests in the file, you can mark the test at the beginning of the file and apply it to all

psav · 2017-07-05T14:52:23Z

cfme/fixtures/pods_per_ready_status.py

+
+
+@pytest.fixture(scope='function')
+def pods_per_ready_status(provider):


I dont see a need for this to be a fixture, this could just be a method on the container object

OK, so it should be on the Pod object as static method for all pods (since we want just 1 API call)

psav

One minor change

psav · 2017-07-11T08:54:08Z

cfme/containers/pod.py

@@ -57,6 +59,19 @@ def get_random_instances(cls, provider, count=1, appliance=None):
        return [cls(obj.name, obj.project_name, provider, appliance=appliance)
                for obj in itertools.islice(pod_list, count)]

+    @staticmethod


So, a few things here,

This function/method is dealing with pods from a specific provider
Therefore, either this function should be a method of the provider object, then there is no need to pass a provider to it.

mshriver · 2017-07-11T14:17:26Z

cfme/containers/pod.py

@@ -3,6 +3,8 @@
 import random
 import itertools

+from wrapanapi.utils import eval_strings


lint bot isn't running right now, this isn't used anywhere in the module?

mshriver · 2017-07-11T14:17:55Z

cfme/containers/provider/__init__.py



+from cfme.base.login import BaseLoggedInPage


mshriver · 2017-07-11T14:19:44Z

cfme/containers/provider/__init__.py

+    def pods_per_ready_status(self):
+        """Grabing the Container Statuses Summary of the pods from API"""
+        #  TODO: Add later this logic to wrapanapi
+        entities_j = self.mgmt.api.get('pod')[1]['items']


why _j on this variable name? nitpicking, but it doesn't seem to make sense, entities would be sufficient.

mshriver · 2017-07-11T14:23:53Z

cfme/containers/provider/__init__.py

@@ -269,6 +270,18 @@ def num_image_registry(self):
    def num_image_registry_ui(self):
        return int(self.get_detail("Relationships", "Image Registries"))

+    def pods_per_ready_status(self):
+        """Grabing the Container Statuses Summary of the pods from API"""
+        #  TODO: Add later this logic to wrapanapi


RedHatQE/wrapanapi#152

(+ @psav ) Indeed, I didn't forgot that, my next step will be to migrate all these functions to wrapanapi (this is not the only one, I have similar TODOS all over the framework)

gshefer force-pushed the general_fixes_pt2 branch 3 times, most recently from af39366 to eb32d7b Compare July 4, 2017 12:23

gshefer changed the title ~~[WIPTEST] General Fixes~~ [RFR] General Fixes Jul 4, 2017

jarovo suggested changes Jul 4, 2017

View reviewed changes

gshefer force-pushed the general_fixes_pt2 branch from eb32d7b to 1d0cb30 Compare July 4, 2017 13:18

gshefer closed this Jul 4, 2017

gshefer reopened this Jul 4, 2017

gshefer changed the title ~~[RFR] General Fixes~~ [1LP][RFR] General Fixes Jul 4, 2017

gshefer closed this Jul 4, 2017

gshefer reopened this Jul 4, 2017

gshefer changed the title ~~[1LP][RFR] General Fixes~~ [1LP][RFR] Add pods_per_ready_status fixture + Fix smart management test Jul 4, 2017

jarovo approved these changes Jul 4, 2017

View reviewed changes

gshefer force-pushed the general_fixes_pt2 branch 4 times, most recently from 09c5d59 to 071011e Compare July 5, 2017 08:14

psav reviewed Jul 5, 2017

View reviewed changes

gshefer force-pushed the general_fixes_pt2 branch from 071011e to 752e05c Compare July 5, 2017 13:04

psav reviewed Jul 5, 2017

View reviewed changes

gshefer force-pushed the general_fixes_pt2 branch 6 times, most recently from 7b441df to f7a84ab Compare July 6, 2017 13:15

gshefer force-pushed the general_fixes_pt2 branch from f7a84ab to 7b441df Compare July 6, 2017 13:16

gshefer changed the title ~~[1LP][RFR] Add pods_per_ready_status fixture + Fix smart management test~~ [1LP][RFR] Add Pod.pods_per_ready_status + Fix smart management test Jul 9, 2017

gshefer changed the title ~~[1LP][RFR] Add Pod.pods_per_ready_status + Fix smart management test~~ [1LP][RFR] Added Pod.pods_per_ready_status + Fix smart management test Jul 9, 2017

psav suggested changes Jul 11, 2017

View reviewed changes

psav changed the title ~~[1LP][RFR] Added Pod.pods_per_ready_status + Fix smart management test~~ [1LP][WIP] Added Pod.pods_per_ready_status + Fix smart management test Jul 11, 2017

gshefer force-pushed the general_fixes_pt2 branch from 7b441df to f8c1a2d Compare July 11, 2017 09:42

gshefer changed the title ~~[1LP][WIP] Added Pod.pods_per_ready_status + Fix smart management test~~ [1LP][WIPTEST] Added Pod.pods_per_ready_status + Fix smart management test Jul 11, 2017

gshefer changed the title ~~[1LP][WIPTEST] Added Pod.pods_per_ready_status + Fix smart management test~~ [1LP][RFR] Added Pod.pods_per_ready_status + Fix smart management test Jul 11, 2017

mshriver changed the title ~~[1LP][RFR] Added Pod.pods_per_ready_status + Fix smart management test~~ [RFR] Added Pod.pods_per_ready_status + Fix smart management test Jul 11, 2017

mshriver added enhancement fix-test labels Jul 11, 2017

mshriver reviewed Jul 11, 2017

View reviewed changes

cfme/containers/provider/__init__.py

from cfme.base.login import BaseLoggedInPage

Copy link

Member

mshriver Jul 11, 2017

Choose a reason for hiding this comment

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

👍

mshriver reviewed Jul 11, 2017

View reviewed changes

gshefer force-pushed the general_fixes_pt2 branch 2 times, most recently from 69cba31 to 91bb084 Compare July 11, 2017 15:03

General Fixes

29a9fd8

gshefer force-pushed the general_fixes_pt2 branch from 91bb084 to 29a9fd8 Compare July 11, 2017 15:08

psav approved these changes Jul 12, 2017

View reviewed changes

psav merged commit 45893f3 into ManageIQ:master Jul 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFR] Added Pod.pods_per_ready_status + Fix smart management test #4922

[RFR] Added Pod.pods_per_ready_status + Fix smart management test #4922

gshefer commented Jun 28, 2017 •

edited

Loading

jarovo Jul 4, 2017

gshefer Jul 4, 2017

jarovo Jul 4, 2017

jarovo left a comment

psav Jul 5, 2017

gshefer Jul 5, 2017

psav Jul 6, 2017

psav Jul 5, 2017

gshefer Jul 5, 2017

psav Jul 5, 2017

gshefer Jul 5, 2017

psav Jul 5, 2017

psav Jul 5, 2017

gshefer Jul 5, 2017

psav left a comment

psav Jul 11, 2017

mshriver Jul 11, 2017

mshriver Jul 11, 2017

mshriver Jul 11, 2017

mshriver Jul 11, 2017

gshefer Jul 11, 2017

		@@ -143,7 +144,6 @@ def test_properties(provider, test_item, soft_assert):

		for instance in instances:

		navigate_to(instance, 'Details')



		@pytest.fixture(scope='function')
		def pods_per_ready_status(provider):

[RFR] Added Pod.pods_per_ready_status + Fix smart management test #4922

[RFR] Added Pod.pods_per_ready_status + Fix smart management test #4922

Conversation

gshefer commented Jun 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarovo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psav left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gshefer commented Jun 28, 2017 •

edited

Loading