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

Cowbird jupyter e2e test #131

Merged
merged 24 commits into from
Nov 30, 2023
Merged

Cowbird jupyter e2e test #131

merged 24 commits into from
Nov 30, 2023

Conversation

ChaamC
Copy link
Contributor

@ChaamC ChaamC commented Nov 15, 2023

Overview

Adds a new test to check the user's workspace in a JupyterLab environment that also uses Cowbird. It checks if the different expected folders are set up properly and if the user has the right permissions on the different test files. The test preparation is done with a script run by a new optional-component test-cowbird-jupyter-access found on birdhouse.

Note that test is split in 2 notebooks :

  • notebooks-auth/test_cowbird_jupyter.ipynb : this notebook is the one executed on the container started by Jenkins. His responsability is to start a JupyterLab instance using the JupyterHub API. This instance will then run the actual tests.
  • notebooks-auth/resources/user_test_cowbird_jupyter.ipynb : this notebook is the one executed on the JupyterLab container and which contains the actual tests.

Related Issue / Discussion

@ChaamC ChaamC self-assigned this Nov 15, 2023
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@tlvu
Copy link
Contributor

tlvu commented Nov 16, 2023

Nice. I always wanted a notebook test for Cowbird because I have not fully understand what it does.

Will try this next week and report back.

By the way, I assume it was tested successfully on Jenkins? No mention in the PR description about this crucial point.

@fmigneault
Copy link
Contributor

@tlvu

By the way, I assume it was tested successfully on Jenkins? No mention in the PR description about this crucial point.

It ran successfully on this commit: bird-house/birdhouse-deploy@9d715b9
(http://daccs-jenkins.crim.ca/job/DACCS-iac-birdhouse/2248/)

However, it needs to be tested again after all changes are applied and using the recently tagged https://github.com/Ouranosinc/cowbird/tree/2.2.0.

@ChaamC
Copy link
Contributor Author

ChaamC commented Nov 21, 2023

@fmigneault @tlvu
This should be ready for a new review. Fixed the different mentionned comments and some remaining bugs.
E2E tests are still running, but this specific notebook was successful : http://daccs-jenkins.crim.ca/job/PAVICS-e2e-workflow-tests/job/cowbird-jupyter-e2e-test/47/

Copy link
Contributor

@fmigneault fmigneault left a comment

Choose a reason for hiding this comment

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

All good. Thanks for this great work!

@ChaamC
Copy link
Contributor Author

ChaamC commented Nov 30, 2023

@tlvu Just to make sure, was this PR good to merge too?

@tlvu
Copy link
Contributor

tlvu commented Nov 30, 2023

@ChaamC yes good to merge for me. I had not have time to try it yet but since this is not enabled by default, it won't break our production run, so I do not have to try it first. Thanks for these tests. I am sure it will help understand cowbird better.

@ChaamC ChaamC merged commit e5e280e into master Nov 30, 2023
@ChaamC ChaamC deleted the cowbird-jupyter-e2e-test branch November 30, 2023 19:18
ChaamC pushed a commit to bird-house/birdhouse-deploy that referenced this pull request Nov 30, 2023
## Changes
- New optional-component `test-cowbird-jupyter-access` that executes a
script to set up a test user along with different
  test files. This component is used for the related e2e test from the 

[PAVICS-e2e-workflow-tests](https://github.com/Ouranosinc/PAVICS-e2e-workflow-tests)
repo.

## Fixes
- Updates incorrect WPS outputs resource name in the cowbird config.

## Additional Information

- Related to PR on `PAVICS-e2e-workflow-tests` :
[#131](Ouranosinc/PAVICS-e2e-workflow-tests#131)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants