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?

-- 
Petr Viktorin

From f4e8290beb21eeee7aba60f1caf7e69b472e1e8e Mon Sep 17 00:00:00 2001
From: Niranjan MR <mrniran...@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 <pvikt...@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

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

Reply via email to