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 <[email protected]> > 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 <[email protected]> > --- > 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 >
pgpC3e4XXrZ3t.pgp
Description: PGP signature
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
