[Freeipa-devel] [python-pytest-multihost]-Ticket-6 run_command produces exit code 1 on windows
Niranjan
mrniranjan at fedoraproject.org
Mon Apr 18 15:25:00 UTC 2016
Petr Viktorin wrote:
> On 04/18/2016 12:39 PM, Niranjan wrote:
> > Niranjan wrote:
> >> Niranjan wrote:
> >>> Petr Viktorin wrote:
> >>>> On 04/06/2016 10:55 AM, Niranjan wrote:
> >>>>> Greetings,
> >>>>>
> >>>>> For https://fedorahosted.org/python-pytest-multihost/ticket/6 , i have proposed
> >>>>> a patch, I think this patch is more of a workaround , than a solution. I would
> >>>>> like to get more inputs on how to use pytest-multihost to execute commands
> >>>>> on Windows. The method i am using requires cygwin with openssh/bash to be
> >>>>> installed.
> >>>>
> >>>> Wow. I'm surprised the only problem on Windows is the "set -e".
> >>>>
> >>>> Regarding the patch:
> >>>>
> >>>> - Why is get_winhost_class needed? I don't see it called anywhere.
> >>>> - Similarly, why is host_type needed? Your patch only sets it.
> >>>>
> >>>> If run_command of WinHost and Unix hosts are this similar, I would put
> >>>> the 'set -e\n' for Unix (and an empty string for Windows) in a class
> >>>> attribute named e.g. "command_prelude", use it in run_command, and move
> >>>> run_command from Host to BaseHost.
> >>>> Would that work, or am I missing something?
> >>> How do we detect the host is windows/unix then , we still need host_type
> >>> to know what the type of host is (unix/windows)?
> >>>>
> >>>>
> >>>> --
> >>>> Petr Viktorin
> >> Please review the attached patch.
>
> Sorry for the delay.
>
> The information about whether the host is Unix or Windows is available:
> the class is either Host or WinHost, so I don't think an extra host_type
> is necessary.
> I'd be open to adding host_type as *configuration* (and corresponding
> attribute) if it also selected the actual Host class. Currently to use
> WinHost you need to write custom code.
>
> I modified the patch so BaseHost takes test_dir as an argument, and sets
> it as self.test_dir, so that users don't need to choose between
> self.config.windows_test_dir/self.config.test_dir themselves.
> (Unfortunately I don't have Windows to test on; I hope the change is
> correct.) Would this approach work for you?
I am testing the patch with windows AD , Will update with my observations
>
> --
> Petr Viktorin
>
> From f4e8290beb21eeee7aba60f1caf7e69b472e1e8e Mon Sep 17 00:00:00 2001
> From: Niranjan MR <mrniranjan at fedoraproject.org>
> Date: Tue, 12 Apr 2016 17:18:10 +0530
> Subject: [PATCH] modify run_command to execute on Windows
>
> Add global parameter windows_test_dir to specify test directory
> for Windows
> Windows hosts (WinHost) use this directory by default.
> test_dir can now be set individually for each host.
> Move run_command from Host class to BaseHost class
>
> Signed-off-by: Petr Viktorin <pviktroi at redhat.com>
> ---
> pytest_multihost/config.py | 3 +++
> pytest_multihost/host.py | 41 ++++++++++++++++++++++-------------------
> 2 files changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/pytest_multihost/config.py b/pytest_multihost/config.py
> index 31045c2..58a3a5f 100644
> --- a/pytest_multihost/config.py
> +++ b/pytest_multihost/config.py
> @@ -45,6 +45,7 @@ class Config(object):
> self.ssh_password = kwargs.get('ssh_password')
> self.ssh_username = kwargs.get('ssh_username', 'root')
> self.ipv6 = bool(kwargs.get('ipv6', False))
> + self.windows_test_dir = kwargs.get('windows_test_dir', '/home/Administrator')
>
> if not self.ssh_password and not self.ssh_key_filename:
> self.ssh_key_filename = '~/.ssh/id_rsa'
> @@ -80,6 +81,8 @@ class Config(object):
> dct['ssh_key_filename'] = dct.pop('root_ssh_key_filename')
> if 'root_password' in dct:
> dct['ssh_password'] = dct.pop('root_password')
> + if 'windows_test_dir' in dct:
> + dct['windows_test_dir'] = dct.pop('windows_test_dir')
>
> all_init_args = set(init_args) | set(cls.extra_init_args)
> extra_args = set(dct) - all_init_args
> diff --git a/pytest_multihost/host.py b/pytest_multihost/host.py
> index e6c0db5..f82ff33 100644
> --- a/pytest_multihost/host.py
> +++ b/pytest_multihost/host.py
> @@ -25,9 +25,11 @@ class BaseHost(object):
> See README for an overview of the core classes.
> """
> transport_class = None
> + command_prelude = ''
>
> def __init__(self, domain, hostname, role, ip=None,
> - external_hostname=None, username=None, password=None):
> + external_hostname=None, username=None, password=None,
> + test_dir=None):
> self.domain = domain
> self.role = str(role)
> if username is None:
> @@ -40,6 +42,10 @@ class BaseHost(object):
> else:
> self.ssh_key_filename = None
> self.ssh_password = password
> + if test_dir is None:
> + self.test_dir = domain.config.test_dir
> + else:
> + self.test_dir = test_dir
>
> shortname, dot, ext_domain = hostname.partition('.')
> self.shortname = shortname
> @@ -78,7 +84,7 @@ class BaseHost(object):
> self.host_key = None
> self.ssh_port = 22
>
> - self.env_sh_path = os.path.join(domain.config.test_dir, 'env.sh')
> + self.env_sh_path = os.path.join(self.test_dir, 'env.sh')
>
> self.log_collectors = []
>
> @@ -204,28 +210,18 @@ class BaseHost(object):
> does not exit with return code 0
> :param cwd: The working directory for the command
> """
> - raise NotImplementedError()
> -
> -
> -class Host(BaseHost):
> - """A Unix host"""
> - transport_class = transport.SSHTransport
> -
> - def run_command(self, argv, set_env=True, stdin_text=None,
> - log_stdout=True, raiseonerr=True,
> - cwd=None):
> - # This will give us a Bash shell
> command = self.transport.start_shell(argv, log_stdout=log_stdout)
> -
> # Set working directory
> if cwd is None:
> - cwd = self.config.test_dir
> + cwd = self.test_dir
> command.stdin.write('cd %s\n' % shell_quote(cwd))
>
> # Set the environment
> if set_env:
> command.stdin.write('. %s\n' % shell_quote(self.env_sh_path))
> - command.stdin.write('set -e\n')
> +
> + if self.command_prelude:
> + command.stdin.write(self.command_prelude)
>
> if isinstance(argv, basestring):
> # Run a shell command given as a string
> @@ -247,11 +243,18 @@ class Host(BaseHost):
> return command
>
>
> +class Host(BaseHost):
> + """A Unix host"""
> + transport_class = transport.SSHTransport
> + command_prelude = 'set -e\n'
> +
> +
> class WinHost(BaseHost):
> """
> Representation of a remote Windows host.
> -
> - This is a stub; Windows hosts can't currently be interacted with.
> """
>
> - pass
> + def __init__(self, domain, hostname, role, **kwargs):
> + # Set test_dir to the Windows directory, if not given explicitly
> + kwargs.setdefault('test_dir', domain.config.windows_test_dir)
> + super(WinHost, self).__init__(domain, hostname, role, **kwargs)
> --
> 2.5.5
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 328 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160418/75b9a31f/attachment.sig>
More information about the Freeipa-devel
mailing list