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] custom image tests and one test removal #1030

Merged
merged 2 commits into from
Nov 24, 2023

Conversation

jstourac
Copy link
Member

@jstourac jstourac commented Nov 20, 2023

This fixes custom image tests against 2.4 RC2 release and removes one test that I believe is redundant now.

There are two tests failing due to the product bug, issues tracked here:


CI: rhods-ci-pr-test/2148

Screenshot from 2023-11-22 13-20-44

@jstourac jstourac self-assigned this Nov 20, 2023
@jstourac 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 Nov 20, 2023
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Robocop found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link
Contributor

Robot Results

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

@jstourac jstourac requested review from manosnoam and lugi0 November 20, 2023 16:20
@jstourac jstourac marked this pull request as ready for review November 20, 2023 16:35
@jstourac jstourac added verified This PR has been tested with Jenkins and removed needs testing Needs to be tested in Jenkins labels Nov 20, 2023
@manosnoam
Copy link
Contributor

@jstourac can you please add screenshot of test execution, and test job number ?

@manosnoam
Copy link
Contributor

@jstourac has Test Bad Image Import become deprecated, or is it already being covered in Test Bad Image URL ?

@jstourac
Copy link
Member Author

jstourac commented Nov 20, 2023

@jstourac can you please add screenshot of test execution, and test job number ?

It's part of the description already. Shall we really include also the screenshot here?

@jstourac has Test Bad Image Import become deprecated, or is it already being covered in Test Bad Image URL ?

Well, based on the descriptions of these two:

Test Bad Image URL
    [Documentation]  Test adding an image with a bad repo URL (should fail)

and

Test Bad Image Import
    [Documentation]  Import a broken image and confirm it is disabled
    ...    in the JH spawner page

Now the functionality doesn't allow you to create custom image in both of these cases. So the latter one doesn't make sense to me anymore.

@manosnoam
Copy link
Contributor

@jstourac can you please add screenshot of test execution, and test job number ?

Looking at the build I see it did not pass:
image

Can you run again and share results ?

@jstourac
Copy link
Member Author

@manosnoam as mentioned in the PR description, re-execution won't help in these two cases. Or are you referring to something else?

@@ -55,26 +57,25 @@ Verify Custom Image Can Be Added
#Should Match ${spawner_packages} ${IMG_PACKAGES}

Spawn Notebook With Arguments image=${IMAGESTREAM_NAME} size=Small
[Teardown] Custom Image Teardown
${CLEANUP}= Set Variable True
Copy link
Contributor

Choose a reason for hiding this comment

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

The reason of using this variable is because you need all the steps in the keyword to be executed before cleaning up?

If that's the reason, you can use the tags robot:continue-on-failure or robot:recursive-continue-on-failure to force all steps being executed. There is more info here: https://robotframework.org/robotframework/latest/RobotFrameworkUserGuide.html#continue-on-failure

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason I added it here is to distinguish the behavior of the Custom Image Teardown. By default it performs also cleanup operations that are tied with the Jupyter Notebook environment. But this part isn't necessary if the test execution fails before this line. So the default value is set to false. Maybe your approach could be used for this too, but I went the way without modifying the current teardown keyword.

@manosnoam
Copy link
Contributor

@manosnoam as mentioned in the PR description, re-execution won't help in these two cases. Or are you referring to something else?

I think we should see that all the smoke tests passed, to indicate that the patch did not break anything.
Especially to see that these tests have passed:

  • Test Image From Local registry
  • Test Bad Image URL
  • Test Bad Image URL

@jstourac
Copy link
Member Author

@manosnoam well, how can I make these tests passed since these are product bugs there?

The custom image tests need to be updated since the UI has changed.
Since the behaviour of the custom image creation now is that you cannot
create an image with a wrong image location at all, this test simply
duplicates what is covered in `Test Bad Image URL` already.
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jgarciao jgarciao self-requested a review November 24, 2023 10:12
@jgarciao jgarciao merged commit 4928c21 into red-hat-data-services:master Nov 24, 2023
@jstourac jstourac deleted the fixCustomImage branch November 24, 2023 10:18
ChughShilpa pushed a commit to ChughShilpa/ods-ci that referenced this pull request Jan 2, 2024
…1030)

* [Fix] Update of custom image tests to match current UI

The custom image tests need to be updated since the UI has changed.

* Delete of the `Test Bad Image Import` from custom images suite

Since the behaviour of the custom image creation now is that you cannot
create an image with a wrong image location at all, this test simply
duplicates what is covered in `Test Bad Image URL` already.
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.

4 participants