koji

Clone

#633 unify runroot CLI interface
Merged 6 years ago by mikem. Opened 6 years ago by tkopecek.
tkopecek/koji issue564  into  master

file modified
+17 -2 
@@ -3,7 +3,8 @@ 
 

  

  import koji
 

  from koji.plugin import export_cli
 

- from koji_cli.lib import _, activate_session, OptionParser, list_task_output_all_volumes
 

+ from koji_cli.lib import _, activate_session, OptionParser, watch_tasks, \
 

+                          list_task_output_all_volumes
 

  

  @export_cli
 

  def handle_runroot(options, session, args):
 
@@ -25,11 +26,18 @@ 
 

      parser.add_option("--new-chroot", action="store_true", default=False,
 

              help=_("Run command with the --new-chroot (systemd-nspawn) option to mock"))
 

      parser.add_option("--repo-id", type="int", help=_("ID of the repo to use"))
 

+     parser.add_option("--nowait", action="store_false", dest="wait",
 

+             default=True, help=_("Do not wait on task"))
 

+     parser.add_option("--watch", action="store_true", help=_("Watch task instead of printing runroot.log"))
 

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

+                       help=_("Do not print the task information"))
 

  

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

  

      if len(args) < 3:
 

          parser.error(_("Incorrect number of arguments"))
 

+         assert False  # pragma: no cover
 

+ 

      activate_session(session, options)
 

      tag = args[0]
 

      arch = args[1]
 
@@ -59,6 +67,14 @@ 
 

      if opts.task_id:
 

          print(task_id)
 

  

+     if not opts.wait:
 

+         return
 

+ 

+     if opts.watch:
 

+         session.logout()
 

+         return watch_tasks(session, [task_id], quiet=opts.quiet,
 

+                            poll_interval=options.poll_interval)
 

+ 

      try:
 

          while True:
 

              # wait for the task to finish
 
@@ -81,4 +97,3 @@ 
 

      state = koji.TASK_STATES[info['state']]
 

      if state in ('FAILED', 'CANCELED'):
 

          sys.exit(1)
 

-     return

file modified
+68 -13 
@@ -8,29 +8,39 @@ 
 

  

  runroot = load_plugin.load_plugin('cli', 'runroot')
 

  

+ 

+ class ParserError(Exception):
 

+     pass
 

+ 

+ 

  class TestListCommands(unittest.TestCase):
 

  

      def setUp(self):
 

          self.options = mock.MagicMock()
 

+         self.options.debug = False
 

          self.session = mock.MagicMock()
 

          self.session.getAPIVersion.return_value = koji.API_VERSION
 

-         self.args = mock.MagicMock()
 

-         runroot.OptionParser = mock.MagicMock()
 

-         self.parser = runroot.OptionParser.return_value
 

+         self.args = ['TAG', 'ARCH', 'COMMAND']
 

+         self.old_OptionParser = runroot.OptionParser
 

+         runroot.OptionParser = mock.MagicMock(side_effect=self.get_parser)
 

+         self.old_watch_tasks = runroot.watch_tasks
 

+         runroot.watch_tasks = mock.MagicMock(name='watch_tasks')
 

+ 

+     def tearDown(self):
 

+         runroot.OptionParser = self.old_OptionParser
 

+         runroot.watch_tasks = self.old_watch_tasks
 

+ 

+     def get_parser(self, *a, **kw):
 

+         # we don't want parser.error to exit
 

+         parser = self.old_OptionParser(*a, **kw)
 

+         parser.error = mock.MagicMock(side_effect=ParserError)
 

+         return parser
 

  

      # Show long diffs in error output...
 

      maxDiff = None
 

  

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

      def test_handle_runroot(self, stdout):
 

-         tag = 'tag'
 

-         arch = 'arch'
 

-         command = 'command'
 

-         arguments = [tag, arch, command]
 

-         options = mock.MagicMock()
 

-         options.new_chroot = False
 

-         self.parser.parse_args.return_value = [options, arguments]
 

- 

          # Mock out the xmlrpc server
 

          self.session.getTaskInfo.return_value = {'state': 1}
 

          self.session.downloadTaskOutput.return_value = 'task output'
 
@@ -41,7 +51,7 @@ 
 

          runroot.handle_runroot(self.options, self.session, self.args)
 

          actual = stdout.getvalue()
 

          actual = actual.replace('nosetests', 'koji')
 

