Move data validation out of the unit tests and strengthen
Having the data validation happening inside the unit tests makes understanding requirement update failures confusing because the output is hard to read and does not explain what the error actually is. Move the checks to their own command and set up a tox env to run it. A separate patch in project-config will add a new job for the repository. Address the dedent comment from https://review.openstack.org/#/c/204181/3/openstack_requirements/tests/test_requirement.py,cm Add a test to ensure that all items in global-requirements.txt are also listed in either upper-constraints.txt or blacklist.txt. Ignore a few items that we don't know how to constrain yet. Add a test to ensure that items in blacklist.txt are not in upper-constraints.txt. Change-Id: Icb717b0f36afb6ea29f50bc6935917dddf47fd4c
This commit is contained in:
Doug Hellmann
parent
a6e59980a3
commit
2dbf39295c
|
|
@ -0,0 +1,75 @@
|
|||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
"""Apply validation rules to the various requirements lists.
|
||||
|
||||
"""
|
||||
|
||||
import argparse
|
||||
|
||||
from openstack_requirements import constraints
|
||||
from openstack_requirements import requirement
|
||||
|
||||
|
||||
def read_requirements_file(filename):
|
||||
with open(filename, 'rt') as f:
|
||||
body = f.read()
|
||||
return requirement.parse(body)
|
||||
|
||||
|
||||
def main():
|
||||
parser = argparse.ArgumentParser()
|
||||
parser.add_argument(
|
||||
'global_requirements',
|
||||
default='global-requirements.txt',
|
||||
help='path to the global-requirements.txt file',
|
||||
)
|
||||
parser.add_argument(
|
||||
'upper_constraints',
|
||||
default='upper-constraints.txt',
|
||||
help='path to the upper-constraints.txt file',
|
||||
)
|
||||
parser.add_argument(
|
||||
'blacklist',
|
||||
default='blacklist.txt',
|
||||
help='path to the blacklist.txt file',
|
||||
)
|
||||
args = parser.parse_args()
|
||||
|
||||
error_count = 0
|
||||
|
||||
# Check the format of the constraints file.
|
||||
print('\nChecking %s' % args.upper_constraints)
|
||||
upper_constraints = read_requirements_file(args.upper_constraints)
|
||||
for msg in constraints.check_format(upper_constraints):
|
||||
print(msg)
|
||||
error_count += 1
|
||||
|
||||
# Check that the constraints and requirements are compatible.
|
||||
print('\nChecking %s' % args.global_requirements)
|
||||
global_reqs = read_requirements_file(args.global_requirements)
|
||||
for msg in constraints.check_compatible(global_reqs, upper_constraints):
|
||||
print(msg)
|
||||
error_count += 1
|
||||
|
||||
# Check that all of the items in the global-requirements list
|
||||
# appear in exactly one of the constraints file or the blacklist.
|
||||
print('\nChecking %s' % args.blacklist)
|
||||
blacklist = read_requirements_file(args.blacklist)
|
||||
for msg in constraints.check_blacklist_coverage(global_reqs,
|
||||
upper_constraints,
|
||||
blacklist,
|
||||
'upper-constraints.txt'):
|
||||
print(msg)
|
||||
error_count += 1
|
||||
|
||||
return 1 if error_count else 0
|
||||
|
|
@ -0,0 +1,105 @@
|
|||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
from packaging import specifiers
|
||||
|
||||
|
||||
# FIXME(dhellmann): These items were not in the constraints list but
|
||||
# should not be blacklisted. We don't know yet what versions they
|
||||
# should have, so just ignore them for a little while until we have
|
||||
# time to figure that out.
|
||||
UNCONSTRAINABLE = set([
|
||||
'argparse',
|
||||
'pip',
|
||||
'setuptools',
|
||||
'wmi',
|
||||
'pywin32',
|
||||
'', # blank lines
|
||||
])
|
||||
|
||||
|
||||
def check_blacklist_coverage(global_reqs, constraints, blacklist,
|
||||
constraints_list_name):
|
||||
"""Report any items that are not properly constrained.
|
||||
|
||||
Check that all of the items in the global-requirements list
|
||||
appear either in the constraints file or the blacklist.
|
||||
"""
|
||||
to_be_constrained = (
|
||||
set(global_reqs.keys()) - set(blacklist.keys())
|
||||
- UNCONSTRAINABLE
|
||||
)
|
||||
constrained = set(constraints.keys()) - set([''])
|
||||
unconstrained = to_be_constrained - constrained
|
||||
for u in sorted(unconstrained):
|
||||
yield ('%r appears in global-requirements.txt '
|
||||
'but not %s or blacklist.txt' % (u, constraints_list_name))
|
||||
|
||||
# Verify that the blacklist packages are not also listed in
|
||||
# the constraints file.
|
||||
dupes = constrained.intersection(set(blacklist.keys()))
|
||||
for d in dupes:
|
||||
yield ('%r appears in both blacklist.txt and upper-constraints.txt'
|
||||
% d)
|
||||
|
||||
|
||||
def check_format(parsed_constraints):
|
||||
"Apply the formatting rules to the pre-parsed constraints."
|
||||
for name, spec_list in parsed_constraints.items():
|
||||
for req, original_line in spec_list:
|
||||
if not req.specifiers.startswith('==='):
|
||||
yield ('Invalid constraint for %s does not have 3 "=": %s' %
|
||||
(name, original_line))
|
||||
|
||||
|
||||
def check_compatible(global_reqs, constraints):
|
||||
"""Check compatibility between requirements and constraints.
|
||||
|
||||
A change to global-requirements that wants to make changes
|
||||
incompatible with the current frozen constraints needs to also raise
|
||||
those constraints.
|
||||
|
||||
* Load global-requirements
|
||||
* Load upper-constraints.txt
|
||||
* Check that every version within upper-constraints.txt is either
|
||||
|
||||
A) Missing from global-requirements - its a transitive dep or
|
||||
a removed dep.
|
||||
B) Compatible with any of the versions in global-requirements.
|
||||
This is not-quite right, because we should in principle match
|
||||
markers, but that requires evaluating the markers which we
|
||||
haven't yet implemented. Being compatible with one of the
|
||||
requirements is good enough proxy to catch most cases.
|
||||
|
||||
:param global_reqs: A set of global requirements after parsing.
|
||||
:param constraints: The same from upper-constraints.txt.
|
||||
:return: A list of the error messages for constraints that failed.
|
||||
"""
|
||||
def satisfied(reqs, name, version, failures):
|
||||
if name not in reqs:
|
||||
return True
|
||||
tested = []
|
||||
for constraint, _ in reqs[name]:
|
||||
spec = specifiers.SpecifierSet(constraint.specifiers)
|
||||
if spec.contains(version):
|
||||
return True
|
||||
tested.append(constraint.specifiers)
|
||||
failures.append('Constraint for %s==%s does not match requirement %s' %
|
||||
(name, version, tested))
|
||||
return False
|
||||
failures = []
|
||||
for pkg_constraints in constraints.values():
|
||||
for constraint, _ in pkg_constraints:
|
||||
name = constraint.package
|
||||
version = constraint.specifiers[3:]
|
||||
satisfied(global_reqs, name, version, failures)
|
||||
return failures
|
||||
|
|
@ -0,0 +1,105 @@
|
|||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import testtools
|
||||
|
||||
from openstack_requirements import constraints
|
||||
from openstack_requirements import requirement
|
||||
|
||||
|
||||
class TestCheckCompatible(testtools.TestCase):
|
||||
|
||||
def test_non_requirement(self):
|
||||
global_reqs = {}
|
||||
good_constraints = requirement.parse("foo===1.2.5\n")
|
||||
self.assertEqual(
|
||||
[],
|
||||
constraints.check_compatible(global_reqs, good_constraints)
|
||||
)
|
||||
|
||||
def test_compatible(self):
|
||||
global_reqs = requirement.parse("foo>=1.2\nbar>2.0\n")
|
||||
good_constraints = requirement.parse("foo===1.2.5\n")
|
||||
self.assertEqual(
|
||||
[],
|
||||
constraints.check_compatible(global_reqs, good_constraints)
|
||||
)
|
||||
|
||||
def test_constraint_below_range(self):
|
||||
global_reqs = requirement.parse("oslo.concurrency>=2.3.0\nbar>1.0\n")
|
||||
bad_constraints = requirement.parse("oslo.concurrency===2.2.0\n")
|
||||
results = constraints.check_compatible(global_reqs, bad_constraints)
|
||||
self.assertNotEqual([], results)
|
||||
|
||||
def test_constraint_above_range(self):
|
||||
global_reqs = requirement.parse("foo>=1.2,<2.0\nbar>1.0\n")
|
||||
bad_constraints = requirement.parse("foo===2.0.1\n")
|
||||
results = constraints.check_compatible(global_reqs, bad_constraints)
|
||||
self.assertNotEqual([], results)
|
||||
|
||||
|
||||
class TestCheckFormat(testtools.TestCase):
|
||||
|
||||
def test_ok(self):
|
||||
good_constraints = requirement.parse("foo===1.2.5\n")
|
||||
self.assertEqual(
|
||||
[],
|
||||
list(constraints.check_format(good_constraints))
|
||||
)
|
||||
|
||||
def test_two_equals(self):
|
||||
bad_constraints = requirement.parse("foo==1.2.5\n")
|
||||
self.assertEqual(
|
||||
1,
|
||||
len(list(constraints.check_format(bad_constraints)))
|
||||
)
|
||||
|
||||
|
||||
class TestBlacklistCoverage(testtools.TestCase):
|
||||
|
||||
def test_constrained(self):
|
||||
global_reqs = requirement.parse("foo>=1.2\nbar>2.0\n")
|
||||
good_constraints = requirement.parse("foo===1.2.5\nbar==2.1")
|
||||
blacklist = requirement.parse('flake8\nhacking')
|
||||
self.assertEqual(
|
||||
[],
|
||||
list(constraints.check_blacklist_coverage(
|
||||
global_reqs, good_constraints, blacklist, 'test'))
|
||||
)
|
||||
|
||||
def test_blacklisted(self):
|
||||
global_reqs = requirement.parse("foo>=1.2\nbar>2.0\n")
|
||||
good_constraints = requirement.parse("foo===1.2.5\n")
|
||||
blacklist = requirement.parse('flake8\nhacking\nbar')
|
||||
self.assertEqual(
|
||||
[],
|
||||
list(constraints.check_blacklist_coverage(
|
||||
global_reqs, good_constraints, blacklist, 'test'))
|
||||
)
|
||||
|
||||
def test_both(self):
|
||||
global_reqs = requirement.parse("foo>=1.2\nbar>2.0\n")
|
||||
good_constraints = requirement.parse("foo===1.2.5\nbar>2.0")
|
||||
blacklist = requirement.parse('flake8\nhacking\nbar')
|
||||
results = list(constraints.check_blacklist_coverage(
|
||||
global_reqs, good_constraints, blacklist, 'test'))
|
||||
self.assertEqual(1, len(results))
|
||||
self.assertIn("'bar' appears in both", results[0])
|
||||
|
||||
def test_neither(self):
|
||||
global_reqs = requirement.parse("foo>=1.2\nbar>2.0\n")
|
||||
good_constraints = requirement.parse("foo===1.2.5\n")
|
||||
blacklist = requirement.parse('flake8\nhacking')
|
||||
results = list(constraints.check_blacklist_coverage(
|
||||
global_reqs, good_constraints, blacklist, 'test'))
|
||||
self.assertEqual(1, len(results))
|
||||
self.assertIn("'bar' appears in global-requirements.txt", results[0])
|
||||
|
|
@ -1,129 +0,0 @@
|
|||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import fixtures
|
||||
from packaging import specifiers
|
||||
import testtools
|
||||
|
||||
from openstack_requirements import requirement
|
||||
|
||||
|
||||
def check_compatible(global_reqs, constraints):
|
||||
"""Check compatibility between requirements and constraints.
|
||||
|
||||
A change to global-requirements that wants to make changes
|
||||
incompatible with the current frozen constraints needs to also raise
|
||||
those constraints.
|
||||
Load global-requirements
|
||||
Load upper-constraints.txt
|
||||
Check that every version within upper-constraints.txt is either
|
||||
A) Missing from global-requirements - its a transitive dep or
|
||||
a removed dep.
|
||||
B) Compatible with any of the versions in global-requirements.
|
||||
This is not-quite right, because we should in principle match
|
||||
markers, but that requires evaluating the markers which we
|
||||
haven't yet implemented. Being compatible with one of the
|
||||
requirements is good enough proxy to catch most cases.
|
||||
|
||||
:param global_reqs: A set of global requirements after parsing.
|
||||
:param constraints: The same from upper-constraints.txt.
|
||||
:return: A list of the parsed package tuples that failed.
|
||||
"""
|
||||
failures = []
|
||||
|
||||
def satisfied(reqs, name, version):
|
||||
if name not in reqs:
|
||||
return True
|
||||
tested = []
|
||||
for constraint, _ in reqs[name]:
|
||||
spec = specifiers.SpecifierSet(constraint.specifiers)
|
||||
if spec.contains(version):
|
||||
return True
|
||||
tested.append(constraint.specifiers)
|
||||
print('Constraint for %s==%s does not match %s' %
|
||||
(name, version, tested))
|
||||
return False
|
||||
for pkg_constraints in constraints.values():
|
||||
for constraint, _ in pkg_constraints:
|
||||
name = constraint.package
|
||||
version = constraint.specifiers[3:]
|
||||
if not satisfied(global_reqs, name, version):
|
||||
failures.append(constraint)
|
||||
return failures
|
||||
|
||||
|
||||
class TestRequirements(testtools.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(TestRequirements, self).setUp()
|
||||
self._stdout_fixture = fixtures.StringStream('stdout')
|
||||
self.stdout = self.useFixture(self._stdout_fixture).stream
|
||||
self.useFixture(fixtures.MonkeyPatch('sys.stdout', self.stdout))
|
||||
|
||||
def test_constraints_format(self):
|
||||
errors = 0
|
||||
constraints_content = open('upper-constraints.txt', 'rt').read()
|
||||
for n, line in enumerate(constraints_content.splitlines(), 1):
|
||||
c = requirement.parse_line(line)
|
||||
spec = c.specifiers
|
||||
if not spec.startswith('==='):
|
||||
print(
|
||||
'Invalid constraint line %d %r, does not have 3 "="' %
|
||||
(n, line)
|
||||
)
|
||||
errors += 1
|
||||
if errors:
|
||||
self.fail('Encountered errors parsing constraints.txt')
|
||||
|
||||
def test_constraints_compatible(self):
|
||||
global_req_content = open('global-requirements.txt', 'rt').read()
|
||||
constraints_content = open('upper-constraints.txt', 'rt').read()
|
||||
global_reqs = requirement.parse(global_req_content)
|
||||
constraints = requirement.parse(constraints_content)
|
||||
self.assertEqual([], check_compatible(global_reqs, constraints))
|
||||
|
||||
|
||||
class TestCheckCompatible(testtools.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(TestCheckCompatible, self).setUp()
|
||||
self._stdout_fixture = fixtures.StringStream('stdout')
|
||||
self.stdout = self.useFixture(self._stdout_fixture).stream
|
||||
self.useFixture(fixtures.MonkeyPatch('sys.stdout', self.stdout))
|
||||
|
||||
def test_non_requirement(self):
|
||||
global_reqs = {}
|
||||
good_constraints = requirement.parse("foo===1.2.5\n")
|
||||
self.assertEqual(
|
||||
[],
|
||||
check_compatible(global_reqs, good_constraints)
|
||||
)
|
||||
|
||||
def test_compatible(self):
|
||||
global_reqs = requirement.parse("foo>=1.2\nbar>2.0\n")
|
||||
good_constraints = requirement.parse("foo===1.2.5\n")
|
||||
self.assertEqual(
|
||||
[],
|
||||
check_compatible(global_reqs, good_constraints)
|
||||
)
|
||||
|
||||
def test_constraint_below_range(self):
|
||||
global_reqs = requirement.parse("oslo.concurrency>=2.3.0\nbar>1.0\n")
|
||||
bad_constraints = requirement.parse("oslo.concurrency===2.2.0\n")
|
||||
results = check_compatible(global_reqs, bad_constraints)
|
||||
self.assertNotEqual([], results)
|
||||
|
||||
def test_constraint_above_range(self):
|
||||
global_reqs = requirement.parse("foo>=1.2,<2.0\nbar>1.0\n")
|
||||
bad_constraints = requirement.parse("foo===2.0.1\n")
|
||||
results = check_compatible(global_reqs, bad_constraints)
|
||||
self.assertNotEqual([], results)
|
||||
|
|
@ -10,6 +10,8 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import textwrap
|
||||
|
||||
import pkg_resources
|
||||
import testscenarios
|
||||
import testtools
|
||||
|
|
@ -131,11 +133,11 @@ class TestToReqs(testtools.TestCase):
|
|||
list(requirement.to_reqs('file:///foo#egg=foo'))
|
||||
|
||||
def test_multiline(self):
|
||||
content = '\n'.join(
|
||||
['oslo.config>=1.11.0 # Apache-2.0',
|
||||
'oslo.concurrency>=2.3.0 # Apache-2.0',
|
||||
'oslo.context>=0.2.0 # Apache-2.0']
|
||||
)
|
||||
content = textwrap.dedent("""\
|
||||
oslo.config>=1.11.0 # Apache-2.0
|
||||
oslo.concurrency>=2.3.0 # Apache-2.0
|
||||
oslo.context>=0.2.0 # Apache-2.0
|
||||
""")
|
||||
reqs = requirement.parse(content)
|
||||
self.assertEqual(
|
||||
set(['oslo.config', 'oslo.concurrency', 'oslo.context']),
|
||||
|
|
|
|||
|
|
@ -26,3 +26,4 @@ console_scripts =
|
|||
edit-constraints = openstack_requirements.cmds.edit_constraint:main
|
||||
generate-constraints = openstack_requirements.cmds.generate:main
|
||||
update-requirements = openstack_requirements.cmds.update:main
|
||||
validate-constraints = openstack_requirements.cmds.validate:main
|
||||
|
|
|
|||
5
tox.ini
5
tox.ini
|
|
@ -1,7 +1,7 @@
|
|||
[tox]
|
||||
minversion = 1.6
|
||||
skipsdist = True
|
||||
envlist = py27,pypy,pep8
|
||||
envlist = validate,py27,pep8
|
||||
|
||||
[testenv]
|
||||
usedevelop = True
|
||||
|
|
@ -20,6 +20,9 @@ commands = update-requirements {posargs}
|
|||
[testenv:generate]
|
||||
commands = generate-constraints {posargs}
|
||||
|
||||
[testenv:validate]
|
||||
commands = validate-constraints {toxinidir}/global-requirements.txt {toxinidir}/upper-constraints.txt {toxinidir}/blacklist.txt
|
||||
|
||||
# work around until pypy vs. setuptools issue in bug 1290562 is fixed
|
||||
[testenv:pypy]
|
||||
deps = setuptools>3.4
|
||||
|
|
|
|||
Loading…
Reference in New Issue