#2694 adding invalid target name error message
Merged a year ago by tkopecek. Opened a year ago by lrossett.

file modified
+9 -1
@@ -4023,15 +4023,23 @@ 

      parser.add_option("--quiet", action="store_true", default=goptions.quiet,

                        help=_("Do not print the header information"))

      (options, args) = parser.parse_args(args)

+ 

      if len(args) != 0:

          parser.error(_("This command takes no arguments"))

      ensure_connection(session)

  

+     targets = session.getBuildTargets(options.name)

+     if len(targets) == 0:

+         if options.name:

+             parser.error(_('Target "%s" does not exist' % options.name))

+         else:

+             parser.error(_('No Targets were found'))

+ 

      fmt = "%(name)-30s %(build_tag_name)-30s %(dest_tag_name)-30s"

      if not options.quiet:

          print("%-30s %-30s %-30s" % ('Name', 'Buildroot', 'Destination'))

          print("-" * 93)

-     tmp_list = sorted([(x['name'], x) for x in session.getBuildTargets(options.name)])

+     tmp_list = sorted([(x['name'], x) for x in targets])

      targets = [x[1] for x in tmp_list]

      for target in targets:

          print(fmt % target)

@@ -0,0 +1,98 @@ 

+ import os

+ import re

+ import time

+ import unittest

+ from optparse import Values

+ 

+ import six

+ import koji

+ import mock

+ 

+ from . import utils

+ from koji_cli.commands import anon_handle_list_targets

+ 

+ 

+ _mock_targets = [

+     {

+         'build_tag': 10,

+         'build_tag_name': 'f33-build',

+         'dest_tag': 20,

+         'dest_tag_name':

+         'f33-updates-candidate',

+         'id': 1,

+         'name': 'f33'

+     }

+ ]

+ 

+ 

+ class TestCliListTargets(utils.CliTestCase):

+     def setUp(self):

+         self.original_timezone = os.environ.get('TZ')

+         os.environ['TZ'] = 'US/Eastern'

+         time.tzset()

+ 

+     def tearDown(self):

+         if self.original_timezone is None:

+             del os.environ['TZ']

+         else:

+             os.environ['TZ'] = self.original_timezone

+         time.tzset()

+ 

+     @mock.patch('sys.stderr', new_callable=six.StringIO)

+     @mock.patch('koji_cli.commands.activate_session')

+     def test_list_targets_error_args(self, ensure_connection_mock, stderr):

+         session = mock.MagicMock(getAPIVersion=lambda: koji.API_VERSION, getBuildTargets=lambda n: [])

+         options = mock.MagicMock(quiet=False)

+         with self.assertRaises(SystemExit) as ex:

+             anon_handle_list_targets(options, session, ['aaa'])

+         self.assertExitCode(ex, 2)

+ 

+     @mock.patch('sys.stderr', new_callable=six.StringIO)

+     @mock.patch('koji_cli.commands.activate_session')

+     def test_list_targets_error_all_not_found(self, ensure_connection_mock, stderr):

+         session = mock.MagicMock(getAPIVersion=lambda: koji.API_VERSION, getBuildTargets=lambda n: [])

+         options = mock.MagicMock(quiet=False)

+         with self.assertRaises(SystemExit) as ex:

+             anon_handle_list_targets(options, session, [])

+         self.assertExitCode(ex, 2)

+         self.assertTrue('No Targets were found' in stderr.getvalue())

+ 

+     @mock.patch('optparse.OptionParser.parse_args', return_value=(Values({'quiet': False, 'name': 'f50'}), []))

+     @mock.patch('sys.stderr', new_callable=six.StringIO)

+     @mock.patch('koji_cli.commands.activate_session')

+     def test_list_targets_error_name_not_found(self, ensure_connection_mock, stderr, opt):

+         session = mock.MagicMock(getAPIVersion=lambda: koji.API_VERSION, getBuildTargets=lambda n: [])

+         options = mock.MagicMock(quiet=False)

+         with self.assertRaises(SystemExit) as ex:

+             anon_handle_list_targets(options, session, [])

+         self.assertExitCode(ex, 2)

+         self.assertTrue('Target "f50" does not exist' in stderr.getvalue())

+ 

+     @mock.patch('sys.stdout', new_callable=six.StringIO)

+     @mock.patch('koji_cli.commands.activate_session')

+     def test_list_targets_all(self, ensure_connection_mock, stdout):

+         session = mock.MagicMock(getAPIVersion=lambda: koji.API_VERSION, getBuildTargets=lambda n: _mock_targets)

+         options = mock.MagicMock(quiet=False)

+         anon_handle_list_targets(options, session, [])

+         expected = [

+             'Name|Buildroot|Destination|',

+             '---------------------------------------------------------------------------------------------',

+             'f33|f33-build|f33-updates-candidate|',

+             ''

+         ]

+         self.assertEqual(expected, [re.sub(' +', '|', l) for l in stdout.getvalue().split('\n')])

+ 

+     @mock.patch('optparse.OptionParser.parse_args', return_value=(Values({'quiet': False, 'name': 'f50'}), []))

+     @mock.patch('sys.stdout', new_callable=six.StringIO)

+     @mock.patch('koji_cli.commands.activate_session')

+     def test_list_targets_one(self, ensure_connection_mock, stdout, opt):

+         session = mock.MagicMock(getAPIVersion=lambda: koji.API_VERSION, getBuildTargets=lambda n: _mock_targets)

+         options = mock.MagicMock(quiet=False)

+         anon_handle_list_targets(options, session, [])

+         expected = [

+             'Name|Buildroot|Destination|',

+             '---------------------------------------------------------------------------------------------',

+             'f33|f33-build|f33-updates-candidate|',

+             ''

+         ]

+         self.assertEqual(expected, [re.sub(' +', '|', l) for l in stdout.getvalue().split('\n')])

Fixes #2600

Changes

  • Sets an error message if on targets were found;
  • Sets a special error message in case --name was used and no records were found;
  • Added test_list_targets.py file

Verification

  • Run tests or run koij list-tragets --name=gimme-an-error

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

a year ago

@tkopecek what do you think about use consistent error message for non existing build target? As I see, we are using 'Invalid build target: non-exist-build-target' or 'Unknown build target: non-exist-build-target'.

Metadata Update from @mfilip:
- Pull-request tagged with: testing-done

a year ago

@jcupova Yep, can you create separate ticket to catch these and reword them in a bulk?

Commit 50db3b9 fixes this pull-request

Pull-Request has been merged by tkopecek

a year ago