From f1eca36019836593a75d9162e6e392a2f9c9884f Mon Sep 17 00:00:00 2001 From: Doug Hellmann Date: Tue, 27 Mar 2018 15:17:02 -0400 Subject: [PATCH] allow project requirements to diverge at the lower bounds Replace the exact match check for requirements with one that ensures that the exclusion set specified is a subset of any exclusions used in the global requirements list. This allows the lower bounds in a project to diverge from the value used in other projects (and the global list) while still ensuring that the allowed set of values is the same. We treat a cap ( --- openstack_requirements/check.py | 35 +++++++--- openstack_requirements/tests/test_check.py | 80 ++++++++++++++++++++-- 2 files changed, 102 insertions(+), 13 deletions(-) diff --git a/openstack_requirements/check.py b/openstack_requirements/check.py index 9d32a675fc..6e77663ff2 100644 --- a/openstack_requirements/check.py +++ b/openstack_requirements/check.py @@ -72,17 +72,37 @@ class RequirementsList(object): self.reqs_by_file[name] = self.extract_reqs(content, strict) +def _get_exclusions(req): + return set( + spec + for spec in req.specifiers.split(',') + if '!=' in spec or '<' in spec + ) + + def _is_requirement_in_global_reqs(req, global_reqs): - # Compare all fields except the extras field as the global - # requirements should not have any lines with the extras syntax - # example: oslo.db[xyz]<1.2.3 + req_exclusions = _get_exclusions(req) for req2 in global_reqs: + if (req.package == req2.package and req.location == req2.location and - req.specifiers == req2.specifiers and req.markers == req2.markers and req.comment == req2.comment): - return True + # This matches the right package and other properties, so + # ensure that any exclusions are a subset of the global + # set. + global_exclusions = _get_exclusions(req2) + if req_exclusions.issubset(global_exclusions): + return True + else: + print( + "Requirement for package {} " + "has an exclusion not found in the " + "global list: {} vs. {}".format( + req.package, req_exclusions, global_exclusions) + ) + return False + return False @@ -103,7 +123,7 @@ def get_global_reqs(content): def _validate_one(name, reqs, branch_reqs, blacklist, global_reqs): "Returns True if there is a failure." - print(name, reqs, branch_reqs, blacklist, global_reqs) + print('_validate_one', name, reqs, branch_reqs, blacklist, global_reqs) if (name in branch_reqs.reqs and reqs == branch_reqs.reqs[name]): # Unchanged [or a change that preserves a current value] @@ -117,8 +137,6 @@ def _validate_one(name, reqs, branch_reqs, blacklist, global_reqs): print("Requirement %s not in openstack/requirements" % str(reqs)) return True - if reqs == global_reqs[name]: - return False counts = {} for req in reqs: if req.extras: @@ -141,6 +159,7 @@ def _validate_one(name, reqs, branch_reqs, blacklist, global_reqs): ('[%s]' % extra) if extra else '', len(global_reqs[name]))) return True + return False def validate(head_reqs, branch_reqs, blacklist, global_reqs): diff --git a/openstack_requirements/tests/test_check.py b/openstack_requirements/tests/test_check.py index fe912f5796..fdbaf0f4e6 100644 --- a/openstack_requirements/tests/test_check.py +++ b/openstack_requirements/tests/test_check.py @@ -66,7 +66,7 @@ class TestIsReqInGlobalReqs(testtools.TestCase): def test_min_mismatch(self): req = requirement.parse('name>=1.3,!=1.4')['name'][0][0] - self.assertFalse( + self.assertTrue( check._is_requirement_in_global_reqs( req, self.global_reqs['name'], @@ -84,7 +84,7 @@ class TestIsReqInGlobalReqs(testtools.TestCase): def test_missing_exclusion(self): req = requirement.parse('name>=1.2')['name'][0][0] - self.assertFalse( + self.assertTrue( check._is_requirement_in_global_reqs( req, self.global_reqs['name'], @@ -92,6 +92,30 @@ class TestIsReqInGlobalReqs(testtools.TestCase): ) +class TestGetExclusions(testtools.TestCase): + + def test_none(self): + req = list(check.get_global_reqs('name>=1.2')['name'])[0] + self.assertEqual( + set(), + check._get_exclusions(req), + ) + + def test_one(self): + req = list(check.get_global_reqs('name>=1.2,!=1.4')['name'])[0] + self.assertEqual( + set(['!=1.4']), + check._get_exclusions(req), + ) + + def test_cap(self): + req = list(check.get_global_reqs('name>=1.2,!=1.4,<2.0')['name'])[0] + self.assertEqual( + set(['!=1.4', '<2.0']), + check._get_exclusions(req), + ) + + class TestValidateOne(testtools.TestCase): def setUp(self): @@ -212,9 +236,32 @@ class TestValidateOne(testtools.TestCase): ) ) - def test_new_item_mismatches_global_list(self): - # If the new item does not match the global value, that is an - # error. + def test_new_item_lower_min(self): + # If the new item has a lower minimum value than the global + # list, that is OK. + reqs = [ + r + for r, line in requirement.parse('name>=1.1,!=1.4')['name'] + ] + branch_reqs = check.RequirementsList( + 'testproj', + {'requirements': {'requirements.txt': ''}}, + ) + branch_reqs.process(False) + global_reqs = check.get_global_reqs('name>=1.2,!=1.4') + self.assertFalse( + check._validate_one( + 'name', + reqs=reqs, + branch_reqs=branch_reqs, + blacklist=requirement.parse(''), + global_reqs=global_reqs, + ) + ) + + def test_new_item_extra_exclusion(self): + # If the new item includes an exclusion that is not present in + # the global list that is not OK. reqs = [ r for r, line in requirement.parse('name>=1.2,!=1.4,!=1.5')['name'] @@ -235,6 +282,29 @@ class TestValidateOne(testtools.TestCase): ) ) + def test_new_item_missing_exclusion(self): + # If the new item does not include an exclusion that is + # present in the global list that is OK. + reqs = [ + r + for r, line in requirement.parse('name>=1.2')['name'] + ] + branch_reqs = check.RequirementsList( + 'testproj', + {'requirements': {'requirements.txt': ''}}, + ) + branch_reqs.process(False) + global_reqs = check.get_global_reqs('name>=1.2,!=1.4') + self.assertFalse( + check._validate_one( + 'name', + reqs=reqs, + branch_reqs=branch_reqs, + blacklist=requirement.parse(''), + global_reqs=global_reqs, + ) + ) + def test_new_item_matches_global_list_with_extra(self): # If the global list has multiple entries for an item with # different "extra" specifiers, the values must all be in the