rewrite RunNode.test_client to use "tahoe run"

src/allmydata/scripts/tahoe_stop.py

@@ -32,12 +32,12 @@ def stop(config):
         print("%s does not look like a running node directory (no twistd.pid)" % quoted_basedir, file=err)
         # we define rc=2 to mean "nothing is running, but it wasn't me who
         # stopped it"
-        return 2
+        return COULD_NOT_STOP
     elif pid == -1:
         print("%s contains an invalid PID file" % basedir, file=err)
         # we define rc=2 to mean "nothing is running, but it wasn't me who
         # stopped it"
-        return 2
+        return COULD_NOT_STOP
 
     # kill it hard (SIGKILL), delete the twistd.pid file, then wait for the
     # process itself to go away. If it hasn't gone away after 20 seconds, warn

src/allmydata/test/cli_node_api.py

@@ -0,0 +1,152 @@
+
+__all__ = [
+    "CLINodeAPI",
+    "Expect",
+    "on_stdout",
+    "wait_for_exit",
+]
+
+import os
+import sys
+
+import attr
+
+from twisted.internet.error import (
+    ProcessDone,
+)
+from twisted.python.filepath import (
+    FilePath,
+)
+from twisted.internet.protocol import (
+    Protocol,
+    ProcessProtocol,
+)
+from twisted.internet.defer import (
+    Deferred,
+    succeed,
+)
+
+from allmydata.client import _Client
+
+class Expect(Protocol):
+    def __init__(self):
+        self._expectations = []
+
+    def expect(self, expectation):
+        if expectation in self._buffer:
+            return succeed(None)
+        d = Deferred()
+        self._expectations.append((expectation, d))
+        return d
+
+    def connectionMade(self):
+        self._buffer = b""
+
+    def dataReceived(self, data):
+        self._buffer += data
+        for i in range(len(self._expectations) - 1, -1, -1):
+            expectation, d = self._expectations[i]
+            if expectation in self._buffer:
+                del self._expectations[i]
+                d.callback(None)
+
+
+class _Stdout(ProcessProtocol):
+    def __init__(self, stdout_protocol):
+        self._stdout_protocol = stdout_protocol
+
+    def connectionMade(self):
+        self._stdout_protocol.makeConnection(self.transport)
+
+    def outReceived(self, data):
+        self._stdout_protocol.dataReceived(data)
+
+    def processEnded(self, reason):
+        self._stdout_protocol.connectionLost(reason)
+
+
+def on_stdout(protocol):
+    return _Stdout(protocol)
+
+
+@attr.s
+class CLINodeAPI(object):
+    reactor = attr.ib()
+    basedir = attr.ib(type=FilePath)
+
+    @property
+    def twistd_pid_file(self):
+        return self.basedir.child(u"twistd.pid")
+
+    @property
+    def node_url_file(self):
+        return self.basedir.child(u"node.url")
+
+    @property
+    def storage_furl_file(self):
+        return self.basedir.child(u"private").child(u"storage.furl")
+
+    @property
+    def config_file(self):
+        return self.basedir.child(u"tahoe.cfg")
+
+    @property
+    def exit_trigger_file(self):
+        return self.basedir.child(_Client.EXIT_TRIGGER_FILE)
+
+    def _execute(self, process_protocol, argv):
+        exe = sys.executable
+        argv = [
+            exe,
+            u"-m",
+            u"allmydata.scripts.runner",
+        ] + argv
+        return self.reactor.spawnProcess(
+            processProtocol=process_protocol,
+            executable=exe,
+            args=argv,
+            env=os.environ,
+        )
+
+    def run(self, protocol):
+        """
+        Start the node running.
+
+        :param ProcessProtocol protocol: This protocol will be hooked up to
+            the node process and can handle output or generate input.
+        """
+        self.process = self._execute(
+            protocol,
+            [u"run", self.basedir.asTextMode().path],
+        )
+        # Don't let the process run away forever.
+        self.active()
+
+    def stop(self, protocol):
+        self._execute(
+            protocol,
+            [u"stop", self.basedir.asTextMode().path],
+        )
+
+    def active(self):
+        # By writing this file, we get two minutes before the client will
+        # exit. This ensures that even if the 'stop' command doesn't work (and
+        # the test fails), the client should still terminate.
+        self.exit_trigger_file.touch()
+
+
+class _WaitForEnd(ProcessProtocol):
+    def __init__(self, ended):
+        self._ended = ended
+
+    def processEnded(self, reason):
+        if reason.check(ProcessDone):
+            self._ended.callback(None)
+        else:
+            self._ended.errback(reason)
+
+
+def wait_for_exit():
+    ended = Deferred()
+    protocol = _WaitForEnd(ended)
+    return protocol, ended

src/allmydata/test/test_runner.py

@@ -9,10 +9,17 @@ from unittest import (
     skipIf,
 )
 
+import attr
+
 from twisted.trial import unittest
 
+from twisted.internet.error import (
+    ProcessTerminated,
+)
+from twisted.internet import reactor
 from twisted.python import usage, runtime
 from twisted.internet.defer import inlineCallbacks, returnValue
+from twisted.python.filepath import FilePath
 
 from allmydata.util import fileutil, pollmixin
 from allmydata.util.encodingutil import unicode_to_argv, unicode_to_output, \
@@ -22,10 +29,21 @@ from allmydata.test import common_util
 import allmydata
 from allmydata import __appname__
 from .common_util import parse_cli, run_cli
-
+from .cli_node_api import (
+    CLINodeAPI,
+    Expect,
+    on_stdout,
+    wait_for_exit,
+)
 from ._twisted_9607 import (
     getProcessOutputAndValue,
 )
+from ..util.eliotutil import (
+    inline_callbacks,
+)
+from ..scripts.tahoe_stop import (
+    COULD_NOT_STOP,
+)
 
 timeout = 240
 
@@ -367,19 +385,22 @@ class CreateNode(unittest.TestCase):
         # can't provide all three
         _test("create-stats-gatherer --hostname=foo --location=foo --port=foo D")
 
+
 class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin,
               RunBinTahoeMixin):
-    # exercise "tahoe start", for both introducer, client node, and
+    """
-    # key-generator, by spawning "tahoe start" as a subprocess. This doesn't
+    exercise "tahoe run" (or "tahoe start", for a while), for both
-    # get us figleaf-based line-level coverage, but it does a better job of
+    introducer, client node, and key-generator, by spawning "tahoe run" (or
-    # confirming that the user can actually run "./bin/tahoe start" and
+    "tahoe start") as a subprocess. This doesn't get us line-level coverage,
-    # expect it to work. This verifies that bin/tahoe sets up PYTHONPATH and
+    but it does a better job of confirming that the user can actually run
-    # the like correctly.
+    "./bin/tahoe run" and expect it to work. This verifies that bin/tahoe
+    sets up PYTHONPATH and the like correctly.
 
-    # This doesn't work on cygwin (it hangs forever), so we skip this test
+    This doesn't work on cygwin (it hangs forever), so we skip this test
-    # when we're on cygwin. It is likely that "tahoe start" itself doesn't
+    when we're on cygwin. It is likely that "tahoe start" itself doesn't
-    # work on cygwin: twisted seems unable to provide a version of
+    work on cygwin: twisted seems unable to provide a version of
-    # spawnProcess which really works there.
+    spawnProcess which really works there.
+    """
 
     def workdir(self, name):
         basedir = os.path.join("test_runner", "RunNode", name)
@@ -568,118 +589,98 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin,
         d.addBoth(self._remove, exit_trigger_file)
         return d
 
-    @skipIf(cannot_daemonize, cannot_daemonize)
+    @inline_callbacks
     def test_client(self):
+        """
+        Test many things.
+
+        0) Verify that "tahoe create-node" takes a --webport option and writes
+           the value to the configuration file.
+
+        1) Verify that "tahoe run" writes a pid file and a node url file.
+
+        2) Verify that the storage furl file has a stable value across a
+           "tahoe run" / "tahoe stop" / "tahoe run" sequence.
+
+        3) Verify that the pid file is removed after "tahoe stop" succeeds.
+        """
         basedir = self.workdir("test_client")
         c1 = os.path.join(basedir, "c1")
-        exit_trigger_file = os.path.join(c1, _Client.EXIT_TRIGGER_FILE)
-        twistd_pid_file = os.path.join(c1, "twistd.pid")
-        node_url_file = os.path.join(c1, "node.url")
-        storage_furl_file = os.path.join(c1, "private", "storage.furl")
-        config_file = os.path.join(c1, "tahoe.cfg")
 
-        d = self.run_bintahoe(["--quiet", "create-node", "--basedir", c1,
+        def stop_and_wait(tahoe):
+            p, d = wait_for_exit()
+            tahoe.stop(p)
+            return d
+
+        tahoe = CLINodeAPI(reactor, FilePath(c1))
+        # Set this up right now so we don't forget later.
+        self.addCleanup(
+            lambda: stop_and_wait(tahoe).addErrback(
+                # Let it fail because the process has already exited.
+                lambda err: (
+                    err.trap(ProcessTerminated)
+                    and self.assertEqual(
+                        err.value.exitCode,
+                        COULD_NOT_STOP,
+                    )
+                )
+            )
+        )
+
+        out, err, rc_or_sig = yield self.run_bintahoe([
+            "--quiet", "create-node", "--basedir", c1,
             "--webport", "0",
-                               "--hostname", "localhost"])
+            "--hostname", "localhost",
-        def _cb(res):
+        ])
-            out, err, rc_or_sig = res
         self.failUnlessEqual(rc_or_sig, 0)
 
         # Check that the --webport option worked.
-            config = fileutil.read(config_file)
+        config = fileutil.read(tahoe.config_file.path)
-            self.failUnlessIn('\nweb.port = 0\n', config)
+        self.assertIn('\nweb.port = 0\n', config)
 
-            # By writing this file, we get two minutes before the client will
+        # After this it's safe to start the node
-            # exit. This ensures that even if the 'stop' command doesn't work
+        tahoe.active()
-            # (and the test fails), the client should still terminate.
-            fileutil.write(exit_trigger_file, "")
-            # now it's safe to start the node
-        d.addCallback(_cb)
 
