From 2389aa6b2328b5613e7bda8a5cea65f81b7dbd06 Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Apr 06 2021 10:22:40 +0000 Subject: unify warn messages Fixes: https://pagure.io/koji/issue/2795 --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index cf1896b..d7524c8 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -1217,9 +1217,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 @@ -1505,9 +1505,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: @@ -1895,7 +1895,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 @@ -4266,8 +4266,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) @@ -6988,7 +6987,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) @@ -7170,13 +7169,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) @@ -7248,9 +7246,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: @@ -7361,8 +7359,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) diff --git a/tests/test_cli/test_dist_repo.py b/tests/test_cli/test_dist_repo.py index 194b661..b1dd87d 100644 --- a/tests/test_cli/test_dist_repo.py +++ b/tests/test_cli/test_dist_repo.py @@ -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""" diff --git a/tests/test_cli/test_import.py b/tests/test_cli/test_import.py index 654b9b7..a191813 100644 --- a/tests/test_cli/test_import.py +++ b/tests/test_cli/test_import.py @@ -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 diff --git a/tests/test_cli/test_import_sig.py b/tests/test_cli/test_import_sig.py index 7de630e..c617d36 100644 --- a/tests/test_cli/test_import_sig.py +++ b/tests/test_cli/test_import_sig.py @@ -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 diff --git a/tests/test_cli/test_regen_repo.py b/tests/test_cli/test_regen_repo.py index bf8d8e6..59c318e 100644 --- a/tests/test_cli/test_regen_repo.py +++ b/tests/test_cli/test_regen_repo.py @@ -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): diff --git a/tests/test_cli/test_wait_repo.py b/tests/test_cli/test_wait_repo.py index 8f9dd38..7d4f152 100644 --- a/tests/test_cli/test_wait_repo.py +++ b/tests/test_cli/test_wait_repo.py @@ -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"""