From d2826fe25351baaebd25745f0eb6ddbbcd3561b9 Mon Sep 17 00:00:00 2001 From: Doug Hellmann Date: Wed, 23 Nov 2016 17:39:55 -0500 Subject: [PATCH] add tests for validating release rules Also fix some of the problems found by those tests. Change-Id: I18b8d2790b5afe0faf3936781de25aad2d3bbd8d Signed-off-by: Doug Hellmann --- openstack_releases/cmds/validate.py | 88 +++++---- openstack_releases/tests/test_validate.py | 222 ++++++++++++++++++++++ 2 files changed, 269 insertions(+), 41 deletions(-) diff --git a/openstack_releases/cmds/validate.py b/openstack_releases/cmds/validate.py index fb25a57ec4..20173df55d 100644 --- a/openstack_releases/cmds/validate.py +++ b/openstack_releases/cmds/validate.py @@ -236,7 +236,7 @@ def validate_releases(deliverable_info, zuul_layout, else: print('Found new version {}'.format(release['version'])) new_releases[release['version']] = release - if project['repo'] not in prev_projects: + if prev_projects and project['repo'] not in prev_projects: print('not included in previous release for %s: %s' % (prev_version, ', '.join(sorted(prev_projects)))) else: @@ -249,52 +249,58 @@ def validate_releases(deliverable_info, zuul_layout, (release['version'], e)) mk_error(msg) - # Check to see if we are re-tagging the same - # commit with a new version. - old_sha = gitutils.sha_for_tag( - workdir, - project['repo'], - prev_version, - ) - if old_sha == project['hash']: - print('Retagging the SHA with a new version') - elif not is_independent: - # Check to see if the commit for the new - # version is in the ancestors of the - # previous release, meaning it is actually - # merged into the branch. - is_ancestor = gitutils.check_ancestry( - workdir, - project['repo'], - prev_version, - project['hash'], - ) - if not is_ancestor: - if series_name == '_independent': - save = mk_warning - else: - save = mk_error - save( - '%s %s receiving %s ' - 'is not a descendant of %s' % ( - project['repo'], - project['hash'], - release['version'], - prev_version, - ) - ) + if is_independent: mk_warning('skipping descendant test for ' 'independent project, verify ' 'branch manually') + + elif not prev_version: + mk_warning('skipping descendant test for ' + 'first release, verify ' + 'branch manually') + + else: + # Check to see if we are re-tagging the same + # commit with a new version. + old_sha = gitutils.sha_for_tag( + workdir, + project['repo'], + prev_version, + ) + if old_sha == project['hash']: + # FIXME(dhellmann): This needs a test. + print('Retagging the SHA with a new version') + else: + # Check to see if the commit for the new + # version is in the ancestors of the + # previous release, meaning it is actually + # merged into the branch. + is_ancestor = gitutils.check_ancestry( + workdir, + project['repo'], + prev_version, + project['hash'], + ) + if not is_ancestor: + mk_error( + '%s %s receiving %s ' + 'is not a descendant of %s' % ( + project['repo'], + project['hash'], + release['version'], + prev_version, + ) + ) + prev_version = release['version'] prev_projects = set(p['repo'] for p in release['projects']) - # Make sure that new entries have been appended to the file. - for v, nr in new_releases.items(): - if nr != deliverable_info['releases'][-1]: - msg = ('new release %s must be listed last, ' - 'with one new release per patch' % nr['version']) - mk_error(msg) + # Make sure that new entries have been appended to the file. + for v, nr in new_releases.items(): + if nr != deliverable_info['releases'][-1]: + msg = ('new release %s must be listed last, ' + 'with one new release per patch' % nr['version']) + mk_error(msg) def validate_new_releases(deliverable_info, filename, diff --git a/openstack_releases/tests/test_validate.py b/openstack_releases/tests/test_validate.py index 94907a994e..16633324b7 100644 --- a/openstack_releases/tests/test_validate.py +++ b/openstack_releases/tests/test_validate.py @@ -14,6 +14,9 @@ from oslotest import base +import fixtures +import mock + from openstack_releases.cmds import validate @@ -307,3 +310,222 @@ class TestValidateModel(base.BaseTestCase): ) self.assertEqual(0, len(warnings)) self.assertEqual(1, len(errors)) + + +class TestValidateReleases(base.BaseTestCase): + + def setUp(self): + super(TestValidateReleases, self).setUp() + self.tmpdir = self.useFixture(fixtures.TempDir()).path + + @mock.patch('openstack_releases.project_config.require_release_jobs_for_repo') + def test_check_release_jobs(self, check_jobs): + deliverable_info = { + 'releases': [ + {'version': '1.5.0', + 'projects': [ + {'repo': 'openstack/automaton', + 'hash': 'be2885f544637e6ee6139df7dc7bf937925804dd'}, + ]} + ], + } + warnings = [] + errors = [] + validate.validate_releases( + deliverable_info, + {'validate-projects-by-name': {}}, + 'ocata', + self.tmpdir, + warnings.append, + errors.append, + ) + self.assertEqual(0, len(warnings)) + self.assertEqual(0, len(errors)) + check_jobs.assert_called_once() + + def test_invalid_hash(self): + deliverable_info = { + 'artifact-link-mode': 'none', + 'releases': [ + {'version': '0.1', + 'projects': [ + {'repo': 'openstack/automaton', + 'hash': 'this-is-not-a-hash'}, + ]} + ], + } + warnings = [] + errors = [] + validate.validate_releases( + deliverable_info, + {'validate-projects-by-name': {}}, + 'ocata', + self.tmpdir, + warnings.append, + errors.append, + ) + self.assertEqual(0, len(warnings)) + self.assertEqual(1, len(errors)) + + def test_valid_existing(self): + deliverable_info = { + 'artifact-link-mode': 'none', + 'releases': [ + {'version': '1.5.0', + 'projects': [ + {'repo': 'openstack/automaton', + 'hash': 'be2885f544637e6ee6139df7dc7bf937925804dd'}, + ]} + ], + } + warnings = [] + errors = [] + validate.validate_releases( + deliverable_info, + {'validate-projects-by-name': {}}, + 'ocata', + self.tmpdir, + warnings.append, + errors.append, + ) + self.assertEqual(0, len(warnings)) + self.assertEqual(0, len(errors)) + + def test_no_such_hash(self): + deliverable_info = { + 'artifact-link-mode': 'none', + 'releases': [ + {'version': '99.0.0', + 'projects': [ + {'repo': 'openstack/automaton', + 'hash': 'de2885f544637e6ee6139df7dc7bf937925804dd'}, + ]} + ], + } + warnings = [] + errors = [] + validate.validate_releases( + deliverable_info, + {'validate-projects-by-name': {}}, + 'ocata', + self.tmpdir, + warnings.append, + errors.append, + ) + self.assertEqual(1, len(warnings)) + self.assertEqual(1, len(errors)) + + def test_mismatch_existing(self): + deliverable_info = { + 'artifact-link-mode': 'none', + 'releases': [ + {'version': '1.5.0', + 'projects': [ + {'repo': 'openstack/automaton', + # hash from the previous release + 'hash': 'c6278ba1a8167447a5f52bdb92c2790abc5d0f87'}, + ]} + ], + } + warnings = [] + errors = [] + validate.validate_releases( + deliverable_info, + {'validate-projects-by-name': {}}, + 'ocata', + self.tmpdir, + warnings.append, + errors.append, + ) + self.assertEqual(0, len(warnings)) + self.assertEqual(1, len(errors)) + + def test_not_descendent(self): + deliverable_info = { + 'artifact-link-mode': 'none', + 'releases': [ + {'version': '1.4.0', + 'projects': [ + {'repo': 'openstack/automaton', + 'hash': 'c6278ba1a8167447a5f52bdb92c2790abc5d0f87'}, + ]}, + {'version': '1.4.999', + 'projects': [ + {'repo': 'openstack/automaton', + # a commit on stable/mitaka instead of stable/newton + 'hash': 'e92b85ec64ac74598983a90bd2f3e1cf232ba9d5'}, + ]}, + ], + } + warnings = [] + errors = [] + validate.validate_releases( + deliverable_info, + {'validate-projects-by-name': {}}, + 'ocata', + self.tmpdir, + warnings.append, + errors.append, + ) + self.assertEqual(0, len(warnings)) + self.assertEqual(1, len(errors)) + + def test_new_not_at_end(self): + deliverable_info = { + 'artifact-link-mode': 'none', + 'releases': [ + {'version': '1.3.999', + 'projects': [ + {'repo': 'openstack/automaton', + 'hash': 'e87dc55a48387d2b8b8c46e02a342c27995dacb1'}, + ]}, + {'version': '1.4.0', + 'projects': [ + {'repo': 'openstack/automaton', + 'hash': 'c6278ba1a8167447a5f52bdb92c2790abc5d0f87'}, + ]}, + ], + } + warnings = [] + errors = [] + validate.validate_releases( + deliverable_info, + {'validate-projects-by-name': {}}, + 'ocata', + self.tmpdir, + warnings.append, + errors.append, + ) + self.assertEqual(1, len(warnings)) + self.assertEqual(1, len(errors)) + + @mock.patch('openstack_releases.versionutils.validate_version') + def test_invalid_version(self, validate_version): + # Set up the nested validation function to produce an error, + # even though there is nothing else wrong with the + # inputs. That ensures we only get the 1 error back. + validate_version.configure_mock( + return_value=['an error goes here'], + ) + deliverable_info = { + 'artifact-link-mode': 'none', + 'releases': [ + {'version': '99.5.0', + 'projects': [ + {'repo': 'openstack/automaton', + 'hash': 'be2885f544637e6ee6139df7dc7bf937925804dd'}, + ]} + ], + } + warnings = [] + errors = [] + validate.validate_releases( + deliverable_info, + {'validate-projects-by-name': {}}, + 'ocata', + self.tmpdir, + warnings.append, + errors.append, + ) + self.assertEqual(1, len(warnings)) + self.assertEqual(1, len(errors))