PR#2803: unify warn messages

Merges #2803
https://pagure.io/koji/pull-request/2803

Fixes: #2795
https://pagure.io/koji/issue/2795
CLI: unify warning messages to stderr
This commit is contained in:
Tomas Kopecek 2021-04-19 15:14:35 +02:00
commit 1066f8f1ad
6 changed files with 94 additions and 63 deletions

View file

@ -1222,9 +1222,9 @@ def handle_import(goptions, session, args):
if prev['payloadhash'] == koji.hex_string(data['sigmd5']):
print(_("RPM already imported: %s") % path)
else:
print(_("WARNING: md5sum mismatch for %s") % path)
print(_(" A different rpm with the same name has already been imported"))
print(_(" Existing sigmd5 is %r, your import has %r") % (
warn("md5sum mismatch for %s" % path)
warn(" A different rpm with the same name has already been imported")
warn(" Existing sigmd5 is %r, your import has %r" % (
prev['payloadhash'], koji.hex_string(data['sigmd5'])))
print(_("Skipping import"))
return
@ -1510,9 +1510,9 @@ def handle_import_sig(goptions, session, args):
print(_("Signature already imported: %s") % path)
continue
else:
print(_("Warning: signature mismatch: %s") % path)
print(_(" The system already has a signature for this rpm with key %s") % sigkey)
print(_(" The two signature headers are not the same"))
warn("signature mismatch: %s" % path)
warn(" The system already has a signature for this rpm with key %s" % sigkey)
warn(" The two signature headers are not the same")
continue
print(_("Importing signature [key %s] from %s...") % (sigkey, path))
if not options.test:
@ -1914,7 +1914,7 @@ def handle_prune_signed_copies(goptions, session, args):
except OSError as e:
print("Error removing %s: %s" % (signedpath, e))
elif len(sigdirs) > 1:
print("Warning: more than one signature dir for %s: %r" % (sigkey, sigdirs))
warn("More than one signature dir for %s: %r" % (sigkey, sigdirs))
if build_files:
total_files += build_files
total_space += build_space
@ -4302,8 +4302,7 @@ def _print_histline(entry, **kwargs):
if event_id != other[0]:
bad_edit = "non-matching"
if bad_edit:
print("Warning: unusual edit at event %i in table %s (%s)" %
(event_id, table, bad_edit))
warn("Unusual edit at event %i in table %s (%s)" % (event_id, table, bad_edit))
# we'll simply treat them as separate events
pprint.pprint(entry)
pprint.pprint(edit)
@ -7024,7 +7023,7 @@ def anon_handle_download_logs(options, session, args):
write_fail_log(task_log_dir, task_id)
count += 1
elif state not in ['CLOSED', 'CANCELED']:
warn(_("Warning: task %s is %s\n") % (task_id, state))
warn(_("Task %s is %s\n") % (task_id, state))
for log_filename, log_volume in logs:
download_log(task_log_dir, task_id, log_filename, volume=log_volume)
@ -7206,13 +7205,12 @@ def anon_handle_wait_repo(options, session, args):
for nvr in builds:
data = session.getLatestBuilds(tag_id, package=nvr["name"])
if len(data) == 0:
print("Warning: package %s is not in tag %s" % (nvr["name"], tag))
warn("Package %s is not in tag %s" % (nvr["name"], tag))
else:
present_nvr = [x["nvr"] for x in data][0]
if present_nvr != "%s-%s-%s" % (nvr["name"], nvr["version"], nvr["release"]):
print(
"Warning: nvr %s-%s-%s is not current in tag %s\n latest build in %s is %s" %
(nvr["name"], nvr["version"], nvr["release"], tag, tag, present_nvr))
warn("nvr %s-%s-%s is not current in tag %s\n latest build in %s is %s" %
(nvr["name"], nvr["version"], nvr["release"], tag, tag, present_nvr))
last_repo = None
repo = session.getRepo(tag_id)
@ -7284,9 +7282,9 @@ def handle_regen_repo(options, session, args):
tag = info['name']
targets = session.getBuildTargets(buildTagID=info['id'])
if not targets:
print("Warning: %s is not a build tag" % tag)
warn("%s is not a build tag" % tag)
if not info['arches']:
print("Warning: tag %s has an empty arch list" % info['name'])
warn("Tag %s has an empty arch list" % info['name'])
if suboptions.debuginfo:
repo_opts['debuginfo'] = True
if suboptions.source:
@ -7397,8 +7395,10 @@ def handle_dist_repo(options, session, args):
parser.error(_('No arches given and no arches associated with tag'))
else:
for a in task_opts.arch:
if not taginfo['arches'] or a not in taginfo['arches']:
print(_('Warning: %s is not in the list of tag arches') % a)
if not taginfo['arches']:
warn('Tag %s has an empty arch list' % taginfo['name'])
elif a not in taginfo['arches']:
warn('%s is not in the list of tag arches' % a)
if task_opts.multilib:
if not os.path.exists(task_opts.multilib):
parser.error(_('could not find %s') % task_opts.multilib)

View file

@ -80,17 +80,20 @@ via 'koji edit-tag -x distrepo.cancel_others=True'
def tearDown(self):
mock.patch.stopall()
def __run_test_handle_dist_repo(self, arguments, return_value=None, expected=''):
@mock.patch('sys.stderr', new_callable=six.StringIO)
@mock.patch('sys.stdout', new_callable=six.StringIO)
def __run_test_handle_dist_repo(self, arguments, stdout, stderr, return_value=None,
expected='', expected_warn=''):
expected = expected + "Creating dist repo for tag " + self.tag_name + "\n"
with mock.patch('sys.stdout', new_callable=six.StringIO) as stdout:
rv = handle_dist_repo(self.options, self.session, arguments)
self.assertEqual(rv, return_value)
rv = handle_dist_repo(self.options, self.session, arguments)
self.assertEqual(rv, return_value)
self.assert_console_message(stdout, expected)
self.assert_console_message(stderr, expected_warn)
self.activate_session.assert_called_with(self.session, self.options)
def test_handle_dist_repo(self):
arguments = [self.tag_name, self.fake_key]
self.__run_test_handle_dist_repo(arguments, True)
self.__run_test_handle_dist_repo(arguments, return_value=True)
self.watch_tasks.assert_called_with(
self.session,
[self.task_id],
@ -99,7 +102,7 @@ via 'koji edit-tag -x distrepo.cancel_others=True'
def test_handle_dist_repo_nowait(self):
arguments = [self.tag_name, self.fake_key, '--nowait']
self.__run_test_handle_dist_repo(arguments, None)
self.__run_test_handle_dist_repo(arguments, return_value=None)
self.activate_session.assert_called_with(self.session, self.options)
self.watch_tasks.assert_not_called()
@ -177,7 +180,7 @@ via 'koji edit-tag -x distrepo.cancel_others=True'
self.session.uploadWrapper = lambda *args, **kwargs: print('uploadWrapper ...')
expected = 'uploadWrapper ...\n\n'
with mock.patch('os.path.exists', return_value=True):
self.__run_test_handle_dist_repo(arguments, True, expected)
self.__run_test_handle_dist_repo(arguments, return_value=True, expected=expected)
def test_handle_dist_repo_with_multiarch_opt(self):
arches = ['i386', 'x86_64', 'ppc', 'src']
@ -185,10 +188,21 @@ via 'koji edit-tag -x distrepo.cancel_others=True'
for a in arches:
arguments += ['--arch', a]
expected = 'Warning: %s is not in the list of tag arches' % arches[0] + "\n"
expected += 'Warning: %s is not in the list of tag arches' % arches[2] + "\n"
expected += 'Warning: %s is not in the list of tag arches' % arches[3] + "\n"
self.__run_test_handle_dist_repo(arguments, True, expected)
expected_warn = '%s is not in the list of tag arches' % arches[0] + "\n"
expected_warn += '%s is not in the list of tag arches' % arches[2] + "\n"
expected_warn += '%s is not in the list of tag arches' % arches[3] + "\n"
self.__run_test_handle_dist_repo(arguments, return_value=True, expected_warn=expected_warn)
def test_handle_dist_repo_with_arch_tag_without_arch(self):
arches = ['i386', 'x86_64']
arguments = [self.tag_name, self.fake_key]
for a in arches:
arguments += ['--arch', a]
self.session.getTag.return_value = {'arches': '', 'id': 1, 'name': self.tag_name}
expected_warn = 'Tag tag_name has an empty arch list\n'
expected_warn += 'Tag tag_name has an empty arch list\n'
self.__run_test_handle_dist_repo(arguments, return_value=True, expected_warn=expected_warn)
@mock.patch('os.path.exists')
def test_handle_dist_repo_with_multilib_opt(self, path_mock):
@ -210,7 +224,10 @@ via 'koji edit-tag -x distrepo.cancel_others=True'
path_mock.return_value = True
arches = [('x86_64', 'i686'), ('s390x', 's390'), ('ppc64', 'ppc')]
for arch in arches:
expected = self.format_error_message(
expected = ''
if arch[0] not in self.TAG['arches']:
expected += '%s is not in the list of tag arches' % arch[0] + "\n"
expected += self.format_error_message(
'The multilib arch (%s) must be included' % arch[1])
self.assert_system_exit(
handle_dist_repo,
@ -224,7 +241,7 @@ via 'koji edit-tag -x distrepo.cancel_others=True'
self.session.getTag.return_value.update({'arches': 'x86_64, i686'})
expected = 'uploadWrapper ...\n\n'
arguments += ['--arch', 'x86_64', '--arch', 'i686']
self.__run_test_handle_dist_repo(arguments, True, expected)
self.__run_test_handle_dist_repo(arguments, return_value=True, expected=expected)
def test_handle_dist_repo_with_delta_rpms_opt(self):
repos = ['test-repo1', 'test-repo2', '3']
@ -248,7 +265,7 @@ via 'koji edit-tag -x distrepo.cancel_others=True'
None,
{'id': 2, 'name': 'test-repo2'}, # state is exprired repo
]
self.__run_test_handle_dist_repo(arguments, True)
self.__run_test_handle_dist_repo(arguments, return_value=True)
def test_handle_dist_repo_help(self):
"""Test handle_dist_repo help message"""

View file

@ -129,18 +129,22 @@ class TestImport(utils.CliTestCase):
session.getRPM.reset_mock()
session.importRPM.reset_mock()
def __skip_import_test(self, options, session, arguments, **kwargs):
@mock.patch('sys.stdout', new_callable=six.StringIO)
@mock.patch('sys.stderr', new_callable=six.StringIO)
@mock.patch('koji_cli.commands.activate_session')
def __skip_import_test(self, options, session, arguments, activate_session_mock,
stderr, stdout, **kwargs):
expected = kwargs.get('expected', None)
expected_warn = kwargs.get('expected_warn', None)
rpm_header = kwargs.get('rpm_header', {})
with mock.patch('koji.get_header_fields') as get_header_fields_mock:
with mock.patch('koji_cli.commands.activate_session') as activate_session_mock:
with mock.patch('sys.stdout', new_callable=six.StringIO) as stdout:
get_header_fields_mock.return_value = rpm_header
handle_import(options, session, arguments)
get_header_fields_mock.return_value = rpm_header
handle_import(options, session, arguments)
# check output message
self.assert_console_message(stdout, expected)
self.assert_console_message(stderr, expected_warn)
# check mock calls
activate_session_mock.assert_called_with(session, options)
@ -271,14 +275,14 @@ class TestImport(utils.CliTestCase):
# Case 2: build exists and status is 'COMPLETE', md5 mismatched
# reseult: import skipped
session.getRPM.return_value['payloadhash'] = false_md5
expected = "WARNING: md5sum mismatch for %s\n" % arguments[0]
expected += " A different rpm with the same name has already been imported\n"
expected += " Existing sigmd5 is %r, your import has %r\n" % (false_md5, self.md5)
expected += "Skipping import\n"
expected_warn = "md5sum mismatch for %s\n" % arguments[0]
expected_warn += " A different rpm with the same name has already been imported\n"
expected_warn += " Existing sigmd5 is %r, your import has %r\n" % (false_md5, self.md5)
expected = "Skipping import\n"
self.__skip_import_test(
options, session, arguments,
rpm_header=self.srpm_header,
expected=expected)
expected=expected, expected_warn=expected_warn)
# Case 3: build exists and status is 'COMPLETE', has external_repo_id
# reseult: import will be performed

View file

@ -70,6 +70,7 @@ class TestImportSIG(utils.CliTestCase):
%s: error: {message}
""" % (self.progname, self.progname)
@mock.patch('sys.stderr', new_callable=six.StringIO)
@mock.patch('sys.stdout', new_callable=six.StringIO)
@mock.patch('koji.rip_rpm_sighdr')
@mock.patch('koji.get_sigpacket_key_id')
@ -81,7 +82,7 @@ class TestImportSIG(utils.CliTestCase):
get_header_fields_mock,
get_sigpacket_key_id_mock,
rip_rpm_sighdr_mock,
stdout):
stdout, stderr):
"""Test handle_import_sig function"""
arguments = ['/path/to/bash', '/path/to/less', '/path/to/sed']
options = mock.MagicMock()
@ -176,9 +177,10 @@ class TestImportSIG(utils.CliTestCase):
expected += "Signature already imported: %s" % pkg + "\n"
else:
sigRpm.append([{'sighash': self.md5sum('wrong-sig-case')}])
expected += "Warning: signature mismatch: %s" % pkg + "\n"
expected += " The system already has a signature for this rpm with key %s" % fake_sigkey + "\n"
expected += " The two signature headers are not the same" + "\n"
expected_warn = "signature mismatch: %s" % pkg + "\n"
expected_warn += " The system already has a signature for this rpm with key %s" \
% fake_sigkey + "\n"
expected_warn += " The two signature headers are not the same" + "\n"
rinfo = copy.deepcopy(self.rpm_headers)
for i, data in enumerate(rinfo):
@ -194,6 +196,7 @@ class TestImportSIG(utils.CliTestCase):
handle_import_sig(options, session, arguments)
self.assert_console_message(stdout, expected)
self.assert_console_message(stderr, expected_warn)
# Case 5, --test options test
# result: everything works fine and addRPMSig/writeSignedRPM should

View file

@ -72,14 +72,17 @@ class TestRegenRepo(utils.CliTestCase):
def tearDown(self):
mock.patch.stopall()
def __run_test_handle_regen_repo(self, arguments, return_value=None, expected=''):
@mock.patch('sys.stderr', new_callable=six.StringIO)
@mock.patch('sys.stdout', new_callable=six.StringIO)
def __run_test_handle_regen_repo(self, arguments, stdout, stderr, return_value=None,
expected='', expected_warn=''):
expected += "Regenerating repo for tag: %s" % self.tag_name + "\n"
expected += "Created task: %d" % self.task_id + "\n"
expected += "Task info: %s/taskinfo?taskID=%s" % (self.options.weburl, self.task_id) + "\n"
with mock.patch('sys.stdout', new_callable=six.StringIO) as stdout:
rv = handle_regen_repo(self.options, self.session, arguments)
self.assertEqual(rv, return_value)
rv = handle_regen_repo(self.options, self.session, arguments)
self.assertEqual(rv, return_value)
self.assert_console_message(stdout, expected)
self.assert_console_message(stderr, expected_warn)
self.activate_session.assert_called_with(self.session, self.options)
def test_handle_regen_repo(self):
@ -100,16 +103,18 @@ class TestRegenRepo(utils.CliTestCase):
# show warning if tag is not a build tag
self.session.getTag.return_value = copy.copy(self.TAG)
self.session.getBuildTargets.return_value = []
expected = "Warning: %s is not a build tag" % self.tag_name + "\n"
self.__run_test_handle_regen_repo([self.tag_name], True, expected=expected)
expected_warn = "%s is not a build tag" % self.tag_name + "\n"
self.__run_test_handle_regen_repo([self.tag_name], return_value=True,
expected_warn=expected_warn)
self.resetMocks()
# show warning message if arch is empty
noarch_tag = copy.copy(self.TAG)
noarch_tag.update({'arches': ''})
self.session.getTag.return_value = noarch_tag
expected += "Warning: tag %s has an empty arch list" % noarch_tag['name'] + "\n"
self.__run_test_handle_regen_repo([self.tag_name], True, expected=expected)
expected_warn += "Tag %s has an empty arch list" % noarch_tag['name'] + "\n"
self.__run_test_handle_regen_repo([self.tag_name], return_value=True,
expected_warn=expected_warn)
def test_handle_regen_repo_with_target_opt(self):
"""Test handle_regen_repo function with --target option"""
@ -128,16 +133,17 @@ class TestRegenRepo(utils.CliTestCase):
self.resetMocks()
self.session.getBuildTarget.return_value = {'build_tag_name': self.tag_name}
self.__run_test_handle_regen_repo(arguments, True)
self.__run_test_handle_regen_repo(arguments, return_value=True)
def test_handle_regen_repo_with_other_opts(self):
"""Test handle_regen_repo function with options"""
# --nowait
self.__run_test_handle_regen_repo([self.tag_name, '--nowait'], None)
self.__run_test_handle_regen_repo([self.tag_name, '--nowait'], return_value=None)
self.resetMocks()
# --source && --debuginfo
self.__run_test_handle_regen_repo([self.tag_name, '--source', '--debuginfo'], True)
self.__run_test_handle_regen_repo([self.tag_name, '--source', '--debuginfo'],
return_value=True)
self.session.newRepo.assert_called_with(self.tag_name, **{'debuginfo': True, 'src': True})
def test_handle_regen_repo_errors(self):

View file

@ -62,7 +62,8 @@ class TestWaitRepo(utils.CliTestCase):
@mock.patch('time.time')
@mock.patch('sys.stdout', new_callable=six.StringIO)
@mock.patch('sys.stderr', new_callable=six.StringIO)
def __test_wait_repo(self, args, expected, stderr, stdout, time_mock, ret_code=0):
def __test_wait_repo(self, args, expected, stderr, stdout, time_mock, ret_code=0,
expected_warn=''):
self.options.quiet = False
time_mock.side_effect = [0, 1, 2, 3]
if ret_code:
@ -74,7 +75,7 @@ class TestWaitRepo(utils.CliTestCase):
else:
rv = anon_handle_wait_repo(self.options, self.session, args)
self.assert_console_message(stdout, expected)
self.assert_console_message(stderr, '')
self.assert_console_message(stderr, expected_warn)
self.assertIn(rv, [0, None])
@mock.patch('time.time')
@ -144,12 +145,12 @@ class TestWaitRepo(utils.CliTestCase):
self.session.getRepo.side_effect = [
{}, {}, {'id': 1, 'name': 'DEFAULT', 'create_event': 1}
]
expected = 'Warning: nvr %s is not current in tag %s\n latest build in %s is %s' % \
expected_warn = 'nvr %s is not current in tag %s\n latest build in %s is %s' % \
(builds[0], self.tag_name, self.tag_name, new_ver) + "\n"
expected += 'Warning: package sed is not in tag %s' % self.tag_name + '\n'
expected += 'Successfully waited 0:03 for %s to appear in the ' \
expected_warn += 'Package sed is not in tag %s' % self.tag_name + '\n'
expected = 'Successfully waited 0:03 for %s to appear in the ' \
'%s repo\n' % (pkgs, self.tag_name)
self.__test_wait_repo(arguments, expected)
self.__test_wait_repo(arguments, expected, expected_warn=expected_warn)
def test_anon_handle_wait_repo_with_build_timeout(self):
"""Test anon_handle_wait_repo function with --build options on timeout cases"""