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.

Sign up for GitHub

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

Jump to bottom

ci: test against modern python versions and fix broken test suite #348

Merged
merged 4 commits into from Nov 2, 2022
Merged

ci: test against modern python versions and fix broken test suite #348

merged 4 commits into from Nov 2, 2022

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented Nov 1, 2022 •
edited

I figure the most important versions to test are the oldest and newest. But, it can help to know if something is just too old or just too new to function as well.

With this PR I update the CI system to run against py36, 37, 310, and 311, instead of py36, 37, 38, 39.

@consideRatio consideRatio added the ci label Nov 1, 2022
@consideRatio consideRatio force-pushed the pr/test-against-modern-python-versions branch from d0d3199 to 9300c31 Compare November 1, 2022 23:40
@bollwyvl
Copy link
Collaborator

bollwyvl commented Nov 2, 2022

jupyter_core might not pick up jupyter_*_config.json in their previous cwd locations... https://discourse.jupyter.org/t/security-advisory-for-jupyter-core-package/16527.

--config-path might work.

bollwyvl
bollwyvl reviewed Nov 2, 2022
View reviewed changes
.github/workflows/test.yaml Outdated
@@ -37,7 +37,7 @@ jobs:
# specified below, these parameters have no meaning on their own and
# gain meaning on how job steps use them.
jupyterlab_version: [2, 3]
python: [3.6, 3.7, 3.8, 3.9]
python: ["3.6", "3.7", "3.10", "3.11"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

3.6 is EOL, probably just drop that, too. Might just need 3.7 and 3.11.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree deal! I'd like to make a dedicated PR for dropping support for python 3.6 though.

@consideRatio consideRatio changed the title ci: test against modern python versions ci: test against modern python versions and fix broken test suite Nov 2, 2022
@consideRatio consideRatio force-pushed the pr/test-against-modern-python-versions branch 2 times, most recently from 177d261 to 9f5f750 Compare November 2, 2022 00:56
@consideRatio
Copy link
Member Author

consideRatio commented Nov 2, 2022 •
edited

@bollwyvl I made a few attempts but can't understand how to pass --config flags to jupyter server via __init__.robot.

I attemted in 58440a0 and a few other similar commits. I've read in source code and documentation without understanding what I can do here.

@consideRatio consideRatio force-pushed the pr/test-against-modern-python-versions branch from 9f5f750 to 58440a0 Compare November 2, 2022 01:08
@consideRatio
Copy link
Member Author

Thank you for helping out with this @bollwyvl!! I'm quite lost and has lots to learn about .robot stuff

@consideRatio consideRatio mentioned this pull request Nov 2, 2022
Closed
@consideRatio consideRatio force-pushed the pr/test-against-modern-python-versions branch from 58440a0 to 6a16ac6 Compare November 2, 2022 02:09
@consideRatio consideRatio force-pushed the pr/test-against-modern-python-versions branch from a36feb6 to 43ee556 Compare November 2, 2022 02:19
@consideRatio
Copy link
Member Author

Wieeee! I failed to set --config but I managed to declare JUPYTER_CONFIG_DIR and indirectly do it. Thank you @bollwyvl for the correct guess on what was going wrong!!

@consideRatio consideRatio merged commit 0208c09 into jupyterhub:main Nov 2, 2022
@consideRatio
Copy link
Member Author

Merging and going for a support removal of py36 PR

@consideRatio consideRatio mentioned this pull request Nov 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bollwyvl bollwyvl bollwyvl left review comments

Assignees
No one assigned
Labels
ci
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Acceptance tests broken for unknown reason
2 participants
@consideRatio @bollwyvl