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

Fix Dashboard Tests for ODH and RHOAI 2.9 #1308

Merged
merged 11 commits into from
Mar 24, 2024

Conversation

manosnoam
Copy link
Contributor

@manosnoam manosnoam commented Mar 19, 2024

Including fixing dependent KW and tests:

  • Fix Verify RHODS Display Name and Version for ODH
  • Set $DASHBOARD_APP_NAME according to RHODS / ODH
  • Fix Verify Favorite Resource Cards test
  • Fix Sort Resources By KW
  • Fix Verify Jupyter Card CSS Style KW
  • Fix Upload Serving Runtime Template in ODHDashboard Settings
  • Update DEFAULT_NOTEBOOK_VER=2024.1 and PREVIOUS_NOTEBOOK_VER=2023.2
  • Search Pod KW returns a list of pods that match regular expression
  • Verify Jupiter Service Is Not Enabled should not wait long (app isn't present)
  • Remove Test Billing Metric test (5+ minutes) from Smoke

image

image

@manosnoam manosnoam added the do not merge Do not merge this yet please label Mar 19, 2024
@@ -133,7 +133,7 @@
... None
... Returns:
... dashboard_pods_info(list(dict)): Dashboard pods selected by label and namespace
@{dashboard_pods_info} = Oc Get kind=Pod api_version=v1 namespace=${APPLICATIONS_NAMESPACE} label_selector=app=rhods-dashboard
@{dashboard_pods_info} = Oc Get kind=Pod api_version=v1 namespace=${APPLICATIONS_NAMESPACE} label_selector=app=${DASHBOARD_APP_NAME}

Check warning

Code scanning / Robocop

The assignment sign is not consistent within the file. Expected '{{ expected_sign }}' but got '{{ actual_sign }}' instead Warning test

The assignment sign is not consistent within the file. Expected '=' but got ' =' instead
@@ -750,7 +744,7 @@

Get Dashboard Pods Names
[Documentation] Retrieves the names of dashboard pods
${dash_pods}= Oc Get kind=Pod namespace=${APPLICATIONS_NAMESPACE} label_selector=app=rhods-dashboard
${dash_pods}= Oc Get kind=Pod namespace=${APPLICATIONS_NAMESPACE} label_selector=app=${DASHBOARD_APP_NAME}

Check warning

Code scanning / Robocop

Line is too long ({{ line_length }}/{{ allowed_length }}) Warning test

Line is too long (123/120)
@@ -761,7 +755,7 @@
Get Dashboard Pod Logs
[Documentation] Fetches the logs from one dashboard pod
[Arguments] ${pod_name}
${pod_logs}= Oc Get Pod Logs name=${pod_name} namespace=${APPLICATIONS_NAMESPACE} container=rhods-dashboard
${pod_logs}= Oc Get Pod Logs name=${pod_name} namespace=${APPLICATIONS_NAMESPACE} container=${DASHBOARD_APP_NAME}

Check warning

Code scanning / Robocop

Line is too long ({{ line_length }}/{{ allowed_length }}) Warning test

Line is too long (123/120)
@@ -73,6 +73,7 @@
${RHODS_VERSION}= Get RHODS Version ${force_fetch}
Set Global Variable ${RHODS_VERSION}
Set Prometheus Variables
Set Global Variable ${DASHBOARD_APP_NAME} ${PRODUCT.lower()}-dashboard

Check notice

Code scanning / Robocop

{{ set_variable_keyword }} can be replaced with VAR Note test

Set Global Variable can be replaced with VAR
Sort Resources By name
${list_of_tile_ids} = Get List Of Ids Of Tiles
Verify Star Icons Are Clickable ${list_of_tile_ids}

${favorite_ids} = Get Slice From List ${list_of_tile_ids} ${27} ${48}
${favorite_ids} = Get Slice From List ${list_of_tile_ids} ${2} ${7}

Check warning

Code scanning / Robocop

The assignment sign is not consistent within the file. Expected '{{ expected_sign }}' but got '{{ actual_sign }}' instead Warning test

The assignment sign is not consistent within the file. Expected '=' but got ' =' instead
@@ -299,7 +311,7 @@
[Tags] Sanity
... Tier1
... ODS-374
${pod_names} Get POD Names ${APPLICATIONS_NAMESPACE} app=rhods-dashboard
${pod_names} Get POD Names ${APPLICATIONS_NAMESPACE} app=${DASHBOARD_APP_NAME}

Check warning

Code scanning / Robocop

The assignment sign is not consistent within the file. Expected '{{ expected_sign }}' but got '{{ actual_sign }}' instead Warning test

The assignment sign is not consistent within the file. Expected '=' but got '' instead
Should Be Equal ${not_clicked} odh-dashboard__favorite
Click Element //*[@id="${id}"]/div[1]/span
${card_star_button}= Set Variable //*[@id="${id}" and contains(@class, "odh-tourable-card")]//button
${not_clicked} = Get Element Attribute ${card_star_button} aria-label

Check warning

Code scanning / Robocop

The assignment sign is not consistent within the file. Expected '{{ expected_sign }}' but got '{{ actual_sign }}' instead Warning test

The assignment sign is not consistent within the file. Expected '=' but got ' =' instead
Should Be Equal ${clicked} odh-dashboard__favorite m-is-favorite
Click Element //*[@id="${id}"]/div[1]/span
${card_star_button}= Set Variable //*[@id="${id}" and contains(@class, "odh-tourable-card")]//button
${clicked} = Get Element Attribute ${card_star_button} aria-label

Check warning

Code scanning / Robocop

The assignment sign is not consistent within the file. Expected '{{ expected_sign }}' but got '{{ actual_sign }}' instead Warning test

The assignment sign is not consistent within the file. Expected '=' but got ' =' instead
@@ -480,18 +494,15 @@
Close Browser

RHODS Dahsboard Pod Should Contain OauthProxy Container
${list_of_pods} = Search Pod namespace=${APPLICATIONS_NAMESPACE} pod_start_with=rhods-dashboard
${list_of_pods} = Search Pod namespace=${APPLICATIONS_NAMESPACE} pod_regex=${DASHBOARD_APP_NAME}

Check warning

Code scanning / Robocop

The assignment sign is not consistent within the file. Expected '{{ expected_sign }}' but got '{{ actual_sign }}' instead Warning test

The assignment sign is not consistent within the file. Expected '=' but got ' =' instead
Copy link
Contributor

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
448 0 0 448 100

@@ -67,6 +67,7 @@
Set Global Variable ${OPERATOR_NAMESPACE} openshift-operators
Set Global Variable ${NOTEBOOKS_NAMESPACE} opendatahub
END
Set Global Variable ${DASHBOARD_APP_NAME} ${PRODUCT.lower()}-dashboard

Check notice

Code scanning / Robocop

{{ set_variable_keyword }} can be replaced with VAR Note

Set Global Variable can be replaced with VAR
@manosnoam manosnoam added enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) and removed do not merge Do not merge this yet please labels Mar 21, 2024
@manosnoam manosnoam added the verified This PR has been tested with Jenkins label Mar 21, 2024
kornys
kornys previously requested changes Mar 21, 2024
Copy link
Contributor

