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

Add test-pypy3.8 test-pypy3.9 test-pypy3.10 #3304

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

ecerulm
Copy link
Contributor

@ecerulm ecerulm commented Jan 22, 2024

Currently there are nox session for test-3.8, test-3.9, test-3.10, test-3.11 and test-pypy.

Then test-pypy is should in my opinion be split into test-pypy3.8, test-pypy3.9, and test-pypy3.10 since that is already done in CI. It easier to run multiple pypy locally in the laptop if those nox sessions are explicit.

With this PR is possible to run the nox sessions for all pypy versions like so

brew install pyenv 
brew install nox

pyenv install pypy3.8
pyenv install pypy3.9
pyenv install pypy3.10
pyenv install 3.8
pyenv install 3.9
pyenv install 3.10
pyenv install 3.11


#zsh
path=(
$(pyenv prefix 3.8)/bin
$(pyenv prefix 3.9)/bin
$(pyenv prefix 3.10)/bin
$(pyenv prefix 3.11)/bin
$(pyenv prefix pypy3.8)/bin
$(pyenv prefix pypy3.9)/bin
$(pyenv prefix pypy3.10)/bin
$path
)

nox -rs format lint mypy test-3.8 test-pypy3.8 test-3.9 test-pypy3.9 test-3.10 test-pypy3.10 

The rational behind this PR is to be able to run the pypy tests locally in my computer for pypy3.8, pypy3.9 and pypy3.10 without having to delete the .nox/pypy in between. Also I think for consistency with the cpython tests where there is a separate nox session for each one.

This PR has #3286 as prerequisite though.

@pquentin pquentin added the Skip Changelog Pull requests that don't require a changelog entry label Jan 23, 2024
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! I think lint is failing? You will need to run nox -rs format.

noxfile.py Show resolved Hide resolved
pquentin
pquentin previously approved these changes Jan 23, 2024
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

@pquentin
Copy link
Member

This would be interesting to understand why we have so many issues with PyPy. Any ideas?

@ecerulm
Copy link
Contributor Author

ecerulm commented Jan 23, 2024

Well, I actually noticed that test-pypy fails very often under pypy3.9 on the CI, and I wanted to investigate more, but then I realized I could not test locally pypy3.8 and pypy3.9 hence this PR.

I don't really understand yet, why the test under pypy3.9 are sometimes very very slow (it takes more than 30 minutes to get to 50% of the test suite) , I want to check if that is something that happens only on the GItHub Actions / CI or if it is something that I can replicate with pypy3.9 locally.

I just wrote an issue to track the the pypy on ci problems: #3305

@ecerulm
Copy link
Contributor Author

ecerulm commented Jan 23, 2024

This would be interesting to understand why we have so many issues with PyPy. Any ideas?

@pquentin , I created PR #3308 which I think solves (or sidesteps) the issues with pypy3.9 in particular.

@sethmlarson
Copy link
Member

@ecerulm Can you resolve the conflicts on this PR with the 3.9 one that I just merged?

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Comments on your changes to the contributing docs.

docs/contributing.rst Outdated Show resolved Hide resolved
docs/contributing.rst Outdated Show resolved Hide resolved
@pquentin pquentin merged commit 89ed0d6 into urllib3:main Jan 24, 2024
32 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog Pull requests that don't require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants