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

Tests do not run when package is freshly installed #46

Closed
theferrit32 opened this issue May 4, 2023 · 3 comments
Closed

Tests do not run when package is freshly installed #46

theferrit32 opened this issue May 4, 2023 · 3 comments

Comments

@theferrit32
Copy link
Contributor

Can replicate with docker file:

FROM python:3.7

COPY . /app
WORKDIR /app
RUN pip install '.[dev,test]'
RUN make test

Output:

[...]
 > [5/5] RUN make test:
#7 0.343 pytest
#7 1.021 ImportError while loading conftest '/app/tests/conftest.py'.
#7 1.021 tests/conftest.py:5: in <module>
#7 1.021     import vcr
#7 1.021 /usr/local/lib/python3.7/site-packages/vcr/__init__.py:2: in <module>
#7 1.021     from .config import VCR
#7 1.021 /usr/local/lib/python3.7/site-packages/vcr/config.py:11: in <module>
#7 1.021     from .cassette import Cassette
#7 1.021 /usr/local/lib/python3.7/site-packages/vcr/cassette.py:12: in <module>
#7 1.021     from .patch import CassettePatcherBuilder
#7 1.021 /usr/local/lib/python3.7/site-packages/vcr/patch.py:41: in <module>
#7 1.021     _VerifiedHTTPSConnection = cpool.VerifiedHTTPSConnection
#7 1.021 E   AttributeError: module 'urllib3.connectionpool' has no attribute 'VerifiedHTTPSConnection'
#7 1.063 make: *** [Makefile:71: test] Error 4
------
executor failed running [/bin/sh -c make test]: exit code: 2

vcrpy version is not specified in setup.cfg
@theferrit32
Copy link
Contributor Author

vcrpy also does not specify a version of its urllib3 dependency. If I specify 1.26.15 (the last version of urllib3 1.x, from March 10, 2023), this reference to urllib3.connectionpool.VerifiedHTTPSConnection exists and the tests run.

@theferrit32
Copy link
Contributor Author

https://urllib3.readthedocs.io/en/latest/v2-migration-guide.html

@theferrit32 theferrit32 mentioned this issue May 4, 2023
Merged
@theferrit32
Copy link
Contributor Author

Closed by #49

DavidCain added a commit to DavidCain/bioutils that referenced this issue Sep 6, 2024
@DavidCain
Stop pinning urllib3 (only done for testing dep)
a2773d3 
This fixes a problem where one cannot install `bioutils` into an
environment where `urllib3` version 2 is being used. In other words,
merely installing `bioutils` forces you to use `urllib3` version 1.

There's a positive side effect too where installing `bioutils` no longer
pulls in an unneeded dependency.

Why are we pinning an unused dependency?
========================================
Previously, `vcrpy` (a test dependency) would not support urllib3
version 2. Tests would fail to run because of an attempted import of
`urllib3.connectionpool.VerifiedHTTPSConnection` (not present in v2):
biocommons#46 (comment)

To resolve the error, `urllib3` was pinned to version `1.26.*`:
biocommons#49

This definitely worked, but `urllib3` is *not* directly used by the
package, only indirectly used during testing.

Why pin a transitive test-only dependency?
==========================================
Unfortunately, the way that `urllib3` was pinned doesn't just affect CI
for this repository, but affects *any* environment in which `bioutils`
is installed. Specifically, `urllib3` will *always* be installed as a
dependency (or possibly downgraded from v2) when you `pip install bioutils`!

There are ways we could have pinned `urllib3` only in CI without
affecting package dependencies, but that's not important anymore.

Is the original problem solved?
===============================
Yes! `vcrpy` version 4.3.1 (released in May of 2023) started supporting
`urllib3` version 2!

> Support urllib3 v1 and v2. NOTE: there is an issue running urllib3 v2
> on Python older than 3.10, so this is currently blocked in the
> requirements. Hopefully we can resolve this situation in the future.
> Thanks to shifqu, hartwork, jairhenrique, pquentin, and vEpiphyte for
> your work on this.

https://vcrpy.readthedocs.io/en/latest/changelog.html
@DavidCain DavidCain mentioned this issue Sep 6, 2024
Merged
DavidCain added a commit to DavidCain/bioutils that referenced this issue Sep 27, 2024
@DavidCain
Stop pinning urllib3 (only done for testing dep)
6f3e4d4 
This fixes a problem where one cannot install `bioutils` into an
environment where `urllib3` version 2 is being used. In other words,
merely installing `bioutils` forces you to use `urllib3` version 1.

There's a positive side effect too where installing `bioutils` no longer
pulls in an unneeded dependency.

Why are we pinning an unused dependency?
========================================
Previously, `vcrpy` (a test dependency) would not support urllib3
version 2. Tests would fail to run because of an attempted import of
`urllib3.connectionpool.VerifiedHTTPSConnection` (not present in v2):
biocommons#46 (comment)

To resolve the error, `urllib3` was pinned to version `1.26.*`:
biocommons#49

This definitely worked, but `urllib3` is *not* directly used by the
package, only indirectly used during testing.

Why pin a transitive test-only dependency?
==========================================
Unfortunately, the way that `urllib3` was pinned doesn't just affect CI
for this repository, but affects *any* environment in which `bioutils`
is installed. Specifically, `urllib3` will *always* be installed as a
dependency (or possibly downgraded from v2) when you `pip install bioutils`!

There are ways we could have pinned `urllib3` only in CI without
affecting package dependencies, but that's not important anymore.

Is the original problem solved?
===============================
Yes! `vcrpy` version 4.3.1 (released in May of 2023) started supporting
`urllib3` version 2!

> Support urllib3 v1 and v2. NOTE: there is an issue running urllib3 v2
> on Python older than 3.10, so this is currently blocked in the
> requirements. Hopefully we can resolve this situation in the future.
> Thanks to shifqu, hartwork, jairhenrique, pquentin, and vEpiphyte for
> your work on this.

https://vcrpy.readthedocs.io/en/latest/changelog.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@theferrit32