-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
Support Pytest 8.1 #135
Support Pytest 8.1 #135
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
Yup, looking at CI it will take a bit more work, switching to draft for now - feel free to pick up @blink1073 |
I ran into several problems: |
Yes, I totally agree 💯 |
pyproject.toml
Outdated
@@ -28,7 +28,7 @@ dependencies = [ | |||
"html5lib", | |||
"nbconvert", | |||
"nbformat", | |||
"pytest >=7.0,<9", | |||
"pytest >=8.1,<9", |
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.
FYI: The changes in this PR are compatible with Pytest 7.0+ ; you don't have to bump the minimum version.
It looks like we're now compatible with Pytest 8, #136. Thanks for looking into this! |
@blink1073 I don't think so; is your CI actually running the newer pytest version? You still need the changes in this PR for pytest 8.1 compat |
Hmm, you're right @mr-c, it didn't actually install pytest 8 even though the constraint was lifted... |
Oh, wait, no, it did, I was looking at a prior run: https://github.com/jupyterlab/pytest-check-links/actions/runs/8537702936/job/23388842190 |
Ah, pytest 8.1.0 breaks without these changes , but then 8.1.0 was pulled. 8.1.1 issues warnings. Will you still merge this PR so that pytest 8.2 or 9.0 won't break? Setting a minimum version of 7.0 for Pytest is also still a good idea |
#133 alternative to #134 I guess