-
Notifications
You must be signed in to change notification settings - Fork 570
[1.x] Fix tests #3341
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
[1.x] Fix tests #3341
Conversation
| env: | ||
| PIP_TRUSTED_HOST: "pypi.python.org pypi.org files.pythonhosted.org" |
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.
We need this so that we don't get a certificate error in Python 3.5 GitHub actions.
| boto3: TESTPATH=tests/integrations/boto3 | ||
| bottle: TESTPATH=tests/integrations/bottle | ||
| celery: TESTPATH=tests/integrations/celery | ||
| py{3.8,3.10}-celery: VIRTUALENV_PIP=23.3.2 |
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.
Newer pip fails to install some celery versions due to "invalid metadata"
antonpirker
left a comment
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.
Nice! Thanks for doing this!
szokeasaurusrex
left a comment
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.
I have some questions and suggestions, but mostly looks good. These do not need to block merging if you disagree
| env: | ||
| PIP_TRUSTED_HOST: "pypi.python.org pypi.org files.pythonhosted.org" |
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.
What is the purpose of adding this line?
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.
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| python-version: ["3.5","3.7","3.8","3.9","3.11","3.12"] |
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.
Shouldn't we keep testing any 1.x changes on these Python versions to ensure we remain compatible?
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.
This whole step should actually not be there anymore at all -- it's testing latest. Looks like I missed some latest somewhere in tox.ini. Will recheck this.
But to answer more generally -- the goal is to get rid of all the -latest jobs and instead test additional pinned versions (stuff that was latest in the past) in the -pinned group.
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.
Yep, missed some stuff: 1f1256c Fixed now.
| fail-fast: false | ||
| matrix: | ||
| python-version: ["3.7","3.8","3.11"] | ||
| python-version: ["3.7","3.8","3.11","3.12"] |
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.
I don't think we need to be testing additional Python versions we were not testing before on 1.x since we are not officially supporting it anymore
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.
The pinned and the latest tests used to be separate workflow steps before (you can see the now removed latest step above this). I've now pinned the last supported version of our GraphQL deps to whatever was "latest" at the time when our last release came out, so the tests essentially just moved to the pinned step instead. We were testing against it before as well -- just in another step.
Remove
-latesttests on the 1.x branch and try freezing the versions to what was out at the time of the last 1.x release.