Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Added onboarding token generation related functional tests for provider-client feature #9938

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

amr1ta
Copy link
Contributor

@amr1ta amr1ta commented Jun 12, 2024

Added onboarding token generation related functional tests for provider-client feature

@amr1ta amr1ta requested review from a team as code owners June 12, 2024 14:11
@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines label Jun 12, 2024
@amr1ta amr1ta force-pushed the onboarding-token-generation-ui branch 14 times, most recently from cc42719 to 9020bb9 Compare June 13, 2024 16:29
jilju
jilju previously approved these changes Jun 14, 2024


@tier1
@green_squad
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a green squad test. May be yellow squad ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have updated in latest commit.

@pytest.fixture(autouse=True)
def setup(self, request):
"""
Resetting the default value of KeyRotation
Copy link
Contributor

Choose a reason for hiding this comment

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

Any code missing in this method for resetting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in latest commit.

Comment on lines 54 to 198
Test to verify the respin of ux pod.
Steps:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Test to verify the respin of ux pod.
Steps:
Test to verify the respin of ux pod.
Steps:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in latest commit.

timeout=180,
), "ux pod is not in running state as expected"

def test_respin_of_ux_server_pod(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a tier4c test. Please add marker in the test case level for all test cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in latest commit.

selector=constants.UX_BACKEND_SERVER_LABEL,
resource_count=1,
timeout=180,
), "ux pod is not in running state as expected"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
), "ux pod is not in running state as expected"
), "ux-backend-server pod is not in running state as expected"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in latest commit.

"""
Test to verify the respin of ux pod.
Steps:
1. navigate to storage-->storage clients page
Copy link
Contributor

Choose a reason for hiding this comment

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

Please mention the step of deleting the ux-backend-server pod.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in latest commit.


ux_pod_obj.delete()
log.info("wait some time for another ux-backend-server pod to come up")
time.sleep(30)
Copy link
Contributor

Choose a reason for hiding this comment

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

The new pod will come up in 3-5 seconds. Please remove sleep for short period and use wait conditions for checking the new pod.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in latest commit.

timeout=180,
), "ux pod is not in running state as expected"

def test_respin_of_ux_server_pod(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a separate test case to check the re-spin of pod is not needed. This will be part of tier4c pod re-spin tests. Please combine the test with test_onboarding_token_generation_option_is_available_in_ui(and change the name of test) to verify the impact of ux pod respin on token generation.

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 combined the test to check ux-backend-server pod is urunning and ux-backend-server pod respin testcases together.

@openshift-ci openshift-ci bot added the lgtm label Jun 14, 2024
@jilju jilju self-requested a review June 14, 2024 10:38
@jilju jilju removed their assignment Jun 14, 2024
fbalak
fbalak previously approved these changes Jun 14, 2024
@amr1ta amr1ta dismissed stale reviews from fbalak and jilju via 5d81873 June 14, 2024 14:41
@openshift-ci openshift-ci bot removed the lgtm label Jun 14, 2024
@ebondare
Copy link
Contributor

Could you please include a test run?

@amr1ta amr1ta force-pushed the onboarding-token-generation-ui branch from 5d81873 to 65e6515 Compare August 7, 2024 06:26
ebondare
ebondare previously approved these changes Aug 13, 2024
assert HostedODF(cluster_name).get_storage_client_status() == "Connected"

log.info("Destroy hosted cluster")
assert destroy_hosted_cluster(cluster_name), "Failed to destroy hosted cluster"
Copy link
Contributor

Choose a reason for hiding this comment

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

fyi destroy_hosted_cluster will leave leftovers, ceph resources on provider.
Lets add this to the docstring of the test?

), f"Failed to switch to cluster '{cluster_name}' and fetch data"

log.info("Test create onboarding key")
HostedClients().download_hosted_clusters_kubeconfig_files()
Copy link
Contributor

Choose a reason for hiding this comment

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

self.download_hosted_clusters_kubeconfig_files() already happened at HostedClients.do_deploy, you can remove this line

)
ux_pod_obj = pod.Pod(**ux_pod[0])
log.info("ux backed server pod respinned")
assert pod.validate_pods_are_respinned_and_running_state([ux_pod_obj])
Copy link
Contributor

Choose a reason for hiding this comment

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

pls add string with failure context

4. user can generate onboarding token by selecting this option.
"""
secret_ocp_obj = ocp.OCP(
kind=constants.SECRET, namespace=constants.OPENSHIFT_STORAGE_NAMESPACE
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to use config.ENV_DATA['cluster_namespace'] instead of constant constants.OPENSHIFT_STORAGE_NAMESPACE

).is_selected(), "Default value unlimited quota is not selected"

# Take screenshot
self.take_screenshot()
Copy link
Contributor

@DanielOsypenko DanielOsypenko Sep 25, 2024

Choose a reason for hiding this comment

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

pls use description param, this way we will identify screenshot by name in the UI logs dir
something like
self.take_screenshot("verify_storage_clients_page")

acm_page["all-clusters_dropdown_item"]
)
all_clusters_item.click()
if not not find_element:
Copy link
Contributor

Choose a reason for hiding this comment

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

due to implementation of wait_for_element_to_be_visible, it will fail if element not found.
The if not not find_element will not work.

Also "not" has been used 2 times

def wait_for_element_to_be_visible(locator, timeout=30):
    """
    Wait for element to be visible. Use when Web element is not have to be clickable (icons, disabled btns, etc.)
    Method does not fail when Web element not found

    Args:
         locator (tuple): (GUI element needs to operate on (str), type (By)).
         timeout (int): Looks for a web element until timeout (sec) occurs

    Returns:
        selenium.webdriver.remote.webelement.WebElement: Visible web element.
    """
    wait = WebDriverWait(SeleniumDriver(), timeout)
    try:
        web_element = wait.until(
            ec.visibility_of_element_located((locator[1], locator[0]))
        )
    except TimeoutException:
        take_screenshot()
        copy_dom()
        raise
    return web_element

@amr1ta amr1ta force-pushed the onboarding-token-generation-ui branch from 98a04e3 to 1cfa5e4 Compare October 7, 2024 08:56
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
…client from the hosted cluster

Signed-off-by: Amrita Mahapatra <[email protected]>
…oken with limited storage quota

Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
…timeout for local-cluster load

Signed-off-by: Amrita Mahapatra <[email protected]>
…own' locator in navigate_to_local_cluster method

Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
@amr1ta amr1ta force-pushed the onboarding-token-generation-ui branch from 1cfa5e4 to 0ef5608 Compare October 10, 2024 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress size/L PR that changes 100-499 lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants