[Fix] to distinguish between PVC in pending and PVC in bound status #1070

jstourac · 2023-12-09T22:15:47Z

The storage size is shown in the UI depending of the PVC status. Before the PVC is bounded, there is shown 'Max XXGi' (or other unit). After the PVC has been bound (even after it has been de-attached from a workbench)
then there is something like: 'XXGi [## ] YYGi' - so there is shown
both the capacity that is taken and total capacity of the storage; in
the middle of them, there is a graphical element to show how much the
capacity is used.

CI: rhods-ci-pr-test/2198 - pass of this test should be enough since this method is used only in one other test and that one is tagged with AutomationBug so it's irrelevant for now anyway.

ods_ci/tests/Resources/Page/ODH/ODHDashboard/ODHDataScienceProject/Storages.resource

github-actions · 2023-12-09T22:18:45Z

Robot Results

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

ods_ci/tests/Resources/Page/ODH/ODHDashboard/ODHDataScienceProject/Storages.resource

+    ${rc}  ${pvc_status_phase}=    Run And Return Rc And Output
+    ...    oc get pvc -n ${namespace} -o jsonpath='{.items[?(@.metadata.annotations.openshift\\.io/display-name=="${name}")].status.phase}'    # robocop: disable
+    Should Be Equal As Integers    ${rc}    0    An error occurred during the check of PVC ${name} .status.phase value!
+    IF    "${pvc_status_phase}" == "Pending"


lugi0

In general it looks ok to me, but I'd like to understand a bit more what the last check on the PVC does after the if/else block. Perhaps add a line or two of comments to explain the flow?

ods_ci/tests/Resources/Page/ODH/ODHDashboard/ODHDataScienceProject/Storages.resource

+        Wait Until Page Contains Element    ${storage_size_el}    timeout=20s
+        ${displayed_size}=    Get Text    ${storage_size_el}
+        Run Keyword And Continue On Failure    Should Be Equal As Strings    ${displayed_size}    Max ${size}Gi
+    ELSE IF    "${pvc_status_phase}" == "Bound"


ods_ci/tests/Resources/Page/ODH/ODHDashboard/ODHDataScienceProject/Storages.resource

jstourac · 2023-12-11T11:53:53Z

@lugi0 Updated addressing both your comments. Will re-run CI now. Please let me know whether it looks good now 🙂

lugi0

thanks @jstourac , LGTM

The storage size is shown in the UI depending of the PVC status. Before the PVC is bounded, there is shown 'Max XXGi' (or other unit). After the PVC has been bound (even after it has been de-attached from a workbench) then there is something like: 'XXGi [## ] YYGi' - so there is shown both the capacity that is taken and total capacity of the storage; in the middle of them, there is a graphical element to show how much the capacity is used.

sonarqubecloud · 2023-12-12T08:02:50Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

jstourac · 2023-12-12T08:04:17Z

I just noticed a robocop warning so I rebased and pushed this change only.

bdattoma · 2023-12-12T10:07:05Z

@jstourac LGTM! thanks for raising this one.
Only one point/question for you.

In my mind, we should be able to test all the situations (pending, success, failure) explicitly. This fix is great but we are not assuming an expected behavior in the test, while IMO we should if we care of this particular UI aspect.
Of course, if we don't care about it, we can ignore it and let the kw to handle the 3 cases as done in this PR.
The risk is that we don't capture a regression or change in the UI behavior because the kw silently handles the situation.

Wdyt? (ofc anybody who has an option about this, feel free to join the discussion)

(i'm approving the PR because it would let our tests to run smoothly for now, and I'd agree that this particular aspect of the UI doesn't have the highest priority for us)

jstourac · 2023-12-12T10:18:15Z

@bdattoma thank you for your review! Yeah, I agree that we should probably extend test to assume the PVC state there and extend to check also the Pending state at least. Let's do it in a separate PR though. Do you want me to add a Todo note to the test?

bdattoma · 2023-12-12T10:48:52Z

Do you want me to add a Todo note to the test?

up to you, to me it would also be okay just to have in our Jira backlog

jstourac · 2023-12-12T11:02:44Z

@bdattoma https://issues.redhat.com/browse/RHOAIENG-821

github-advanced-security bot found potential problems Dec 9, 2023

View reviewed changes

jstourac force-pushed the fixPvcCheck branch from 973cc18 to 316105f Compare December 9, 2023 22:40

github-advanced-security bot found potential problems Dec 9, 2023

View reviewed changes

ods_ci/tests/Resources/Page/ODH/ODHDashboard/ODHDataScienceProject/Storages.resource Fixed Show fixed Hide fixed

ods_ci/tests/Resources/Page/ODH/ODHDashboard/ODHDataScienceProject/Storages.resource Fixed Show fixed Hide fixed

jstourac force-pushed the fixPvcCheck branch 2 times, most recently from 1dc62e3 to 26b3534 Compare December 9, 2023 22:43

jstourac marked this pull request as ready for review December 9, 2023 23:18

jstourac self-assigned this Dec 10, 2023

jstourac added needs testing Needs to be tested in Jenkins enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) labels Dec 10, 2023

jstourac force-pushed the fixPvcCheck branch from 26b3534 to 796bc68 Compare December 10, 2023 12:59

github-advanced-security bot found potential problems Dec 10, 2023

View reviewed changes

jstourac added verified This PR has been tested with Jenkins and removed needs testing Needs to be tested in Jenkins labels Dec 10, 2023

jstourac requested review from diegolovison, bdattoma, jgarciao and lugi0 December 10, 2023 13:14

lugi0 previously approved these changes Dec 11, 2023

View reviewed changes

ods_ci/tests/Resources/Page/ODH/ODHDashboard/ODHDataScienceProject/Storages.resource Show resolved Hide resolved

ods_ci/tests/Resources/Page/ODH/ODHDashboard/ODHDataScienceProject/Storages.resource Show resolved Hide resolved

jstourac force-pushed the fixPvcCheck branch from 796bc68 to 7c598f0 Compare December 11, 2023 10:24

jstourac dismissed lugi0’s stale review via 9be52ad December 11, 2023 10:25

jstourac force-pushed the fixPvcCheck branch from 7c598f0 to 9be52ad Compare December 11, 2023 10:25

jstourac added needs testing Needs to be tested in Jenkins and removed verified This PR has been tested with Jenkins labels Dec 11, 2023

jstourac marked this pull request as draft December 11, 2023 10:26

github-advanced-security bot found potential problems Dec 11, 2023

View reviewed changes

ods_ci/tests/Resources/Page/ODH/ODHDashboard/ODHDataScienceProject/Storages.resource Fixed Show fixed Hide fixed

jstourac force-pushed the fixPvcCheck branch from 9be52ad to dfed425 Compare December 11, 2023 11:52

github-advanced-security bot found potential problems Dec 11, 2023

View reviewed changes

jstourac removed the needs testing Needs to be tested in Jenkins label Dec 11, 2023

jstourac added the verified This PR has been tested with Jenkins label Dec 11, 2023

jstourac marked this pull request as ready for review December 11, 2023 12:47

jstourac requested a review from lugi0 December 11, 2023 12:47

lugi0 previously approved these changes Dec 11, 2023

View reviewed changes

jstourac force-pushed the fixPvcCheck branch from dfed425 to d337f74 Compare December 12, 2023 08:01

jstourac dismissed lugi0’s stale review via cd237b3 December 12, 2023 08:02

jstourac force-pushed the fixPvcCheck branch from d337f74 to cd237b3 Compare December 12, 2023 08:02

jstourac requested a review from lugi0 December 12, 2023 08:04

bdattoma approved these changes Dec 12, 2023

View reviewed changes

jgarciao approved these changes Dec 12, 2023

View reviewed changes

jgarciao merged commit 6f9bed7 into red-hat-data-services:master Dec 12, 2023
8 checks passed

jstourac deleted the fixPvcCheck branch December 12, 2023 14:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix] to distinguish between PVC in pending and PVC in bound status #1070

[Fix] to distinguish between PVC in pending and PVC in bound status #1070

jstourac commented Dec 9, 2023 •

edited

Loading

github-actions bot commented Dec 9, 2023

lugi0 left a comment

jstourac commented Dec 11, 2023

lugi0 left a comment

sonarqubecloud bot commented Dec 12, 2023

jstourac commented Dec 12, 2023

bdattoma commented Dec 12, 2023 •

edited

Loading

jstourac commented Dec 12, 2023

bdattoma commented Dec 12, 2023

jstourac commented Dec 12, 2023

[Fix] to distinguish between PVC in pending and PVC in bound status #1070

[Fix] to distinguish between PVC in pending and PVC in bound status #1070

Conversation

jstourac commented Dec 9, 2023 • edited Loading

github-actions bot commented Dec 9, 2023

Robot Results

lugi0 left a comment

Choose a reason for hiding this comment

jstourac commented Dec 11, 2023

lugi0 left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 12, 2023

jstourac commented Dec 12, 2023

bdattoma commented Dec 12, 2023 • edited Loading

jstourac commented Dec 12, 2023

bdattoma commented Dec 12, 2023

jstourac commented Dec 12, 2023

jstourac commented Dec 9, 2023 •

edited

Loading

bdattoma commented Dec 12, 2023 •

edited

Loading