@kornys kornys left a comment

Choose a reason for hiding this comment

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

comments

~/.kube/config Outdated Show resolved Hide resolved
@jiridanek jiridanek modified the milestones: 2.10.0, 2.8.1 Mar 21, 2024
@manosnoam manosnoam force-pushed the odh_dash400 branch 2 times, most recently from 10ec841 to 5d10308 Compare March 21, 2024 14:01
@@ -49,7 +49,7 @@
Maybe Skip Tour
[Documentation] If we are in the openshift web console, maybe skip the first time
... tour popup given to users, otherwise RETURN.
${should_cont} = Is Current Domain Equal To https://console-openshift-console
${should_cont} = Does Current Sub Domain Start With https://console-openshift-console

Check warning

Code scanning / Robocop

The assignment sign is not consistent within the file. Expected '{{ expected_sign }}' but got '{{ actual_sign }}' instead Warning test

The assignment sign is not consistent within the file. Expected '=' but got ' =' instead
jstourac
jstourac previously approved these changes Mar 21, 2024
bdattoma
bdattoma previously approved these changes Mar 22, 2024
Includes:
- Change `Is Current Domain Equal To` KW to support either
"oauth-openshift" or "oauth" Subdomain.
- Add `Wait For Dashboard Page Title` KW to get page title that could
reside inside h1 child element.
- `Search Pod` KW returns a list of pods match regular expression.
(It is required when installing ODH, and verifying Dashboard pods
that includes "dashboard", instead of "rhods-dashboard").
- Setting global var $DASHBOARD_APP_NAME according to product, for those
dependent KWs that uses app = "odh-dashboard" or "rhods-dashboard".

Signed-off-by: manosnoam <[email protected]>
- Fix `Upload Serving Runtime Template` in ODHDashboard Settings
- Verify that the color of the Jupyter Card code block is gray

Signed-off-by: manosnoam <[email protected]>

Verify that the color of the Jupyter Card code block is gray

Signed-off-by: manosnoam <[email protected]>
jstourac
jstourac previously approved these changes Mar 22, 2024
Also fix Camel-Case of `Wait for RHODS Dashboard to Load`

Signed-off-by: manosnoam <[email protected]>
Copy link

sonarcloud bot commented Mar 22, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@@ -97,21 +98,25 @@
Click Element xpath://a[.="Log out"]
Wait Until Page Contains Log in with OpenShift