-         expected = 'successfully connected to hub\n1\ntask output'
 

+         expected = 'task output'
 

          self.assertMultiLineEqual(actual, expected)
 

  

          # Finally, assert that things were called as we expected.
 
@@ -50,7 +60,52 @@ 
 

          self.session.downloadTaskOutput.assert_called_once_with(
 

              1, 'runroot.log', volume='DEFAULT')
 

          self.session.runroot.assert_called_once_with(
 

-             tag, arch, command, repo_id=mock.ANY, weight=mock.ANY,
 

+             'TAG', 'ARCH', ['COMMAND'], repo_id=mock.ANY, weight=mock.ANY,
 

              mounts=mock.ANY, packages=mock.ANY, skip_setarch=mock.ANY,
 

              channel=mock.ANY,
 

          )
 

+ 

+     def test_handle_runroot_watch(self):
 

+         args = ['--watch', 'TAG', 'ARCH', 'COMMAND']
 

+ 

+         # Mock out the xmlrpc server
 

+         self.session.runroot.return_value = 1
 

+ 

+         # Run it and check immediate output
 

+         runroot.handle_runroot(self.options, self.session, args)
 

+ 

+         # Finally, assert that things were called as we expected.
 

+         runroot.watch_tasks.assert_called_once()
 

+         self.session.getTaskInfo.assert_not_called()
 

+         self.session.listTaskOutput.assert_not_called()
 

+         self.session.downloadTaskOutput.assert_not_called()
 

+         self.session.runroot.assert_called_once()
 

+ 

+     def test_invalid_arguments(self):
 

+         args = ['TAG', 'COMMAND']  # no arch
 

+ 

+         # Run it and check immediate output
 

+         with self.assertRaises(ParserError):
 

+             runroot.handle_runroot(self.options, self.session, args)
 

+ 

+         # Finally, assert that things were called as we expected.
 

+         self.session.getTaskInfo.assert_not_called()
 

+         self.session.listTaskOutput.assert_not_called()
 

+         self.session.downloadTaskOutput.assert_not_called()
 

+         self.session.runroot.assert_not_called()
 

+ 

+     def test_nowait(self):
 

+         args = ['--nowait', 'TAG', 'ARCH', 'COMMAND']
 

+ 

+         # Mock out the xmlrpc server
 

+         self.session.runroot.return_value = 1
 

+ 

+         # Run it and check immediate output
 

+         runroot.handle_runroot(self.options, self.session, args)
 

+ 

+         # Finally, assert that things were called as we expected.
 

+         runroot.watch_tasks.assert_not_called()
 

+         self.session.getTaskInfo.assert_not_called()
 

+         self.session.listTaskOutput.assert_not_called()
 

+         self.session.downloadTaskOutput.assert_not_called()
 

+         self.session.runroot.assert_called_once()

Related: https://pagure.io/koji/issue/564

Treat runroot task similarly to other build commands.

1 new commit added

  • fix unit test
6 years ago

The thing is, runroot is not quite like other build commands.

The command is designed to act more like running a command via ssh. That is, you issue the command and then you (eventually) get the output on stdout. This PR changes that behavior, which I'm fairly sure some tools rely upon.

I'm fine with adding a --nowait option as #564 asks. I'm even fine with adding a --watch option (or whatever name) that would give this new behavior. However the existing behavior needs to be preserved by default.

rebased onto 283d5de01932d23176783f8825bf3609220dc13c
6 years ago

Updated to be backwards compatible. Added some more tests.

the wait option needs default=True, otherwise we change the default behavior

1 new commit added

  • correct default for --wait
6 years ago

rebased onto 8ce80831f92cfa8f3937abb312790bea158d69f8
6 years ago

parser.error should always raise an error. There are a few places in the code that we put assert False # pragma: no cover afterwards out of paranoia, but a return is not right.

I assume this is here because of behavior in a unit test where the parser is mocked. We should fix the unit test instead of adding a return here.

 
$ git diff master...|flake8 --diff
./plugins/cli/runroot.py:6:1: F401 'koji_cli.lib._running_in_bg' imported but unused

the unit test is doing a number of questionable things actually...

rebased onto 6c9b942
6 years ago

Commit dec0c7b fixes this pull-request

Pull-Request has been merged by mikem@redhat.com
6 years ago
Changes Summary 2
+17 -2
file changed
plugins/cli/runroot.py
+68 -13
file changed
tests/test_plugins/test_runroot_cli.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.