-
-
Notifications
You must be signed in to change notification settings - Fork 622
PEP 440 direct reference tweak unit tests #1453
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
Changes from 21 commits
abb07fc
210fa12
4999357
dee9ddf
4ac34f1
3ec83ce
7f8685f
e850781
d45422d
10cfb27
54ab1ea
3f50330
cad473b
6c0d539
fe10214
c75572b
d7aa047
aa50f02
b52eafe
5ec56ec
aa3049f
14ce893
4413b41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,3 +31,6 @@ coverage.xml | |
requirements.in | ||
requirements.txt | ||
.eggs/ | ||
|
||
# OS | ||
.DS_Store | ||
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,7 +2,6 @@ | |||||
import shutil | ||||||
import subprocess | ||||||
import sys | ||||||
from pathlib import Path | ||||||
from textwrap import dedent | ||||||
from unittest import mock | ||||||
|
||||||
|
@@ -513,40 +512,34 @@ def test_locally_available_editable_package_is_not_archived_in_cache_dir( | |||||
@pytest.mark.parametrize( | ||||||
("line", "dependency"), | ||||||
( | ||||||
# zip URL | ||||||
# use pip-tools version prior to its use of setuptools_scm, | ||||||
# which is incompatible with https: install | ||||||
( | ||||||
pytest.param( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||||||
"https://github.com/jazzband/pip-tools/archive/" | ||||||
"7d86c8d3ecd1faa6be11c7ddc6b29a30ffd1dae3.zip", | ||||||
"\nclick==", | ||||||
id="Zip URL", | ||||||
), | ||||||
# scm URL | ||||||
( | ||||||
pytest.param( | ||||||
"git+git://github.com/jazzband/pip-tools@" | ||||||
"7d86c8d3ecd1faa6be11c7ddc6b29a30ffd1dae3", | ||||||
"\nclick==", | ||||||
id="VCS URL", | ||||||
), | ||||||
# wheel URL | ||||||
( | ||||||
pytest.param( | ||||||
"https://files.pythonhosted.org/packages/06/96/" | ||||||
"89872db07ae70770fba97205b0737c17ef013d0d1c790" | ||||||
"899c16bb8bac419/pip_tools-3.6.1-py2.py3-none-any.whl", | ||||||
"\nclick==", | ||||||
id="Wheel URL", | ||||||
), | ||||||
( | ||||||
pytest.param( | ||||||
"pytest-django @ git+git://github.com/pytest-dev/pytest-django" | ||||||
"@21492afc88a19d4ca01cd0ac392a5325b14f95c7" | ||||||
"#egg=pytest-django", | ||||||
"git+git://github.com/pytest-dev/pytest-django" | ||||||
"@21492afc88a19d4ca01cd0ac392a5325b14f95c7#egg=pytest-django", | ||||||
), | ||||||
( | ||||||
"git+git://github.com/open-telemetry/opentelemetry-python.git" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed this test and created a local one with a subdirectory inside I did this to decrease the time it takes to run this test. |
||||||
"@v1.3.0#subdirectory=opentelemetry-api" | ||||||
"&egg=opentelemetry-api", | ||||||
"git+git://github.com/open-telemetry/opentelemetry-python.git" | ||||||
"@v1.3.0#subdirectory=opentelemetry-api", | ||||||
id="VCS with direct reference and egg", | ||||||
), | ||||||
), | ||||||
) | ||||||
|
@@ -565,25 +558,23 @@ def test_url_package(runner, line, dependency, generate_hashes): | |||||
@pytest.mark.parametrize( | ||||||
("line", "dependency", "rewritten_line"), | ||||||
( | ||||||
# file:// wheel URL | ||||||
( | ||||||
pytest.param( | ||||||
path_to_url( | ||||||
os.path.join( | ||||||
MINIMAL_WHEELS_PATH, "small_fake_with_deps-0.1-py2.py3-none-any.whl" | ||||||
) | ||||||
), | ||||||
"\nsmall-fake-a==0.1", | ||||||
None, | ||||||
id="Wheel URI", | ||||||
), | ||||||
# file:// directory | ||||||
( | ||||||
pytest.param( | ||||||
path_to_url(os.path.join(PACKAGES_PATH, "small_fake_with_deps")), | ||||||
"\nsmall-fake-a==0.1", | ||||||
None, | ||||||
id="Local project URI", | ||||||
), | ||||||
# bare path | ||||||
# will be rewritten to file:// URL | ||||||
( | ||||||
pytest.param( | ||||||
os.path.join( | ||||||
MINIMAL_WHEELS_PATH, "small_fake_with_deps-0.1-py2.py3-none-any.whl" | ||||||
), | ||||||
|
@@ -593,11 +584,38 @@ def test_url_package(runner, line, dependency, generate_hashes): | |||||
MINIMAL_WHEELS_PATH, "small_fake_with_deps-0.1-py2.py3-none-any.whl" | ||||||
) | ||||||
), | ||||||
id="Bare path to file URI", | ||||||
), | ||||||
pytest.param( | ||||||
os.path.join( | ||||||
MINIMAL_WHEELS_PATH, "small_fake_with_deps-0.1-py2.py3-none-any.whl" | ||||||
), | ||||||
"\nsmall-fake-with-deps @ " | ||||||
+ path_to_url( | ||||||
os.path.join( | ||||||
MINIMAL_WHEELS_PATH, "small_fake_with_deps-0.1-py2.py3-none-any.whl" | ||||||
) | ||||||
), | ||||||
"\nsmall-fake-with-deps @ " | ||||||
+ path_to_url( | ||||||
os.path.join( | ||||||
MINIMAL_WHEELS_PATH, "small_fake_with_deps-0.1-py2.py3-none-any.whl" | ||||||
) | ||||||
), | ||||||
id="Local project with absolute URI", | ||||||
), | ||||||
pytest.param( | ||||||
path_to_url(os.path.join(PACKAGES_PATH, "small_fake_with_subdir")) | ||||||
+ "#subdirectory=subdir&egg=small_fake_a", | ||||||
path_to_url(os.path.join(PACKAGES_PATH, "small_fake_with_subdir")) | ||||||
+ "#subdirectory=subdir&egg=small_fake_a", | ||||||
None, | ||||||
id="Local project with subdirectory", | ||||||
), | ||||||
), | ||||||
) | ||||||
@pytest.mark.parametrize("generate_hashes", ((True,), (False,))) | ||||||
def test_local_url_package( | ||||||
def test_local_file_uri_package( | ||||||
pip_conf, runner, line, dependency, rewritten_line, generate_hashes | ||||||
): | ||||||
if rewritten_line is None: | ||||||
|
@@ -612,38 +630,31 @@ def test_local_url_package( | |||||
assert dependency in out.stderr | ||||||
|
||||||
|
||||||
@pytest.mark.parametrize( | ||||||
("line", "dependency", "rewritten_line"), | ||||||
( | ||||||
pytest.param( | ||||||
os.path.join( | ||||||
MINIMAL_WHEELS_PATH, "small_fake_with_deps-0.1-py2.py3-none-any.whl" | ||||||
), | ||||||
"\nfile:small_fake_with_deps-0.1-py2.py3-none-any.whl", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is now a standard unit test called |
||||||
"file:small_fake_with_deps-0.1-py2.py3-none-any.whl", | ||||||
id="Relative URL", | ||||||
def test_relative_file_uri_package(pip_conf, runner): | ||||||
# Copy wheel into temp dir | ||||||
shutil.copy( | ||||||
os.path.join( | ||||||
MINIMAL_WHEELS_PATH, "small_fake_with_deps-0.1-py2.py3-none-any.whl" | ||||||
), | ||||||
pytest.param( | ||||||
os.path.join( | ||||||
MINIMAL_WHEELS_PATH, "small_fake_with_deps-0.1-py2.py3-none-any.whl" | ||||||
), | ||||||
"\nsmall-fake-with-deps" | ||||||
" @ file://{absolute_path}/small_fake_with_deps-0.1-py2.py3-none-any.whl", | ||||||
"small-fake-with-deps" | ||||||
" @ file://{absolute_path}/small_fake_with_deps-0.1-py2.py3-none-any.whl", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
id="Relative URL", | ||||||
), | ||||||
), | ||||||
) | ||||||
def test_relative_url_package(pip_conf, runner, line, dependency, rewritten_line): | ||||||
dependency = dependency.format(absolute_path=Path(".").absolute()) | ||||||
rewritten_line = rewritten_line.format(absolute_path=Path(".").absolute()) | ||||||
shutil.copy(line, ".") | ||||||
".", | ||||||
) | ||||||
with open("requirements.in", "w") as req_in: | ||||||
req_in.write(dependency) | ||||||
req_in.write("file:small_fake_with_deps-0.1-py2.py3-none-any.whl") | ||||||
out = runner.invoke(cli, ["-n", "--rebuild"]) | ||||||
assert out.exit_code == 0 | ||||||
assert rewritten_line in out.stderr | ||||||
assert "file:small_fake_with_deps-0.1-py2.py3-none-any.whl" in out.stderr | ||||||
|
||||||
|
||||||
def test_direct_reference_with_extras(runner): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @atugushev this is the new unit test to confirm extras with direct reference is working and we don't need to add any logic to the existing codebase (I also double-checked manually by installing the version of pip-tools that contains my changes). |
||||||
with open("requirements.in", "w") as req_in: | ||||||
req_in.write( | ||||||
"piptools[testing,coverage] @ git+https://github.com/jazzband/[email protected]" | ||||||
) | ||||||
out = runner.invoke(cli, ["-n", "--rebuild"]) | ||||||
assert out.exit_code == 0 | ||||||
assert "pip-tools @ git+https://github.com/jazzband/[email protected]" in out.stderr | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't there be extras included in the package name?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, basically at some point it takes into account the extras that is passed in the
And so it produces the
If I remove the extras from the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another example is the ouput of from:
the
not:
So the current behavior of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And from
So unless the specification was not implemented correctly in the other tools, I think we're fine? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also if we specify the extras in the Like for So if you have:
I think it would mean when it installs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case you have specified There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh sure, but I'm talking about a mixed workflow. Does pip actually store the extras you select anywhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure to understand your workflow so I'll try to answer as best I can. If I install coverage with pip it does the following:
Then if I upgrade:
By the way you can see that
Please let me know if I answered your question, otherwise could you give me more details about your workflow? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, that's what I feared. The fact that pip-compile left the extras in made me think they were important There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah I think EDIT:
But it's broken when you do
and the
The Pipfile is right though:
Well... I am seeing so many issues with |
||||||
assert "pytest==" in out.stderr | ||||||
assert "pytest-cov==" in out.stderr | ||||||
|
||||||
|
||||||
def test_input_file_without_extension(pip_conf, runner): | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,5 @@ | |
"SQLAlchemy!=0.9.5,<2.0.0,>=0.7.8,>=1.0.0", | ||
"python-memcached>=1.57,<2.0", | ||
"xmltodict<=0.11,>=0.4.6", | ||
"requests @ git+git://github.com/psf/[email protected]", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed this line since it seemed to be useless IMO. I added it when I started working on my first PR in |
||
], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
from setuptools import setup | ||
|
||
setup(name="small_fake_a", version=0.1) |
Uh oh!
There was an error while loading. Please reload this page.