Keep default as no timeout, add unit tests

docs/docs/zerodeploy.rst

@@ -118,4 +118,5 @@ Timeouts
 --------
 You can pass a ``timeout`` argument, in seconds, to the ``close()`` method.  A ``TimeoutExpired`` is raised if
 any subprocess communication takes longer than the timeout, after the subprocess has been told to terminate.  By
-default, the timeout is four minutes.  The timeout prevents a ``close()`` call blocking indefinitely.
+default, the timeout is ``None`` i.e. infinite.  A timeout value prevents a ``close()`` call blocking
+indefinitely.

rpyc/utils/zerodeploy.py

@@ -152,7 +152,7 @@ def __enter__(self):
     def __exit__(self, t, v, tb):
         self.close()
 
-    def close(self, timeout=4*60):
+    def close(self, timeout=None):
         if self.proc is not None:
             try:
                 self.proc.terminate()

tests/test_deploy.py

@@ -1,6 +1,9 @@
 from __future__ import with_statement
+
 import unittest
+import subprocess
 import sys
+
 from plumbum import SshMachine
 from plumbum.machines.paramiko_machine import ParamikoMachine
 from rpyc.utils.zerodeploy import DeployedServer
@@ -11,7 +14,6 @@
     _paramiko_import_failed = True
 
 
-@unittest.skipIf(_paramiko_import_failed, "Paramiko is not available")
 class TestDeploy(unittest.TestCase):
     def test_deploy(self):
         rem = SshMachine("localhost")
@@ -30,6 +32,51 @@ def test_deploy(self):
             self.fail("expected an EOFError")
         rem.close()
 
+    def test_close_timeout(self):
+        expected_timeout = 4
+        observed_timeouts = []
+        original_communicate = subprocess.Popen.communicate
+
+        def replacement_communicate(self, input=None, timeout=None):
+            observed_timeouts.append(timeout)
+            return original_communicate(self, input, timeout)
+
+        try:
+            subprocess.Popen.communicate = replacement_communicate
+            rem = SshMachine("localhost")
+            SshMachine.python = rem[sys.executable]
+            dep = DeployedServer(rem)
+            dep.classic_connect()
+            dep.close(timeout=expected_timeout)
+            rem.close()
+        finally:
+            subprocess.Popen.communicate = original_communicate
+        # The last three calls to communicate() happen during close(), so check they
+        # applied the timeout.
+        assert observed_timeouts[-3:] == [expected_timeout] * 3
+
+    def test_close_timeout_default_none(self):
+        observed_timeouts = []
+        original_communicate = subprocess.Popen.communicate
+
+        def replacement_communicate(self, input=None, timeout=None):
+            observed_timeouts.append(timeout)
+            return original_communicate(self, input, timeout)
+
+        try:
+            subprocess.Popen.communicate = replacement_communicate
+            rem = SshMachine("localhost")
+            SshMachine.python = rem[sys.executable]
+            dep = DeployedServer(rem)
+            dep.classic_connect()
+            dep.close()
+            rem.close()
+        finally:
+            subprocess.Popen.communicate = original_communicate
+        # No timeout specified, so Popen.communicate should have been called with timeout None.
+        assert observed_timeouts == [None] * len(observed_timeouts)
+
+    @unittest.skipIf(_paramiko_import_failed, "Paramiko is not available")
     def test_deploy_paramiko(self):
         rem = ParamikoMachine("localhost", missing_host_policy=paramiko.AutoAddPolicy())
         with DeployedServer(rem) as dep: