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

Adding tests for juypterhub-ssh, jhub-client, and vs code #1123

Merged
merged 10 commits into from
Feb 28, 2022

Conversation

costrouc
Copy link
Member

@costrouc costrouc commented Feb 26, 2022

From my testing jupyterhub-ssh does not fully work as an expected ssh session. Currently it is hanging at times with

/opt/conda/condabin/conda
/opt/conda/condabin/conda

And then hanging and not giving a shell.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@costrouc costrouc changed the title Adding tests for juypterhub-ssh working as expected Adding tests for juypterhub-ssh, jhub-client, and vs code Feb 26, 2022
@trallard trallard added needs: review 👀 This PR is complete and ready for reviewing area: testing ✅ Testing labels Feb 28, 2022
@danlester
Copy link
Contributor

Looks good, although would be good not to have to start/stop the JupyterLab server for every test - I guess we can work on that if we add extra tests such as for Dask Gateway.

@danlester danlester merged commit ed6f577 into main Feb 28, 2022
@costrouc costrouc deleted the tests-jupyterhub-ssh branch February 28, 2022 15:46
@costrouc
Copy link
Member Author

@danlester I agree. This would make the tests faster. But I don't know how we can get around this since in the UI tests we want to test the start and stop button. So this will require that at a minimum we start and stop twice.

@danlester
Copy link
Contributor

Yes, this was one reason I built the Python tests into Cypress initially - click to Start, then run the tests, then stop!

But as long as multiple notebooks can be done in one session, maybe that's OK.

@costrouc
Copy link
Member Author

costrouc commented Feb 28, 2022

Ah that makes perfect sense. The pytest test_deployments will also need to launch servers but this was added with this PR as well. Do agree that starting/stoping costs around 15-30 seconds in the CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: testing ✅ Testing needs: review 👀 This PR is complete and ready for reviewing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants