Some more fixes and tests. Almost complete coverage! (more cannot be done)

Makefile

@@ -35,6 +35,7 @@ coverage: all
 		set -e; \
 		PYTHONPATH="$(BUILDDIR):$$PYTHONPATH" $(COVERAGE) -x $$i; \
 		done
+	$(COVERAGE) -c
 	$(COVERAGE) -r -m `find "$(BUILDDIR)" -name \\*.py -type f`
 	rm -f .coverage
 

src/netns/protocol.py

@@ -494,7 +494,7 @@ class Client(object):
             exitcode = int(text.split()[0])
             return exitcode
         if code / 100 == 4:
-            return Null
+            return None
         else:
             raise "Error on command: %d %s" % (code, text)
 

src/netns/subprocess.py

@@ -66,22 +66,21 @@ class Subprocess(object):
     def poll(self):
         """Checks status of program, returns exitcode or None if still running.
         See Popen.poll."""
-        r = self._slave.poll(self._pid)
-        if r != None:
-            del self._pid
-            self._returncode = r
+        if self._returncode == None:
+            self._returncode = self._slave.poll(self._pid)
         return self.returncode
 
     def wait(self):
         """Waits for program to complete and returns the exitcode.
         See Popen.wait"""
-        self._returncode = self._slave.wait(self._pid)
-        del self._pid
+        if self._returncode == None:
+            self._returncode = self._slave.wait(self._pid)
         return self.returncode
 
     def signal(self, sig = signal.SIGTERM):
         """Sends a signal to the process."""
-        return self._slave.signal(self._pid, sig)
+        if self._returncode == None:
+            self._slave.signal(self._pid, sig)
 
     @property
     def returncode(self):
@@ -95,7 +94,7 @@ class Subprocess(object):
             return -os.WTERMSIG(self._returncode)
         if os.WIFEXITED(self._returncode):
             return os.WEXITSTATUS(self._returncode)
-        raise RuntimeError("Invalid return code")
+        raise RuntimeError("Invalid return code") # pragma: no cover
 
     # FIXME: do we have any other way to deal with this than having explicit
     # destroy?
@@ -228,11 +227,11 @@ def backticks_raise(node, args):
         args = [ '/bin/sh', '/bin/sh', '-c', args ]
     p = Popen(node, args[0], args[1:], stdout = PIPE)
     out = p.communicate()[0]
-    if p.returncode > 0:
-        raise RuntimeError("Command failed with return code %d." %
-                p.returncode)
-    if p.returncode < 0:
-        raise RuntimeError("Command killed by signal %d." % -p.returncode)
+    ret = p.returncode
+    if ret > 0:
+        raise RuntimeError("Command failed with return code %d." % ret)
+    if ret < 0:
+        raise RuntimeError("Command killed by signal %d." % -ret)
     return out
 
 # =======================================================================
@@ -261,7 +260,7 @@ def spawn(executable, argv = None, cwd = None, env = None, stdin = None,
     filtered_userfd = filter(lambda x: x != None and x >= 0, userfd)
     for i in range(3):
         if userfd[i] != None and not isinstance(userfd[i], int):
-            userfd[i] = userfd[i].fileno()
+            userfd[i] = userfd[i].fileno() # pragma: no cover
 
     # Verify there is no clash
     assert not (set([0, 1, 2]) & set(filtered_userfd))
@@ -284,7 +283,8 @@ def spawn(executable, argv = None, cwd = None, env = None, stdin = None,
 
     (r, w) = os.pipe()
     pid = os.fork()
-    if pid == 0:
+    if pid == 0: # pragma: no cover
+        # coverage doesn't seem to understand fork
         try:
             # Set up stdio piping
             for i in range(3):
@@ -298,16 +298,14 @@ def spawn(executable, argv = None, cwd = None, env = None, stdin = None,
             flags = fcntl.fcntl(w, fcntl.F_GETFD)
             fcntl.fcntl(w, fcntl.F_SETFD, flags | fcntl.FD_CLOEXEC)
 
-            if close_fds == False:
-                pass
-            elif close_fds == True:
+            if close_fds == True:
                 for i in xrange(3, MAXFD):
                     if i != w:
                         try:
                             os.close(i)
                         except:
                             pass
-            else:
+            elif close_fds != False:
                 for i in close_fds:
                     os.close(i)
 
@@ -386,7 +384,7 @@ def _eintr_wrapper(f, *args):
     while True:
         try:
             return f(*args)
-        except OSError, e:
+        except OSError, e: # pragma: no cover
             if e.errno == errno.EINTR:
                 continue
             else:
@@ -394,11 +392,11 @@ def _eintr_wrapper(f, *args):
 
 try:
     MAXFD = os.sysconf("SC_OPEN_MAX")
-except:
+except: # pragma: no cover
     MAXFD = 256
 
 # Used to print extra info in nested exceptions
-def _custom_hook(t, v, tb):
+def _custom_hook(t, v, tb): # pragma: no cover
     if hasattr(v, "child_traceback"):
         sys.stderr.write("Nested exception, original traceback " +
                 "(most recent call last):\n")

t/test_subprocess.py

@@ -2,7 +2,7 @@
 # vim:ts=4:sw=4:et:ai:sts=4
 
 import netns, netns.subprocess, test_util
-import grp, os, pwd, signal, sys, unittest
+import grp, os, pwd, signal, socket, sys, time, unittest
 
 from netns.subprocess import *
 
@@ -115,10 +115,12 @@ class TestSubprocess(unittest.TestCase):
         p = spawn('/bin/cat', stdout = w0, stdin = r1, close_fds = [r0, w1])
         os.close(w0)
         os.close(r1)
+        self.assertEquals(poll(p), None)
         os.write(w1, "hello world\n")
         os.close(w1)
         self.assertEquals(_readall(r0), "hello world\n")
         os.close(r0)
+        self.assertEquals(wait(p), 0)
 
     def test_Subprocess_basic(self):
         node = netns.Node(nonetns = True)
@@ -138,12 +140,29 @@ class TestSubprocess(unittest.TestCase):
         # Test that the environment is cleared: sleep should not be found
         self.assertRaises(RuntimeError, Subprocess, node,
                 'sleep', env = {'PATH': ''})
-        #import pdb; pdb.set_trace()
+
+        # Piping
         r, w = os.pipe()
-        p = Subprocess(node, '/bin/echo', ['echo', 'hello world'], stdout = w)
+        p = Subprocess(node, 'echo', ['echo', 'hello world'], stdout = w)
         os.close(w)
         self.assertEquals(_readall(r), "hello world\n")
         os.close(r)
+        p.wait()
+
+        p = Subprocess(node, 'sleep', ['sleep', '100'])
+        self.assertTrue(p.pid > 0)
+        self.assertEquals(p.poll(), None) # not finished
+        p.signal()
+        p.signal() # verify no-op (otherwise there will be an exception)
+        self.assertEquals(p.wait(), -signal.SIGTERM)
+        self.assertEquals(p.wait(), -signal.SIGTERM) # no-op
+        self.assertEquals(p.poll(), -signal.SIGTERM) # no-op
+
+        p = Subprocess(node, 'sleep', ['sleep', '100'])
+        os.kill(p.pid, signal.SIGTERM)
+        time.sleep(0.2)
+        p.signal() # since it has not been waited for, it should not raise
+        self.assertEquals(p.wait(), -signal.SIGTERM)
 
     def test_Popen(self):
         node = netns.Node(nonetns = True, debug=0)
@@ -151,7 +170,7 @@ class TestSubprocess(unittest.TestCase):
         # repeat test with Popen interface
         r0, w0 = os.pipe()
         r1, w1 = os.pipe()
-        p = Popen(node, '/bin/cat', stdout = w0, stdin = r1)
+        p = Popen(node, 'cat', stdout = w0, stdin = r1)
         os.close(w0)
         os.close(r1)
         os.write(w1, "hello world\n")
@@ -159,18 +178,32 @@ class TestSubprocess(unittest.TestCase):
         self.assertEquals(_readall(r0), "hello world\n")
         os.close(r0)
 
+        # now with a socketpair, not using integers
+        (s0, s1) = socket.socketpair(socket.AF_UNIX, socket.SOCK_STREAM, 0)
+        p = Popen(node, 'cat', stdout = s0, stdin = s0)
+        s0.close()
+        s1.send("hello world\n")
+        self.assertEquals(s1.recv(512), "hello world\n")
+        s1.close()
+
         # pipes
-        p = Popen(node, '/bin/cat', stdin = PIPE, stdout = PIPE)
+        p = Popen(node, 'cat', stdin = PIPE, stdout = PIPE)
         p.stdin.write("hello world\n")
         p.stdin.close()
         self.assertEquals(p.stdout.readlines(), ["hello world\n"])
         self.assertEquals(p.stderr, None)
         self.assertEquals(p.wait(), 0)
 
-        p = Popen(node, '/bin/cat', stdin = PIPE, stdout = PIPE)
+        p = Popen(node, 'cat', stdin = PIPE, stdout = PIPE)
         self.assertEquals(p.communicate(_longstring), (_longstring, None))
 
-        #
+        p = Popen(node, 'cat', stdin = PIPE, stdout = PIPE)
+        p.stdin.write(_longstring)
+        self.assertEquals(p.communicate(), (_longstring, None))
+
+        p = Popen(node, 'cat', stdin = PIPE)
+        self.assertEquals(p.communicate(), (None, None))
+
         p = Popen(node, '/bin/sh', ['sh', '-c', 'cat >&2'],
                 stdin = PIPE, stderr = PIPE)
         p.stdin.write("hello world\n")
@@ -231,7 +264,9 @@ class TestSubprocess(unittest.TestCase):
         self.assertEquals(backticks(node, ["echo", "echo", "hello", "world"]),
                 "hello world\n")
         self.assertEquals(backticks(node, "echo hello world > /dev/null"), "")
+        self.assertEquals(backticks_raise(node, "true"), "")
         self.assertRaises(RuntimeError, backticks_raise, node, "false")
+        self.assertRaises(RuntimeError, backticks_raise, node, "kill $$")
 
     def test_system(self): 
         node = netns.Node(nonetns = True, debug=0)