-        def _start(res):
+        p = Expect()
-            return self.run_bintahoe(["--quiet", "start", c1])
+        # This will run until we stop it.
-        d.addCallback(_start)
+        tahoe.run(on_stdout(p))
+        # Wait for startup to have proceeded to a reasonable point.
+        yield p.expect("client running")
+        tahoe.active()
 
-        def _cb2(res):
+        # read the storage.furl file so we can check that its contents don't
-            out, err, rc_or_sig = res
+        # change on restart
-            fileutil.write(exit_trigger_file, "")
+        storage_furl = fileutil.read(tahoe.storage_furl_file.path)
-            errstr = "rc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err)
+        self.assertTrue(tahoe.twistd_pid_file.exists())
-            self.failUnlessEqual(rc_or_sig, 0, errstr)
-            self.failUnlessEqual(out, "", errstr)
-            # self.failUnlessEqual(err, "", errstr) # See test_client_no_noise -- for now we ignore noise.
-
-            # the parent (twistd) has exited. However, twistd writes the pid
-            # from the child, not the parent, so we can't expect twistd.pid
-            # to exist quite yet.
-
-            # the node is running, but it might not have made it past the
-            # first reactor turn yet, and if we kill it too early, it won't
-            # remove the twistd.pid file. So wait until it does something
-            # that we know it won't do until after the first turn.
-        d.addCallback(_cb2)
-
-        def _node_has_started():
-            return os.path.exists(node_url_file)
-        d.addCallback(lambda res: self.poll(_node_has_started))
-
-        def _started(res):
-            # read the storage.furl file so we can check that its contents
-            # don't change on restart
-            self.storage_furl = fileutil.read(storage_furl_file)
-
-            fileutil.write(exit_trigger_file, "")
-            self.failUnless(os.path.exists(twistd_pid_file))
 
         # rm this so we can detect when the second incarnation is ready
-            os.unlink(node_url_file)
+        tahoe.node_url_file.remove()
-            return self.run_bintahoe(["--quiet", "restart", c1])
+        yield stop_and_wait(tahoe)
-        d.addCallback(_started)
 
-        def _cb3(res):
+        p = Expect()
-            out, err, rc_or_sig = res
+        # We don't have to add another cleanup for this one, the one from
+        # above is still registered.
+        tahoe.run(on_stdout(p))
+        yield p.expect("client running")
+        tahoe.active()
 
-            fileutil.write(exit_trigger_file, "")
+        self.assertEqual(
-            errstr = "rc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err)
+            storage_furl,
-            self.failUnlessEqual(rc_or_sig, 0, errstr)
+            fileutil.read(tahoe.storage_furl_file.path),
-            self.failUnlessEqual(out, "", errstr)
+        )
-            # self.failUnlessEqual(err, "", errstr) # See test_client_no_noise -- for now we ignore noise.
-        d.addCallback(_cb3)
 
-        # again, the second incarnation of the node might not be ready yet,
+        self.assertTrue(
-        # so poll until it is
+            tahoe.twistd_pid_file.exists(),
-        d.addCallback(lambda res: self.poll(_node_has_started))
+            "PID file ({}) didn't exist when we expected it to.  These exist: {}".format(
+                tahoe.twistd_pid_file,
+                tahoe.twistd_pid_file.parent().listdir(),
+            ),
+        )
+        yield stop_and_wait(tahoe)
 
-        def _check_same_furl(res):
+        # twistd.pid should be gone by now.
-            self.failUnlessEqual(self.storage_furl,
+        self.assertFalse(tahoe.twistd_pid_file.exists())
-                                 fileutil.read(storage_furl_file))
-        d.addCallback(_check_same_furl)
 
-        # now we can kill it. TODO: On a slow machine, the node might kill
-        # itself before we get a chance to, especially if spawning the
-        # 'tahoe stop' command takes a while.
-        def _stop(res):
-            fileutil.write(exit_trigger_file, "")
-            self.failUnless(os.path.exists(twistd_pid_file),
-                            (twistd_pid_file, os.listdir(os.path.dirname(twistd_pid_file))))
-            return self.run_bintahoe(["--quiet", "stop", c1])
-        d.addCallback(_stop)
-
-        def _cb4(res):
-            out, err, rc_or_sig = res
-
-            fileutil.write(exit_trigger_file, "")
-            # the parent has exited by now
-            errstr = "rc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err)
-            self.failUnlessEqual(rc_or_sig, 0, errstr)
-            self.failUnlessEqual(out, "", errstr)
-            # self.failUnlessEqual(err, "", errstr) # See test_client_no_noise -- for now we ignore noise.
-            # the parent was supposed to poll and wait until it sees
-            # twistd.pid go away before it exits, so twistd.pid should be
-            # gone by now.
-            self.failIf(os.path.exists(twistd_pid_file))
-        d.addCallback(_cb4)
-        d.addBoth(self._remove, exit_trigger_file)
-        return d
 
     def _remove(self, res, file):
         fileutil.remove(file)