Skip to content

Commit

Permalink
Added a sudo argument to method cli.Client.run to avoid boilerplate.
Browse files Browse the repository at this point in the history
As part of our `less boilerplate` effort I added a new method `sudo` argument
to `cli.Client.run`.

In a lot of test cases there are code like:

```python
sudo = () if cli.is_root(cfg) else ('sudo',)
...
client.run(sudo + ('command_name', 'argument1', ...))
```

The idea is to replace the above boilerplate with

```python
client.run(('command_name', 'argument1', ...), sudo=True)
```

With this change we will be able to remove the `sudo = () if cli.is_root(cfg) else ('sudo',)` boilerplate in those Pulp-2-tests referenced:

```bash

pulp_2_tests/tests/docker/utils.py:    sudo = '' if cli.is_root(cfg) else 'sudo'
pulp_2_tests/tests/puppet/api_v2/test_install_distributor.py:        sudo = () if cli.is_root(self.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_content_sources.py:        sudo = () if cli.is_root(cls.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_content_sources.py:        sudo = () if cli.is_root(cls.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_content_sources.py:    sudo = '' if cli.is_root(cfg) else 'sudo'
pulp_2_tests/tests/rpm/api_v2/test_crud.py:        sudo = () if cli.is_root(self.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_download_policies.py:        sudo = '' if cli.is_root(cls.cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/test_errata.py:        # sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_errata.py:        sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_export.py:        self.__sudo = None
pulp_2_tests/tests/rpm/api_v2/test_export.py:            self.__sudo = '' if cli.is_root(self.cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/test_iso_sync_publish.py:        sudo = () if cli.is_root(self.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_modularity.py:        sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_republish.py:        sudo = () if cli.is_root(self.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_rich_weak_dependencies.py:        sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_rsync_distributor.py:        sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_rsync_distributor.py:        sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_rsync_distributor.py:        sudo = '' if cli.is_root(cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/test_service_resiliency.py:        sudo = '' if cli.is_root(self.cfg) else 'sudo'
pulp_2_tests/tests/rpm/api_v2/test_service_resiliency.py:        sudo = () if cli.is_root(self.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/utils.py:        sudo = '' if cli.is_root(cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/utils.py:        sudo = '' if cli.is_root(cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/utils.py:        sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/utils.py:        sudo = '' if cli.is_root(cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/utils.py:    sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/cli/test_copy_units.py:        sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/cli/test_process_recycling.py:    sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/cli/test_process_recycling.py:        sudo = () if cli.is_root(cfg) else ('sudo',)

```
rochacbruno committed Sep 6, 2018
1 parent 52561d9 commit f8a21c3
Showing 2 changed files with 62 additions and 1 deletion.
11 changes: 10 additions & 1 deletion pulp_smash/cli.py
Original file line number Diff line number Diff line change
@@ -213,14 +213,20 @@ def __init__(self, cfg, response_handler=None, pulp_host=None):
else:
self.response_handler = response_handler

def run(self, args, **kwargs):
self.cfg = cfg

def run(self, args, sudo=False, **kwargs):
"""Run a command and ``return self.response_handler(result)``.
This method is a thin wrapper around Plumbum's `BaseCommand.run`_
method, which is itself a thin wrapper around the standard library's
`subprocess.Popen`_ class. See their documentation for detailed usage
instructions. See :class:`pulp_smash.cli.Client` for a usage example.
:param args: Any arguments to be passed to the process (a tuple).
:param sudo: If the command should run as superuser (a boolean).
:param kwargs: Extra named arguments passed to plumbumBaseCommand.run.
.. _BaseCommand.run:
http://plumbum.readthedocs.io/en/latest/api/commands.html#plumbum.commands.base.BaseCommand.run
.. _subprocess.Popen:
@@ -230,6 +236,9 @@ def run(self, args, **kwargs):
# https://plumbum.readthedocs.io/en/latest/api/commands.html#plumbum.commands.base.BaseCommand.run
kwargs.setdefault('retcode')

if sudo and args[0] != 'sudo' and not is_root(self.cfg):
args = ('sudo',) + tuple(args)

code, stdout, stderr = self.machine[args[0]].run(args[1:], **kwargs)
completed_process = CompletedProcess(args, code, stdout, stderr)
return self.response_handler(completed_process)
52 changes: 52 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
@@ -167,6 +167,58 @@ def test_explicit_pulp_host(self):
plumbum.machines.SshMachine.assert_called_once_with(
cfg.hosts[1].hostname)

def test_run(self):
"""Test run commands."""
cfg = _get_pulp_smash_config(hosts=[
config.PulpHost(
hostname=socket.getfqdn(),
roles={'shell': {}},
)
])
client = cli.Client(cfg)
with mock.patch.object(client, 'machine') as machine:

machine.__getitem__.return_value = machine
machine.run.return_value = (0, 'ok', None)

result = client.run(('ls', '-la'))

# Internal call is: `machine[args[0]].run(args[1:], **kwargs)`
# So assert `machine['ls']` is called
machine.__getitem__.assert_called_once_with('ls')
# then `.run(('-la',), **kwargs)`
machine.run.assert_called_once_with(('-la',), retcode=None)

self.assertEqual(result.returncode, 0)
self.assertEqual(result.stdout, 'ok')
self.assertIsNone(result.stderr)

def test_run_as_sudo(self):
"""Test run commands as sudo."""
cfg = _get_pulp_smash_config(hosts=[
config.PulpHost(
hostname=socket.getfqdn(),
roles={'shell': {}},
)
])
client = cli.Client(cfg)
with mock.patch.object(client, 'machine') as machine:

machine.__getitem__.return_value = machine
machine.run.return_value = (0, 'ok', None)

result = client.run(('ls', '-la'), sudo=True)

# Internal call is: `machine[args[0]].run(args[1:], **kwargs)`
# So assert `machine['sudo']` is called
machine.__getitem__.assert_called_once_with('sudo')
# then `.run(('ls', '-la'), **kwargs)`
machine.run.assert_called_once_with(('ls', '-la'), retcode=None)

self.assertEqual(result.returncode, 0)
self.assertEqual(result.stdout, 'ok')
self.assertIsNone(result.stderr)


class IsRootTestCase(unittest.TestCase):
"""Tests for :class:`pulp_smash.cli.is_root`."""

0 comments on commit f8a21c3

Please sign in to comment.