Updating blockpool tests with capacity and usage verifications #9784

shivamdurgbuns · 2024-05-09T12:46:54Z

Re-opening old PR #6609

ocs-ci

PR validation on existing cluster

Cluster Name: sdurgbun-test411
Cluster Configuration:
PR Test Suite: tier1
PR Test Path: tests/cross_functional/ui/test_create_pool_block_pool.py
Additional Test Params:
OCP VERSION: 4.11-ga
OCS VERSION: 4.11
tested against branch: master

Job PASSED.

ebondare · 2024-05-13T08:39:54Z

ocs_ci/ocs/ui/views.py

+    ),
+    "immediate_binding_mode": ("button[id='Immediate-link']", By.CSS_SELECTOR),
+    "storage_class_breadcrumb": (
+        "//a[@class='pf-c-breadcrumb__link'][text()='StorageClasses']",


This locator won't work in 4.16, after patternfly 5 changes. Could you please re-check the locators for a 4.16 cluster?

Sure will check it out

the tests are working fine

Could it be that you're not actually using this locator in the tests?

yea I see its not used, will remove storage_class_breadcrumb locater

DanielOsypenko · 2024-05-13T09:31:34Z

PR verification was done against OCP VERSION: 4.11-ga OCS VERSION: 4.11 and we want to merge to 4.16/master branch.
Please run the PR validation job against master branch, looks like you want to make the test pass on 4.12 and above, so you will need to merge to 4.16 first and cherry-pick to older versions.
The original bug was open against 4.9

DanielOsypenko · 2024-05-13T12:13:59Z

tests/cross_functional/ui/test_create_pool_block_pool.py

-        pytest.param(*[2, False], marks=pytest.mark.polarion_id("OCS-2586")),
+        # pytest.param(*[3, False], marks=pytest.mark.polarion_id("OCS-2588")),
+        # pytest.param(*[2, True], marks=pytest.mark.polarion_id("OCS-2587")),
+        # pytest.param(*[2, False], marks=pytest.mark.polarion_id("OCS-2586")),


most likely this is a leftover

removed the comments

DanielOsypenko · 2024-05-13T12:21:05Z

ocs_ci/ocs/ui/block_pool.py

+                f"Raw capacity of {pool_name} is {raw_capacity} MiB as checked by CLI"
+            )
+
+            used_raw_capacity_in_UI = self.get_element_text(


I think we have this functionality, please check - def get_raw_capacity_card_values(self)

Updated with function call

ocs-ci

PR validation on existing cluster

Cluster Name: sdurgbun-odf416
Cluster Configuration:
PR Test Suite: tier1
PR Test Path: tests/cross_functional/ui/test_create_pool_block_pool.py
Additional Test Params:
OCP VERSION: 4.16
OCS VERSION: 4.16
tested against branch: master

Job PASSED.

ebondare · 2024-05-15T14:47:38Z

ocs_ci/ocs/ui/block_pool.py

+
+        raw_capacity_loaded = self.check_element_text(
+            "Available",
+            "div[@class='ceph-raw-card-legend__title']",


Could you please move the actual locator to the views.py file?

ebondare · 2024-05-15T14:52:31Z

ocs_ci/ocs/ui/views.py

@@ -1135,6 +1131,15 @@
        "//p[@data-test='pool-bound-message']",
        By.XPATH,
    ),
+    "used_raw_capacity_in_UI": ("//div[@class='ceph-raw-card-legend__text']", By.XPATH),
+    "actions_inside_pool": (
+        "//div[@class='pf-c-dropdown pf-m-align-right']",


This locator also shouldn't be working in 4.16 and I don't see it used anywhere

This is used in older versions here

ocs-ci/ocs_ci/ocs/ui/block_pool.py

Line 90 in 354fc23

self.do_click(self.bp_loc["actions_inside_pool"])

and the code is also working for 4.16 as well I have checked.

openshift-ci · 2024-05-16T07:33:46Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: shivamdurgbuns

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2024-05-16T07:33:47Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: shivamdurgbuns

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ocs-ci

PR validation on existing cluster

Cluster Name: sdurgbun-test416
Cluster Configuration:
PR Test Suite: tier1
PR Test Path: tests/cross_functional/ui/test_create_pool_block_pool.py
Additional Test Params:
OCP VERSION: 4.16
OCS VERSION: 4.16
tested against branch: master

Job UNSTABLE (some or all tests failed).

shivamdurgbuns · 2024-05-16T08:37:59Z

All the tests have passed here

ebondare · 2024-05-16T09:16:21Z

ocs_ci/ocs/ui/block_pool.py

+        """
+        self.navigate_block_pool_page()
+        self.page_has_loaded()
+        self.do_click((f"//a[text()='{pool_name}']", By.XPATH))


Could you move the locator to views.py file?

we cant move this locator to the views.py as the locator is dynamic and we cant pass the pool name value to the views file as we are picking the locators from dictionary.

I'd suggest defining it the way we do in other such cases: https://github.com/red-hat-storage/ocs-ci/blob/master/ocs_ci/ocs/ui/views.py#L287

ebondare · 2024-05-16T09:17:07Z

ocs_ci/ocs/ui/block_pool.py

+
+    def select_blockpool(self, pool_name):
+        """
+        Selects and clicks on the blocpool according to the blockpool name passed.


Typo: blocpool

DanielOsypenko · 2024-05-16T09:28:17Z

ocs_ci/ocs/ui/block_pool.py

+            bf_obj = BlockAndFile()
+            used_raw_capacity_in_UI, _ = bf_obj.get_raw_capacity_card_values()
+
+            if used_raw_capacity_in_UI == f"{str(raw_capacity)} MiB":


raw capacity units are dynamic, may be MiB and may be not.

Since this function is a utility and not supposed to work only against one test, and we don't know what pool_name will be in the future use of it, we need to convert MiB from the "int(pools["size_kb"]) / 1024, 1" in whatever values are correct in a human readable format.

or

use another approach:
Convert values from UI and from CLI to bytes and compare bytes. Use human_to_bytes_ui() from ocs_ci/utility/utils.py

DanielOsypenko · 2024-05-16T09:32:33Z

ocs_ci/ocs/ui/block_pool.py

+                return True
+            else:
+                logger.error(
+                    f"UI value (i.e {used_raw_capacity_in_UI}) did not matched as per CLI for the Raw Capacity"


did not match

ocs-ci

PR validation on existing cluster

Cluster Name: sdurgbun-odf416
Cluster Configuration:
PR Test Suite: tier1
PR Test Path: tests/cross_functional/ui/test_create_pool_block_pool.py
Additional Test Params:
OCP VERSION: 4.16
OCS VERSION: 4.16
tested against branch: master

Job UNSTABLE (some or all tests failed).

shivamdurgbuns · 2024-05-20T14:13:44Z

All the tests passed here, error in teardown

ocs-ci

PR validation on existing cluster

Cluster Name: sdurgbun-odf416
Cluster Configuration:
PR Test Suite: tier1
PR Test Path: tests/cross_functional/ui/test_create_pool_block_pool.py
Additional Test Params:
OCP VERSION: 4.16
OCS VERSION: 4.16
tested against branch: master

Job UNSTABLE (some or all tests failed).

ocs-ci

PR validation on existing cluster

Cluster Name: sdurgbun-odf416
Cluster Configuration:
PR Test Suite: tier1
PR Test Path: tests/cross_functional/ui/test_create_pool_block_pool.py
Additional Test Params:
OCP VERSION: 4.16
OCS VERSION: 4.16
tested against branch: master

Job UNSTABLE (some or all tests failed).

ocs-ci

PR validation on existing cluster

Cluster Name: sdurgbun-odf416
Cluster Configuration:
PR Test Suite: tier1
PR Test Path: tests/cross_functional/ui/test_create_pool_block_pool.py
Additional Test Params:
OCP VERSION: 4.16
OCS VERSION: 4.16
tested against branch: master

Job PASSED.

ocs-ci

PR validation on existing cluster

Cluster Name: sdurgbun-test416
Cluster Configuration:
PR Test Suite: tier1
PR Test Path: tests/cross_functional/ui/test_create_pool_block_pool.py
Additional Test Params:
OCP VERSION: 4.16
OCS VERSION: 4.16
tested against branch: master

Job UNSTABLE (some or all tests failed).

shivamdurgbuns · 2024-05-30T15:14:10Z

I have ran the test here and all have passed, could we please merge this pr
job: https://ocs4-jenkins-csb-odf-qe.apps.ocp-c1.prod.psi.redhat.com/job/qe-deploy-ocs-cluster/38134/parameters/
test results: https://ocs4-jenkins-csb-odf-qe.apps.ocp-c1.prod.psi.redhat.com/job/qe-deploy-ocs-cluster/38134/testReport/

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

DanielOsypenko · 2024-05-30T15:32:10Z

ocs_ci/ocs/exceptions.py

@@ -706,7 +706,10 @@ class ClusterNotInSTSModeException(Exception):

 class APIRequestError(Exception):
    pass
+<<<<<<< HEAD


please remove merge conflict leftovers

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

DanielOsypenko · 2024-06-02T07:27:48Z

ocs_ci/helpers/helpers.py

@@ -4776,3 +4776,22 @@ def is_rbd_default_storage_class(custom_sc=None):

    logger.error("Storageclass {default_rbd_sc} is not a default  RBD StorageClass.")
    return False
+def custom_round(number):


I think, no new line before the end of the previous method return statement and custom_round is one of the reasons PR does not pass PR validation (Tox) check

DanielOsypenko · 2024-06-02T07:40:20Z

ocs_ci/helpers/helpers.py

+    number_str = str(number)
+
+    # Check if the decimal part exists and ends in ".5"
+    if "." in number_str and number_str.endswith(".5"):


Hey.
Func may cause the bugs

Input: 2.504
Expected Output: 3 (since it's slightly more than 2.5)
Actual Output: 2 (since .endswith(".5") check fails due to floating-point representation)

Negative numbers and numbers such as 3.49 will also cause the bug.

We don't need this function in helpers, it is not universal and to specific to your needs, please move the logic to the test or function to the test module.

DanielOsypenko · 2024-06-02T07:44:03Z

ocs_ci/ocs/ui/block_pool.py

+            # splitting the ouptut with spaces
+            # the blockpool used capacity will always be at index 17 and 18
+            # where the index 17 is the value and index 18 is the unit
+            used_capacity_in_CLI, unit = df_op.split()[17:19]


[17:19] - is this aways the case, even when numbers and units are different? I think we need some dynamic/intelectual parsing here to get numbers from the text. Please use regex-like approaches to get what you need from the string

DanielOsypenko · 2024-06-02T07:44:56Z

ocs_ci/ocs/ui/block_pool.py

+        self.navigate_block_pool_page()
+        self.page_has_loaded()
+        self.do_click(
+            locator=format_locator(self.generic_locators["blockpool_name"], pool_name)


DanielOsypenko · 2024-06-02T07:45:19Z

tests/cross_functional/ui/test_create_pool_block_pool.py

@@ -118,6 +120,13 @@ def test_create_delete_pool(
        # Getting IO results
        get_fio_rw_iops(self.pod_obj)

+        # Checking the raw capcity is loaded on the UI or not.
+        blockpool_ui_object = BlockPoolUI()
+        assert blockpool_ui_object.pool_raw_capacity_loaded(self.pool_name)


nice! Please add a string explanation to the assertion
assert blockpool_ui_object.pool_raw_capacity_loaded(self.pool_name), "Block pool raw capacity is not visible on UI"

DanielOsypenko · 2024-06-02T07:47:07Z

tests/cross_functional/ui/test_create_pool_block_pool.py

+        assert blockpool_ui_object.pool_raw_capacity_loaded(self.pool_name)
+
+        # Cross checking the raw capacity of the blockpool between CLI and UI
+        assert blockpool_ui_object.cross_check_raw_capacity(self.pool_name)


same as above, please use string explanation

shivamdurgbuns requested review from a team as code owners May 9, 2024 12:46

pull-request-size bot added the size/L PR that changes 100-499 lines label May 9, 2024

ocs-ci reviewed May 9, 2024

View reviewed changes

ramkiperiy previously approved these changes May 9, 2024

View reviewed changes

openshift-ci bot assigned ramkiperiy May 9, 2024

openshift-ci bot added the lgtm label May 9, 2024

shivamdurgbuns added the Verified Mark when PR was verified and log provided label May 9, 2024

ebondare reviewed May 13, 2024

View reviewed changes

DanielOsypenko added the Squad/Black Covers UI specific test cases such as ODF Dashboard etc. label May 13, 2024

DanielOsypenko requested a review from a team May 13, 2024 09:25

DanielOsypenko requested changes May 13, 2024

View reviewed changes

openshift-ci bot assigned DanielOsypenko May 13, 2024

openshift-ci bot removed the lgtm label May 13, 2024

ocs-ci reviewed May 15, 2024

View reviewed changes

ebondare reviewed May 15, 2024

View reviewed changes

shivamdurgbuns dismissed ramkiperiy’s stale review via 1063590 May 16, 2024 07:33

ocs-ci reviewed May 16, 2024

View reviewed changes

shivamdurgbuns requested review from ramkiperiy, DanielOsypenko and ebondare May 16, 2024 08:38

ebondare reviewed May 16, 2024

View reviewed changes

DanielOsypenko reviewed May 16, 2024

View reviewed changes

ocs-ci reviewed May 20, 2024

View reviewed changes

shivamdurgbuns force-pushed the reopen/bz199967 branch from 88e2e71 to 72b5b4b Compare May 22, 2024 10:28

ocs-ci reviewed May 22, 2024

View reviewed changes

ocs-ci reviewed May 23, 2024

View reviewed changes

This comment was marked as duplicate.

Sign in to view

ocs-ci reviewed May 23, 2024

View reviewed changes

openshift-merge-robot added the needs-rebase label May 27, 2024

shivamdurgbuns added 10 commits May 30, 2024 20:49

Updating blockpool tests with capacity and usage verifications

6b1d5c5

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

Uncommenting the params

69a4719

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

Function change

1b3ef5b

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

Removing exceptions and adding assertions

4f7c1bf

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

Updating the function for calculating used capacity

7c0d22f

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

Uncommenting the tests

e562a08

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

Adding blockpool generic locator

b4ccdc3

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

Adding custom round function

f145c88

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

Adding sanity check

6390191

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

Removing sanity check

3241cda

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

shivamdurgbuns force-pushed the reopen/bz199967 branch from b907c6c to 3241cda Compare May 30, 2024 15:21

openshift-merge-robot removed the needs-rebase label May 30, 2024

DanielOsypenko reviewed May 30, 2024

View reviewed changes

Resolving conflict

04b0940

Signed-off-by: Shivam Durgbuns <sdurgbun@redhat.com>

DanielOsypenko reviewed Jun 2, 2024

View reviewed changes

Updating blockpool tests with capacity and usage verifications #9784

Are you sure you want to change the base?

Updating blockpool tests with capacity and usage verifications #9784

Conversation

shivamdurgbuns commented May 9, 2024

ocs-ci 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

DanielOsypenko commented May 13, 2024

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

ocs-ci 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

openshift-ci bot commented May 16, 2024

openshift-ci bot commented May 16, 2024

ocs-ci left a comment

Choose a reason for hiding this comment

shivamdurgbuns commented May 16, 2024

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

ocs-ci left a comment

Choose a reason for hiding this comment

shivamdurgbuns commented May 20, 2024

ocs-ci left a comment

Choose a reason for hiding this comment

ocs-ci left a comment

Choose a reason for hiding this comment

ocs-ci left a comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

ocs-ci left a comment

Choose a reason for hiding this comment

shivamdurgbuns commented May 30, 2024

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

DanielOsypenko Jun 2, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielOsypenko Jun 2, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielOsypenko Jun 2, 2024 •

edited

DanielOsypenko Jun 2, 2024 •

edited