#2803 unify warn messages
Merged a year ago by tkopecek. Opened a year ago by jcupova.
jcupova/koji issue2795  into  master

file modified
+18 -18
@@ -1217,9 +1217,9 @@ 

              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 @@ 

                  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 @@ 

                      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 @@ 

          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 @@ 

                  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 @@ 

      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 @@ 

          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 @@ 

              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)

@@ -80,17 +80,20 @@ 

      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 @@ 

  

      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 @@ 

          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 @@ 

          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 @@ 

          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 @@ 

          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 @@ 

              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"""

file modified
+14 -10
@@ -129,18 +129,22 @@ 

          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 @@ 

          # 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

@@ -70,6 +70,7 @@ 

  %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 @@ 

              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 @@ 

                  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 @@ 

          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

@@ -72,14 +72,17 @@ 

      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 @@ 

          # 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 @@ 

          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):

@@ -62,7 +62,8 @@ 

      @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 @@ 

          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 @@ 

          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"""