PR#2694: adding invalid target name error message - koji

koji

#2694 adding invalid target name error message

Merged a year ago by tkopecek. Opened a year ago by lrossett.

lrossett/koji issue-2600_list-targets-error into master

adding invalid target name error message

lrossett • a year ago

2a8bb15

cli/koji_cli/commands.py

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

tests/test_cli/test_list_targets.py

file added

+98

		`@@ -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')])`

lrossett commented a year ago

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 commented a year ago

:thumbsup:

jcupova commented 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'.