[Freeipa-devel] [python-pytest-multihost]-Ticket-6 run_command produces exit code 1 on windows

Niranjan mrniranjan at fedoraproject.org
Mon Apr 18 10:39:22 UTC 2016


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.
> 
> 
> 
> > -- 
> > 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
> 
Could you please review this patch

> From 5b01d7eb1bc3723201b6083a814467f87d865367 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
> Add parameter host_type(per host) to specify the type of host
> (host_type:windows)
> move run_command from Host class to BaseHost class
> 
> Signed-off-by: Niranjan MR <mrniranjan at fedoraproject.org>
> ---
>  pytest_multihost/config.py |  3 ++
>  pytest_multihost/host.py   | 87 +++++++++++++++++++++++++---------------------
>  2 files changed, 50 insertions(+), 40 deletions(-)
> 
> diff --git a/pytest_multihost/config.py b/pytest_multihost/config.py
> index 31045c21e8ee67c1793f154f841375f8df7b365f..58a3a5fbc8dbcdaac35e3fd305f23999d5f5b09f 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 e6c0db50799b2042e699d66ed9ec5f04f6592d31..828ba81279b563e4c5d2735bc6396480f4152304 100644
> --- a/pytest_multihost/host.py
> +++ b/pytest_multihost/host.py
> @@ -27,9 +27,13 @@ class BaseHost(object):
>      transport_class = None
>  
>      def __init__(self, domain, hostname, role, ip=None,
> -                 external_hostname=None, username=None, password=None):
> +                 external_hostname=None, username=None, 
> +                 password=None, windows_test_dir=None, 
> +                 host_type=None):
>          self.domain = domain
>          self.role = str(role)
> +        self.host_type = host_type
> +        self.command_prelude = 'set -e\n'
>          if username is None:
>              self.ssh_username = self.config.ssh_username
>          else:
> @@ -40,6 +44,8 @@ class BaseHost(object):
>          else:
>              self.ssh_key_filename = None
>              self.ssh_password = password
> +        if windows_test_dir is None:
> +            self.windows_test_dir = self.config.windows_test_dir
>  
>          shortname, dot, ext_domain = hostname.partition('.')
>          self.shortname = shortname
> @@ -118,11 +124,13 @@ class BaseHost(object):
>  
>          username = dct.pop('username', None)
>          password = dct.pop('password', None)
> +        windows_test_dir = dct.pop('windows_test_dir', None)
> +        host_type = dct.pop('host_type', None)
>  
>          check_config_dict_empty(dct, 'host %s' % hostname)
>  
>          return cls(domain, hostname, role, ip, external_hostname,
> -                   username, password)
> +                   username, password, windows_test_dir, host_type)
>  
>      def to_dict(self):
>          """Export info about this Host to a dict"""
> @@ -131,6 +139,8 @@ class BaseHost(object):
>              'ip': self.ip,
>              'role': self.role,
>              'external_hostname': self.external_hostname,
> +            'windows_test_dir': self.windows_test_dir,
> +            'host_type': self.host_type
>          }
>  
>      @property
> @@ -199,54 +209,51 @@ class BaseHost(object):
>                          be sourced before running the command.
>          :param stdin_text: If given, will be written to the command's stdin
>          :param log_stdout: If false, standard output will not be logged
> -                           (but will still be available as cmd.stdout_text)
> +                              (but will still be available as cmd.stdout_text)
>          :param raiseonerr: If true, an exception will be raised if the command
>                             does not exit with return code 0
>          :param cwd: The working directory for the command
>          """
> -        raise NotImplementedError()
> +        command = self.transport.start_shell(argv, log_stdout=log_stdout)
> +        #set working directory
> +        if cwd is None:
> +            if self.host_type == 'windows':
> +                cwd = self.config.windows_test_dir
> +            else:
> +                cwd = self.config.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))
> +
> +            if self.host_type != 'windows':
> +                command.stdin.write(self.command_prelude)
> +
> +            if isinstance(argv, basestring):
> +                #Run a shell command given as string
> +                command.stdin.write('(')
> +                command.stdin.write(argv)
> +                command.stdin.write(')')
> +            else:
> +                # Run a command given as a popen-style list (no shell expansion)
> +                for arg in argv:
> +                    command.stdin.write(shell_quote(arg))
> +                    command.stdin.write(' ')
> +            
> +            command.stdin.write(';exit\n')
> +            if stdin_text:
> +                command.stdin.write(stdin_text)
> +            command.stdin.flush()
> +
> +            command.wait(raiseonerr=raiseonerr)
> +            return command
>  
>  
>  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
> -        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 isinstance(argv, basestring):
> -            # Run a shell command given as a string
> -            command.stdin.write('(')
> -            command.stdin.write(argv)
> -            command.stdin.write(')')
> -        else:
> -            # Run a command given as a popen-style list (no shell expansion)
> -            for arg in argv:
> -                command.stdin.write(shell_quote(arg))
> -                command.stdin.write(' ')
> -
> -        command.stdin.write(';exit\n')
> -        if stdin_text:
> -            command.stdin.write(stdin_text)
> -        command.stdin.flush()
> -
> -        command.wait(raiseonerr=raiseonerr)
> -        return command
> -
> -
>  class WinHost(BaseHost):
>      """
>      Representation of a remote Windows host.
> -- 
> 1.8.3.1
> 




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

-------------- 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/483b613d/attachment.sig>


More information about the Freeipa-devel mailing list