-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Revert "Python: skip flaky poetry spec with issue comment" #6191
Conversation
I think the failure here might be because the test uses python 2, but we dropped support for python 2... so the test needs updating to use python 3:
Can't drop the |
3d4576d
to
6e78926
Compare
context "with a specified Python version" do | ||
let(:pyproject_fixture_name) { "python_2.toml" } | ||
let(:lockfile_fixture_name) { "python_2.lock" } | ||
context "with a specified Python version", :slow do |
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 annotated the test with :slow
because it's it's using a version that isn't pre-installed... because of the logic in #6079 we ignore the patch version, so it'll just use whatever 3.10 version is already pre-downloaded... so it has to untar it and install it, but not actually download it.
Not sure if I should leave it at 3.10.7 as that's semi-realistic use case to ensure poetry handles it or switch to 3.10 to avoid confusing future test writers.
Theoretically we could also avoid untarring by putting it at 3.11, but reality is in the future we'll forget to bump the 3.11 when 3.12 drops, and we'll be back to untar'ing...
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 know what our threshold for :slow
is, but the untar/unzip takes about 5s. I moved a couple other tests out of :slow
with the pre-compiled python versions.
6e78926
to
06eb88b
Compare
The remaining error is surfaced by the test, but not caused by the test: It is a bug that landed in #6079, @pavera is working on a fix right now as this will impact production jobs. |
The associated ticket on Poetry (python-poetry/poetry#3010) was closed because the underlying triggers were mostly resolved in `poetry` `1.2` using the new installer. In particular, see python-poetry/poetry#3010 (comment). We flipped to the new installer in #5838, so this skip should no longer be necessary. This reverts commit ceb5700. Additionally, this flips the fixture from a Python 2 to a Python 3 fixture. We dropped support for Python 2 a while ago, but didn't notice this test needed updating because it was skipped. Another test still needs a python 2 fixture to ensure that lockfiles still pinned to python 2 throw the correct exception. So copied the existing fixture, updated it to specify a python 3 version, and then regenerated the lockfile using: ``` PYENV_VERSION=3.10.7 pyenv exec poetry lock --no-update ```
06eb88b
to
6d71200
Compare
Adding a test to ensure we don't update the pyproject.toml with a new python version, but temporarily we expand the range to allow the update to proceed.
The associated ticket on Poetry
(python-poetry/poetry#3010) was closed because the underlying triggers were mostly resolved in
poetry
1.2
using the new installer. In particular, seepython-poetry/poetry#3010 (comment).
We flipped to the new installer in
#5838, so this skip should no longer be necessary.
This reverts commit ceb5700.
Additionally, this adds a Python 3 fixture.
We dropped support for Python 2 a while ago, but didn't notice
this test needed updating because it was skipped.
Couldn't remove the old python 2 fixture because another test still
needs a python 2 fixture to ensure that old lockfiles
still pinned to python 2 throw the correct exception.
So copied the existing fixture, updated it to specify a python 3
version, and then regenerated the lockfile using: