-
Notifications
You must be signed in to change notification settings - Fork 76
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
Add more test cases for R Studio notebook #392
Conversation
General workflow suggestion: What's the point of having a Since we care whether the tests run in CI or not, I think the most practical way to test tests is to create a PR, wait for CI to run the added tests, and then add a link to the test report from the CI. The instructions on how to run tests locally on your own machine should be in a .md file in the repository, not repeated every time on every PR. |
We need enhancements for sure! However, this section serves as a mandatory requirement for the developer to document the testing process steps. It is crucial to provide clear instructions to the reviewer, ensuring they can easily catch up and replicate the pull request on their end. |
Sure, but since what you are adding here are automated test scripts, it seems unproductive to me to repeatedly document the steps needed for their execution in every PR raised. This should be done once, in a https://github.com/opendatahub-io/notebooks/blob/main/docs/developer-guide.md or a separate document linked to it, and then it can be referenced as needed. Besides, what is important is not checking that the tests run as expected on people's laptops, but that the tests run on the CI. Reviewers should not need to manually repeat automated tests that the CI is already running. (CI is running this, I hope). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work,
one small comment
@jiridanek: changing LGTM is restricted to collaborators In response to this: 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/test-infra repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
thanks 💯
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: harshad16, jiridanek 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 |
/cherrypick 2023b |
@harshad16: new pull request created: #424 In response to this:
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/test-infra repository. |
* Update file via digest-updater-11184497402 GitHub action * Fix the runtime-image to be referenced from modh repos Signed-off-by: Harshad Reddy Nalla <[email protected]> --------- Signed-off-by: Harshad Reddy Nalla <[email protected]> Co-authored-by: GitHub Actions <github-actions[bot]@users.noreply.github.com> Co-authored-by: Harshad Reddy Nalla <[email protected]>
Description
This is an enhancement on the test framework for RStudio notebook.
The new tests include:
How Has This Been Tested?
make rstudio-c9s-python-3.9 -e IMAGE_REGISTRY=quay.io/${YOUR_QUAY_USER}/workbench-images
make deploy-c9s-rstudio-c9s-python-3.9
make validate-rstudio-image image=rstudio-c9s-python-3.9-2023b_20230129
(check to be the correct tag)make undeploy-c9s-rstudio-c9s-python-3.9
to destroy the testing podBellow are are the logs from the
make validate-rstudio-image image=rstudio-c9s-python-3.9-2023b_20230129
execution:Merge criteria: