Fix validation to ensure that first release of series isn't a bugfix
We recently observed that this check was skipped. Indeed it was
possible to propose a bugfix version on the first release of a series
without facing validation errors [1].
The described situation was allowed by checks that we introduced
during Wallaby [2]. Those was designed to ensure that we can
build sdist from the proposed tag. Indeed, they temporarly created
the new git tag to build sdist from it. However we never delete this
tag after our tests so this temporary tag is see as an existing tag
and so that led us to the observed bug.
Our validation check that the first version number of series isn't a
bugfix after it tried to build the sdist from the new tag, so, without
this patch the tag newly created by the sdist check remains available
in the repo, and so a couple of next checks are skipped, including the
validation of the version number, because these checks are decorated
to ignore existing tags.
Indeed the function decorated with `skip_existing_tags` detect this
temporary tag as an existing tag, so, the decorated functions are
skipped.
We should notice that it surely impacted other checks in the same manner.
Indeed, all validation functions decorated with this function
`skip_existing_tags` are impacted by this bug.
Hopefully it didn't have too much side effects.
These changes delete the temporary tag after usage to ensure to restore
the initial environment and to stop skipping next checks.
[1] https://review.opendev.org/c/openstack/releases/+/795836
[2] 80652b5232
Change-Id: I5d9024528d08487a7582e0a0e73576b22708a6cf
This commit is contained in:
parent
156b6df7b5
commit
9504e96b5d
@ -836,6 +836,15 @@ def validate_build_sdist(deliv, context):
|
||||
context.error(
|
||||
'Failed to build sdist for {}: {}'.format(
|
||||
project.repo.name, err))
|
||||
finally:
|
||||
# Removing the temporary tag to avoid to mislead next tests, else
|
||||
# if not deleted the tag will remain active in the repo and it
|
||||
# will be see as an existing tag.
|
||||
gitutils.delete_tag(
|
||||
context.workdir,
|
||||
project.repo.name,
|
||||
release.version
|
||||
)
|
||||
|
||||
|
||||
@skip_em_eol_tags
|
||||
|
@ -355,6 +355,21 @@ def add_tag(workdir, repo, tag, sha):
|
||||
return None
|
||||
|
||||
|
||||
def delete_tag(workdir, repo, tag):
|
||||
cmd = ['git', 'tag', '-d', tag]
|
||||
try:
|
||||
LOG.info(' '.join(cmd))
|
||||
return processutils.check_output(
|
||||
cmd,
|
||||
cwd=os.path.join(workdir, repo),
|
||||
stderr=subprocess.STDOUT,
|
||||
).decode('utf-8').strip()
|
||||
except processutils.CalledProcessError as e:
|
||||
LOG.warning('failed to delete tag: %s [%s]',
|
||||
e, e.output.strip())
|
||||
return None
|
||||
|
||||
|
||||
def get_branches(workdir, repo):
|
||||
try:
|
||||
output = processutils.check_output(
|
||||
|
@ -2710,6 +2710,37 @@ class TestValidateSeriesFirst(base.BaseTestCase):
|
||||
self.assertEqual(0, len(self.ctx.warnings))
|
||||
self.assertEqual(0, len(self.ctx.errors))
|
||||
|
||||
def test_version_ko(self):
|
||||
series_a_dir = self.tmpdir + '/a'
|
||||
series_a_filename = series_a_dir + '/automaton.yaml'
|
||||
os.makedirs(series_a_dir)
|
||||
deliverable_data = textwrap.dedent('''
|
||||
---
|
||||
releases:
|
||||
- version: 1.1.1
|
||||
projects:
|
||||
- repo: openstack/automaton
|
||||
hash: be2885f544637e6ee6139df7dc7bf937925804dd
|
||||
''')
|
||||
with open(series_a_filename, 'w') as f:
|
||||
f.write(deliverable_data)
|
||||
deliv = deliverable.Deliverable(
|
||||
team='team',
|
||||
series='a',
|
||||
name='name',
|
||||
data=yamlutils.loads(deliverable_data),
|
||||
)
|
||||
validate.validate_series_first(
|
||||
deliv,
|
||||
self.ctx,
|
||||
)
|
||||
self.ctx.show_summary()
|
||||
self.assertEqual(0, len(self.ctx.warnings))
|
||||
# Starting a cycle with a bugfix version (1.1.1) isn't allowed,
|
||||
# so, in this case our validation should fail accordingly to that
|
||||
# rule.
|
||||
self.assertEqual(1, len(self.ctx.errors))
|
||||
|
||||
def test_ignore_if_second_release(self):
|
||||
series_a_dir = self.tmpdir + '/a'
|
||||
series_a_filename = series_a_dir + '/automaton.yaml'
|
||||
|
Loading…
Reference in New Issue
Block a user