Wait for RHODS Dashboard to Load
Wait For RHODS Dashboard To Load

Check warning

Code scanning / Robocop

Missing documentation in '{{ name }}' keyword Warning test

Missing documentation in 'Wait For RHODS Dashboard To Load' keyword
[Documentation] Wait until the visible title (h1) of the current Dashboard page is '${page_title}'
[Arguments] ${page_title} ${timeout}=10s
${page_title_element}= Set Variable //*[@data-testid="app-page-title"]
Wait Until Element is Visible ${page_title_element} timeout=${timeout}

Check warning

Code scanning / Robocop

Keyword name '{{ keyword_name }}' does not follow case convention Warning test

Keyword name 'Wait Until Element is Visible' does not follow case convention
Execute Javascript arguments[0].click(); ARGUMENTS ${element}
${status}= Run Keyword and Return Status Wait Until Page Contains Element xpath://div[contains(@class,'pf-v5-c-drawer__panel-main')]
Click Element ${card_locator}
${status}= Run Keyword and Return Status Wait Until Page Contains Element xpath://div[contains(@class,'pf-v5-c-drawer__panel-main')]

Check warning

Code scanning / Robocop

Line is too long ({{ line_length }}/{{ allowed_length }}) Warning test

Line is too long (145/120)
Execute Javascript arguments[0].click(); ARGUMENTS ${element}
${status}= Run Keyword and Return Status Wait Until Page Contains Element xpath://div[contains(@class,'pf-v5-c-drawer__panel-main')]
Click Element ${card_locator}
${status}= Run Keyword and Return Status Wait Until Page Contains Element xpath://div[contains(@class,'pf-v5-c-drawer__panel-main')]

Check warning

Code scanning / Robocop

{{ bad_indent_msg }} Warning test

Line is over-indented
Execute Javascript arguments[0].click(); ARGUMENTS ${element}
${status}= Run Keyword and Return Status Wait Until Page Contains Element xpath://div[contains(@class,'pf-v5-c-drawer__panel-main')]
Click Element ${card_locator}
${status}= Run Keyword and Return Status Wait Until Page Contains Element xpath://div[contains(@class,'pf-v5-c-drawer__panel-main')]

Check warning

Code scanning / Robocop

Keyword name '{{ keyword_name }}' does not follow case convention Warning test

Keyword name 'Run Keyword and Return Status' does not follow case convention
@manosnoam
Copy link
Contributor Author

we should merge this with squashing the commits, unless you update this discrepancy in the commit history properly (which I would prefer, though).

Thanks @jstourac , I squashed the commits.

@jstourac
Copy link
Member

jstourac commented Mar 22, 2024

Thanks @jstourac , I squashed the commits.

JFTR - the commits aren't squashed (and I like it since they are well structured and thus can be merged as they are from my point of view). What was squashed/fixed-up was the change discrepancy - inadvertent change in first commit was fixed in a different commit - now this has been removed from the original commit directly and thus makes the commit history clean. Thank you.

@jiridanek jiridanek dismissed kornys’s stale review March 22, 2024 15:11

change has been made

@bdattoma
Copy link
Contributor

Thanks @jstourac , I squashed the commits.

JFTR - the commits aren't squashed (and I like it since they are well structured and thus can be merged as they are from my point of view). What was squashed/fixed-up was the change discrepancy - inadvertent change in first commit was fixed in a different commit - now this has been removed from the original commit directly and thus makes the commit history clean. Thank you.

I'd vote for squashing anyway, it makes the overall history cleaner, if everybody starts merging new 10 commits or more it becomes difficult.
Moreover, I believe that when PR are so big, you cannot just revert a commit without the risk of missing dependent code. Hence if there is a regression introduced by a such PR, it would be easier to revert the single squashed commit and raise the PR again with the fix

@jiridanek
Copy link
Member

I guess I wouldn't squash. The individual commits have meaningful commit messages, so it would help with git blames and later investigations to keep them separate.

@bdattoma
Copy link
Contributor

I guess I wouldn't squash. The individual commits have meaningful commit messages, so it would help with git blames and later investigations to keep them separate.

in case of regression it can become a hunt for the commits you need to revert. Even squashing you can see who edited a line with git blame. What's the problem?

@jiridanek
Copy link
Member

GitHub webui gives you option to revert entire PR. And I guess I now see yet another good use of merging with merge commits.

In git blame, I like to be able to go to commit message and see whats the message associated with each change. Usually it's obvious but sometimes it isn't and then it's useful info to have.

@manosnoam
Copy link
Contributor Author

I will squash the PR, since we can still see all the commits within it.
Take a look at another squashed commit: 8910a4d - You can click on the PR #1235 link in it and see all the squashed commits it includes.

@manosnoam manosnoam merged commit a46b289 into red-hat-data-services:master Mar 24, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) verified This PR has been tested with Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants