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

Cypress e2e Test - Verify User Can Access Jupyter Launcher From DS Project Page #3553

Merged

Conversation

ConorOM1
Copy link
Contributor

@ConorOM1 ConorOM1 commented Dec 6, 2024

https://issues.redhat.com/browse/RHOAIENG-12652

Description

Migrate ODS-1877 to Cypress with additional enhancements added

How Has This Been Tested?

  • An oc login should be performed in the cluster before running the test

  • test-variables.yml should be configured properly

  • Export the path to the test-variables.yml: $ export CY_TEST_CONFIG=<path_to>/test-variables.yml

  • Tested against ODH-Nightly & RHOAI nightly

image

How to Run

After exporting the test-variables.yml we have 2 different ways:

Using the UI
Go to odh-dashboard/frontend/src/tests/cypress and run the command npx cypress open . This will open the Cypress UI where testLaunchStandaloneNotebook.cy.ts can be run.

Headless
Go to odh-dashboard/frontend/src/tests/cypress and run the command npx cypress run --spec "cypress/tests/e2e/dataScienceProjects/testLaunchStandaloneNotebook.cy.ts" --browser chrome

Test Impact:

  • This is already a test

Self checklist (all need to be checked):

  • The developer has manually tested the changes and verified that the changes work

If you have UI changes:

  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change.

After the PR is posted & before it merges:

  • The developer has tested their solution on a cluster by using the image produced by the PR to main

Copy link
Contributor

openshift-ci bot commented Dec 6, 2024

Hi @ConorOM1. Thanks for your PR.

I'm waiting for a opendatahub-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-ci openshift-ci bot added the needs-ok-to-test The openshift bot needs to label PRs from non members to avoid strain on the CI label Dec 6, 2024
@ConorOM1 ConorOM1 force-pushed the test_jupyter_project_page branch from f697aae to 87de068 Compare December 12, 2024 09:26
@manosnoam
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added lgtm and removed lgtm labels Dec 12, 2024
Copy link
Contributor

openshift-ci bot commented Dec 12, 2024

New changes are detected. LGTM label has been removed.

@manosnoam manosnoam added lgtm and removed needs-ok-to-test The openshift bot needs to label PRs from non members to avoid strain on the CI labels Dec 12, 2024
@ConorOM1 ConorOM1 force-pushed the test_jupyter_project_page branch from c98efb8 to b4c341c Compare December 12, 2024 12:38
@openshift-ci openshift-ci bot removed the lgtm label Dec 12, 2024
Copy link
Contributor

openshift-ci bot commented Dec 12, 2024

New changes are detected. LGTM label has been removed.

@ConorOM1 ConorOM1 force-pushed the test_jupyter_project_page branch from b4c341c to bb27abe Compare December 12, 2024 12:42
@manosnoam
Copy link
Contributor

/approve

Copy link
Contributor

openshift-ci bot commented Dec 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: manosnoam

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

Copy link
Contributor

@antowaddle antowaddle left a comment

Choose a reason for hiding this comment

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

Great work Conor, I've added some minor comments.

Copy link

codecov bot commented Dec 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.16%. Comparing base (793bdb7) to head (5311b8a).
Report is 28 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3553      +/-   ##
==========================================
- Coverage   85.57%   85.16%   -0.41%     
==========================================
  Files        1371     1380       +9     
  Lines       31180    31542     +362     
  Branches     8727     8818      +91     
==========================================
+ Hits        26681    26863     +182     
- Misses       4499     4679     +180     
Files with missing lines Coverage Δ
...otebookController/screens/server/ImageVersions.tsx 93.75% <ø> (ø)

... and 304 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 793bdb7...5311b8a. Read the comment docs.

@ConorOM1 ConorOM1 force-pushed the test_jupyter_project_page branch from 09ad344 to 5007cb3 Compare December 12, 2024 17:38
@antowaddle
Copy link
Contributor

@ConorOM1 thanks for the latest changes, some good improvements. Can you check please check this also by running the test in Jenkins along with the other current tests? There is a step that opens a new tab which I've seen running the tests locally physically opened a new tab, we need to ensure that there are no unintended consequences of doing this to other tests.

@ConorOM1
Copy link
Contributor Author

@ConorOM1 thanks for the latest changes, some good improvements. Can you check please check this also by running the test in Jenkins along with the other current tests? There is a step that opens a new tab which I've seen running the tests locally physically opened a new tab, we need to ensure that there are no unintended consequences of doing this to other tests.

@antowaddle tested in Jenkins, looks good, no effect on other tests, videos, screenshots etc. Here: cypress/job/dashboard-tests/143/Test_20Reports/

@antowaddle
Copy link
Contributor

/lgtm /approve

@manosnoam manosnoam added the lgtm label Dec 16, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit c5b0a95 into opendatahub-io:main Dec 16, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants