From 0dad94e7251779bf31bbd922bf7b51cf824bd875 Mon Sep 17 00:00:00 2001 From: Vincent Philippon Date: Thu, 30 Jul 2020 15:32:48 -0400 Subject: [PATCH] Make RequirementSummary equality and hash unsentive to version format variation `packaging.specifiers.SpecifierSet` (`ireq.specifier`) provides equality and hash definition that are based on a canonical specifier representation. Basing RequirementSummary equality and hash on those allows to easily side-step any version/specifier normalization concerns on our side. Effectively, makes equality and hash of `test_package==1.2.0` and `test_package==1.2` to be the same, as expected. --- piptools/resolver.py | 14 +++-- tests/test_resolver.py | 127 ++++++++++++++++++++++++++++++++++++++++- tox.ini | 3 +- 3 files changed, 136 insertions(+), 8 deletions(-) diff --git a/piptools/resolver.py b/piptools/resolver.py index 6e347aae5..954f751ab 100644 --- a/piptools/resolver.py +++ b/piptools/resolver.py @@ -31,17 +31,21 @@ class RequirementSummary(object): def __init__(self, ireq): self.req = ireq.req self.key = key_from_ireq(ireq) - self.extras = str(sorted(ireq.extras)) - self.specifier = str(ireq.specifier) + self.extras = frozenset(ireq.extras) + self.specifier = ireq.specifier def __eq__(self, other): - return str(self) == str(other) + return ( + self.key == other.key + and self.specifier == other.specifier + and self.extras == other.extras + ) def __hash__(self): - return hash(str(self)) + return hash((self.key, self.specifier, self.extras)) def __str__(self): - return repr([self.key, self.specifier, self.extras]) + return repr((self.key, str(self.specifier), sorted(self.extras))) def combine_install_requirements(repository, ireqs): diff --git a/tests/test_resolver.py b/tests/test_resolver.py index d725776e4..89f4352ce 100644 --- a/tests/test_resolver.py +++ b/tests/test_resolver.py @@ -1,7 +1,8 @@ import pytest +from piptools._compat import PIP_VERSION from piptools.exceptions import NoCandidateFound -from piptools.resolver import combine_install_requirements +from piptools.resolver import RequirementSummary, combine_install_requirements @pytest.mark.parametrize( @@ -316,3 +317,127 @@ def test_compile_failure_shows_provenance(resolver, from_line): lines[-1].strip() == "celery==3.1.18 (from fake-piptools-test-with-pinned-deps==0.1)" ) + + +@pytest.mark.parametrize( + ("left_hand", "right_hand", "expected"), + ( + ("test_package", "test_package", True), + ("test_package==1.2.3", "test_package==1.2.3", True), + ("test_package>=1.2.3", "test_package>=1.2.3", True), + pytest.param( + "test_package==1.2", + "test_package==1.2.0", + True, + marks=pytest.mark.skipif( + PIP_VERSION[:2] < (20, 2), reason="Required only for pip>=20.2" + ), + ), + pytest.param( + "test_package>=1.2", + "test_package>=1.2.0", + True, + marks=pytest.mark.skipif( + PIP_VERSION[:2] < (20, 2), reason="Required only for pip>=20.2" + ), + ), + ("test_package[foo,bar]==1.2", "test_package[bar,foo]==1.2", True), + ("test_package[foo,bar]>=1.2", "test_package[bar,foo]>=1.2", True), + pytest.param( + "test_package[foo,bar]==1.2", + "test_package[bar,foo]==1.2.0", + True, + marks=pytest.mark.skipif( + PIP_VERSION[:2] < (20, 2), reason="Required only for pip>=20.2" + ), + ), + pytest.param( + "test_package[foo,bar]>=1.2", + "test_package[bar,foo]>=1.2.0", + True, + marks=pytest.mark.skipif( + PIP_VERSION[:2] < (20, 2), reason="Required only for pip>=20.2" + ), + ), + ("test_package", "other_test_package", False), + ("test_package==1.2.3", "other_test_package==1.2.3", False), + ("test_package==1.2.3", "test_package==1.2.4", False), + ("test_package>=1.2.3", "test_package>=1.2.4", False), + ("test_package>=1.2.3", "test_package<=1.2.3", False), + ("test_package==1.2", "test_package==1.2.3", False), + ("test_package>=1.2", "test_package>=1.2.3", False), + ("test_package[foo]==1.2", "test_package[bar]==1.2.0", False), + ("test_package[foo]>=1.2", "test_package[bar]>=1.2.0", False), + ("test_package[foo,bar]>=1.2", "test_package[bar]>=1.2.0", False), + ("test_package[foo,bar]>=1.2", "test_package[bar,zee]>=1.2.0", False), + ), +) +def test_RequirementSummary_equality(from_line, left_hand, right_hand, expected): + """ + RequirementSummary should report proper equality. + """ + lh_summary = RequirementSummary(from_line(left_hand)) + rh_summary = RequirementSummary(from_line(right_hand)) + assert (lh_summary == rh_summary) is expected + + +@pytest.mark.parametrize( + ("left_hand", "right_hand", "expected"), + ( + ("test_package", "test_package", True), + ("test_package==1.2.3", "test_package==1.2.3", True), + ("test_package>=1.2.3", "test_package>=1.2.3", True), + pytest.param( + "test_package==1.2", + "test_package==1.2.0", + True, + marks=pytest.mark.skipif( + PIP_VERSION[:2] < (20, 2), reason="Required only for pip>=20.2" + ), + ), + pytest.param( + "test_package>=1.2", + "test_package>=1.2.0", + True, + marks=pytest.mark.skipif( + PIP_VERSION[:2] < (20, 2), reason="Required only for pip>=20.2" + ), + ), + ("test_package[foo,bar]==1.2", "test_package[bar,foo]==1.2", True), + ("test_package[foo,bar]>=1.2", "test_package[bar,foo]>=1.2", True), + pytest.param( + "test_package[foo,bar]==1.2", + "test_package[bar,foo]==1.2.0", + True, + marks=pytest.mark.skipif( + PIP_VERSION[:2] < (20, 2), reason="Required only for pip>=20.2" + ), + ), + pytest.param( + "test_package[foo,bar]>=1.2", + "test_package[bar,foo]>=1.2.0", + True, + marks=pytest.mark.skipif( + PIP_VERSION[:2] < (20, 2), reason="Required only for pip>=20.2" + ), + ), + ("test_package", "other_test_package", False), + ("test_package==1.2.3", "other_test_package==1.2.3", False), + ("test_package==1.2.3", "test_package==1.2.4", False), + ("test_package>=1.2.3", "test_package>=1.2.4", False), + ("test_package>=1.2.3", "test_package<=1.2.3", False), + ("test_package==1.2", "test_package==1.2.3", False), + ("test_package>=1.2", "test_package>=1.2.3", False), + ("test_package[foo]==1.2", "test_package[bar]==1.2.0", False), + ("test_package[foo]>=1.2", "test_package[bar]>=1.2.0", False), + ("test_package[foo,bar]>=1.2", "test_package[bar]>=1.2.0", False), + ("test_package[foo,bar]>=1.2", "test_package[bar,zee]>=1.2.0", False), + ), +) +def test_RequirementSummary_hash_equality(from_line, left_hand, right_hand, expected): + """ + RequirementSummary hash for equivalent requirements should be equal. + """ + lh_summary = RequirementSummary(from_line(left_hand)) + rh_summary = RequirementSummary(from_line(right_hand)) + assert (hash(lh_summary) == hash(rh_summary)) is expected diff --git a/tox.ini b/tox.ini index b6cf4ce75..f0439eaf7 100644 --- a/tox.ini +++ b/tox.ini @@ -15,8 +15,7 @@ deps = ; TODO: remove all 20.0 mentions after pip-20.2 being released pip20.0: pip==20.0.* pip20.1: pip==20.1.* -; TODO: change to pip==20.2.* after pip-20.2 being released - pip20.2: -e git+https://github.com/pypa/pip.git@master#egg=pip + pip20.2: pip==20.2.* setenv = piplatest: PIP=latest pipmaster: PIP=master