From 57bed47495dca1fb00dbb26ee4e12ac1790534a4 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Thu, 8 Sep 2016 00:02:15 -0700
Subject: [PATCH 01/10] runner.py: remove unused arguments

---
 src/allmydata/scripts/runner.py    | 17 +++--------------
 src/allmydata/test/cli/test_cli.py |  3 +--
 2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/src/allmydata/scripts/runner.py b/src/allmydata/scripts/runner.py
index 361cdfa46..cf0880d1c 100644
--- a/src/allmydata/scripts/runner.py
+++ b/src/allmydata/scripts/runner.py
@@ -91,8 +91,7 @@ for module in (create_node, stats_gatherer):
 
 def runner(argv,
            run_by_human=True,
-           stdin=None, stdout=None, stderr=None,
-           install_node_control=True, additional_commands=None):
+           stdin=None, stdout=None, stderr=None):
 
     assert sys.version_info < (3,), ur"Tahoe-LAFS does not run under Python 3. Please use Python 2.7.x."
 
@@ -101,14 +100,6 @@ def runner(argv,
     stderr = stderr or sys.stderr
 
     config = Options()
-    if install_node_control:
-        config.subCommands.extend(startstop_node.subCommands)
-
-    ac_dispatch = {}
-    if additional_commands:
-        for ac in additional_commands:
-            config.subCommands.extend(ac.subCommands)
-            ac_dispatch.update(ac.dispatch)
 
     try:
         config.parseOptions(argv)
@@ -148,21 +139,19 @@ def runner(argv,
         rc = cli.dispatch[command](so)
     elif command in magic_folder_cli.dispatch:
         rc = magic_folder_cli.dispatch[command](so)
-    elif command in ac_dispatch:
-        rc = ac_dispatch[command](so, stdout, stderr)
     else:
         raise usage.UsageError()
 
     return rc
 
 
-def run(install_node_control=True):
+def run():
     try:
         if sys.platform == "win32":
             from allmydata.windows.fixups import initialize
             initialize()
 
-        rc = runner(sys.argv[1:], install_node_control=install_node_control)
+        rc = runner(sys.argv[1:])
     except Exception:
         import traceback
         traceback.print_exc()
diff --git a/src/allmydata/test/cli/test_cli.py b/src/allmydata/test/cli/test_cli.py
index 43b24264e..b1713f8e9 100644
--- a/src/allmydata/test/cli/test_cli.py
+++ b/src/allmydata/test/cli/test_cli.py
@@ -553,9 +553,8 @@ class CLI(CLITestMixin, unittest.TestCase):
         ns = Namespace()
 
         ns.runner_called = False
-        def call_runner(args, install_node_control=True):
+        def call_runner(args):
             ns.runner_called = True
-            self.failUnlessEqual(install_node_control, True)
             raise exc
 
         ns.sys_exit_called = False

From 720aa1a51f41e5739cd461f7c733bf92e9aaafca Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Thu, 8 Sep 2016 02:21:04 -0700
Subject: [PATCH 02/10] unify signature of all CLI dispatch functions

Now they all take a single 'config' argument, instead of some also
taking stdout/stderr args.
---
 src/allmydata/scripts/create_node.py    | 16 +++++++++-------
 src/allmydata/scripts/runner.py         | 13 +++++++------
 src/allmydata/scripts/startstop_node.py | 19 ++++++++++++-------
 src/allmydata/scripts/stats_gatherer.py |  5 +++--
 4 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py
index 159e7a8a4..891c03a82 100644
--- a/src/allmydata/scripts/create_node.py
+++ b/src/allmydata/scripts/create_node.py
@@ -1,6 +1,4 @@
-
-import os, sys
-
+import os
 from allmydata.scripts.common import BasedirOptions, NoDefaultBasedirOptions
 from allmydata.scripts.default_nodedir import _default_nodedir
 from allmydata.util.assertutil import precondition
@@ -129,7 +127,9 @@ def write_client_config(c, config):
     c.write("enabled = false\n")
     c.write("\n")
 
-def create_node(config, out=sys.stdout, err=sys.stderr):
+def create_node(config):
+    out = config.stdout
+    err = config.stderr
     basedir = config['basedir']
     # This should always be called with an absolute Unicode basedir.
     precondition(isinstance(basedir, unicode), basedir)
@@ -162,12 +162,14 @@ def create_node(config, out=sys.stdout, err=sys.stderr):
         print >>out, " Please set [node]nickname= in tahoe.cfg"
     return 0
 
-def create_client(config, out=sys.stdout, err=sys.stderr):
+def create_client(config):
     config['no-storage'] = True
-    return create_node(config, out=out, err=err)
+    return create_node(config)
 
 
-def create_introducer(config, out=sys.stdout, err=sys.stderr):
+def create_introducer(config):
+    out = config.stdout
+    err = config.stderr
     basedir = config['basedir']
     # This should always be called with an absolute Unicode basedir.
     precondition(isinstance(basedir, unicode), basedir)
diff --git a/src/allmydata/scripts/runner.py b/src/allmydata/scripts/runner.py
index cf0880d1c..d512eca16 100644
--- a/src/allmydata/scripts/runner.py
+++ b/src/allmydata/scripts/runner.py
@@ -128,20 +128,21 @@ def runner(argv,
     so.stdin = stdin
 
     if command in create_dispatch:
-        rc = create_dispatch[command](so, stdout, stderr)
+        f = create_dispatch[command]
     elif command in startstop_node.dispatch:
-        rc = startstop_node.dispatch[command](so, stdout, stderr)
+        f = startstop_node.dispatch[command]
     elif command in debug.dispatch:
-        rc = debug.dispatch[command](so)
+        f = debug.dispatch[command]
     elif command in admin.dispatch:
-        rc = admin.dispatch[command](so)
+        f = admin.dispatch[command]
     elif command in cli.dispatch:
-        rc = cli.dispatch[command](so)
+        f = cli.dispatch[command]
     elif command in magic_folder_cli.dispatch:
-        rc = magic_folder_cli.dispatch[command](so)
+        f = magic_folder_cli.dispatch[command]
     else:
         raise usage.UsageError()
 
+    rc = f(so)
     return rc
 
 
diff --git a/src/allmydata/scripts/startstop_node.py b/src/allmydata/scripts/startstop_node.py
index 761984e0f..a7a629f4c 100644
--- a/src/allmydata/scripts/startstop_node.py
+++ b/src/allmydata/scripts/startstop_node.py
@@ -99,7 +99,9 @@ def identify_node_type(basedir):
             return t
     return None
 
-def start(config, out=sys.stdout, err=sys.stderr):
+def start(config):
+    out = config.stdout
+    err = config.stderr
     basedir = config['basedir']
     quoted_basedir = quote_local_unicode_path(basedir)
     print >>out, "STARTING", quoted_basedir
@@ -169,7 +171,9 @@ def start(config, out=sys.stdout, err=sys.stderr):
     # we should only reach here if --nodaemon or equivalent was used
     return 0
 
-def stop(config, out=sys.stdout, err=sys.stderr):
+def stop(config):
+    out = config.stdout
+    err = config.stderr
     basedir = config['basedir']
     quoted_basedir = quote_local_unicode_path(basedir)
     print >>out, "STOPPING", quoted_basedir
@@ -227,23 +231,24 @@ def stop(config, out=sys.stdout, err=sys.stderr):
     # we define rc=1 to mean "I think something is still running, sorry"
     return 1
 
-def restart(config, stdout, stderr):
-    rc = stop(config, stdout, stderr)
+def restart(config):
+    stderr = config.stderr
+    rc = stop(config)
     if rc == 2:
         print >>stderr, "ignoring couldn't-stop"
         rc = 0
     if rc:
         print >>stderr, "not restarting"
         return rc
-    return start(config, stdout, stderr)
+    return start(config)
 
-def run(config, stdout, stderr):
+def run(config):
     config.twistd_args = config.twistd_args + ("--nodaemon",)
     # Previously we would do the equivalent of adding ("--logfile",
     # "tahoesvc.log"), but that redirects stdout/stderr which is often
     # unhelpful, and the user can add that option explicitly if they want.
 
-    return start(config, stdout, stderr)
+    return start(config)
 
 
 subCommands = [
diff --git a/src/allmydata/scripts/stats_gatherer.py b/src/allmydata/scripts/stats_gatherer.py
index 71a5ece14..cb5d1400c 100644
--- a/src/allmydata/scripts/stats_gatherer.py
+++ b/src/allmydata/scripts/stats_gatherer.py
@@ -1,4 +1,4 @@
-import os, sys
+import os
 from twisted.python import usage
 from allmydata.scripts.common import NoDefaultBasedirOptions
 from allmydata.scripts.create_node import write_tac
@@ -56,7 +56,8 @@ class CreateStatsGathererOptions(NoDefaultBasedirOptions):
     """
 
 
-def create_stats_gatherer(config, out=sys.stdout, err=sys.stderr):
+def create_stats_gatherer(config):
+    err = config.stderr
     basedir = config['basedir']
     # This should always be called with an absolute Unicode basedir.
     precondition(isinstance(basedir, unicode), basedir)

From 45e5d5b8917dcec13ef25e9e311af4f4ec1ee6c1 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 9 Sep 2016 00:29:59 -0700
Subject: [PATCH 03/10] test_configutil doesn't need CLITestMixin

---
 src/allmydata/test/test_configutil.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/allmydata/test/test_configutil.py b/src/allmydata/test/test_configutil.py
index ef58dfcdf..7fb34cc61 100644
--- a/src/allmydata/test/test_configutil.py
+++ b/src/allmydata/test/test_configutil.py
@@ -4,12 +4,11 @@ from twisted.trial import unittest
 
 from allmydata.util import configutil
 from allmydata.test.no_network import GridTestMixin
-from .cli.test_cli import CLITestMixin
 from ..scripts import create_node
 from .. import client
 
 
-class ConfigUtilTests(CLITestMixin, GridTestMixin, unittest.TestCase):
+class ConfigUtilTests(GridTestMixin, unittest.TestCase):
 
     def test_config_utils(self):
         self.basedir = "cli/ConfigUtilTests/test-config-utils"

From d85d1ea4990de138853a5e2b96dc2ddf1da830c4 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 9 Sep 2016 00:00:39 -0700
Subject: [PATCH 04/10] CLITestMixin: move into common.py

Also move parse_options(). It was kind of awkward having other test
files import these from test_cli.py.
---
 src/allmydata/test/cli/common.py            | 48 +++++++++++++++++++++
 src/allmydata/test/cli/test_backup.py       |  2 +-
 src/allmydata/test/cli/test_check.py        |  2 +-
 src/allmydata/test/cli/test_cli.py          | 45 +------------------
 src/allmydata/test/cli/test_cp.py           |  2 +-
 src/allmydata/test/cli/test_create_alias.py |  2 +-
 src/allmydata/test/cli/test_list.py         |  2 +-
 src/allmydata/test/cli/test_magic_folder.py |  2 +-
 src/allmydata/test/cli/test_mv.py           |  2 +-
 src/allmydata/test/cli/test_put.py          |  2 +-
 10 files changed, 58 insertions(+), 51 deletions(-)
 create mode 100644 src/allmydata/test/cli/common.py

diff --git a/src/allmydata/test/cli/common.py b/src/allmydata/test/cli/common.py
new file mode 100644
index 000000000..e41825df6
--- /dev/null
+++ b/src/allmydata/test/cli/common.py
@@ -0,0 +1,48 @@
+from cStringIO import StringIO
+from twisted.trial import unittest
+from twisted.internet import threads # CLI tests use deferToThread
+from ...util.assertutil import precondition
+from ...util.encodingutil import (unicode_platform,
+                                  get_filesystem_encoding,
+                                  unicode_to_argv)
+from ...scripts import runner
+from ..common_util import ReallyEqualMixin
+
+def parse_options(basedir, command, args):
+    o = runner.Options()
+    o.parseOptions(["--node-directory", basedir, command] + args)
+    while hasattr(o, "subOptions"):
+        o = o.subOptions
+    return o
+
+class CLITestMixin(ReallyEqualMixin):
+    def do_cli(self, verb, *args, **kwargs):
+        precondition(not [True for arg in args if not isinstance(arg, str)],
+                     "arguments to do_cli must be strs -- convert using unicode_to_argv", args=args)
+
+        # client_num is used to execute client CLI commands on a specific client.
+        client_num = kwargs.get("client_num", 0)
+
+        nodeargs = [
+            "--node-directory", unicode_to_argv(self.get_clientdir(i=client_num)),
+            ]
+        argv = nodeargs + [verb] + list(args)
+        stdin = kwargs.get("stdin", "")
+        stdout, stderr = StringIO(), StringIO()
+        d = threads.deferToThread(runner.runner, argv, run_by_human=False,
+                                  stdin=StringIO(stdin),
+                                  stdout=stdout, stderr=stderr)
+        def _done(rc):
+            return rc, stdout.getvalue(), stderr.getvalue()
+        d.addCallback(_done)
+        return d
+
+    def skip_if_cannot_represent_filename(self, u):
+        precondition(isinstance(u, unicode))
+
+        enc = get_filesystem_encoding()
+        if not unicode_platform():
+            try:
+                u.encode(enc)
+            except UnicodeEncodeError:
+                raise unittest.SkipTest("A non-ASCII filename could not be encoded on this platform.")
diff --git a/src/allmydata/test/cli/test_backup.py b/src/allmydata/test/cli/test_backup.py
index 5d39e2825..bc154f1be 100644
--- a/src/allmydata/test/cli/test_backup.py
+++ b/src/allmydata/test/cli/test_backup.py
@@ -14,7 +14,7 @@ from allmydata.util.namespace import Namespace
 from allmydata.scripts import cli, backupdb
 from ..common_util import StallMixin
 from ..no_network import GridTestMixin
-from .test_cli import CLITestMixin, parse_options
+from .common import CLITestMixin, parse_options
 
 timeout = 480 # deep_check takes 360s on Zandr's linksys box, others take > 240s
 
diff --git a/src/allmydata/test/cli/test_check.py b/src/allmydata/test/cli/test_check.py
index 0eaf69099..6b1de52d3 100644
--- a/src/allmydata/test/cli/test_check.py
+++ b/src/allmydata/test/cli/test_check.py
@@ -10,7 +10,7 @@ from allmydata.mutable.publish import MutableData
 from allmydata.immutable import upload
 from allmydata.scripts import debug
 from ..no_network import GridTestMixin
-from .test_cli import CLITestMixin
+from .common import CLITestMixin
 
 timeout = 480 # deep_check takes 360s on Zandr's linksys box, others take > 240s
 
diff --git a/src/allmydata/test/cli/test_cli.py b/src/allmydata/test/cli/test_cli.py
index b1713f8e9..932fae373 100644
--- a/src/allmydata/test/cli/test_cli.py
+++ b/src/allmydata/test/cli/test_cli.py
@@ -31,55 +31,14 @@ from allmydata.scripts.common import DEFAULT_ALIAS, get_aliases, get_alias, \
 from allmydata.scripts import cli, debug, runner
 from ..common_util import ReallyEqualMixin
 from ..no_network import GridTestMixin
+from .common import CLITestMixin, parse_options
 from twisted.internet import threads # CLI tests use deferToThread
 from twisted.python import usage
 
-from allmydata.util.assertutil import precondition
-from allmydata.util.encodingutil import listdir_unicode, unicode_platform, \
-    get_io_encoding, get_filesystem_encoding, unicode_to_argv
+from allmydata.util.encodingutil import listdir_unicode, get_io_encoding
 
 timeout = 480 # deep_check takes 360s on Zandr's linksys box, others take > 240s
 
-def parse_options(basedir, command, args):
-    o = runner.Options()
-    o.parseOptions(["--node-directory", basedir, command] + args)
-    while hasattr(o, "subOptions"):
-        o = o.subOptions
-    return o
-
-class CLITestMixin(ReallyEqualMixin):
-    def do_cli(self, verb, *args, **kwargs):
-        precondition(not [True for arg in args if not isinstance(arg, str)],
-                     "arguments to do_cli must be strs -- convert using unicode_to_argv", args=args)
-
-        # client_num is used to execute client CLI commands on a specific client.
-        client_num = kwargs.get("client_num", 0)
-
-        nodeargs = [
-            "--node-directory", unicode_to_argv(self.get_clientdir(i=client_num)),
-            ]
-        argv = nodeargs + [verb] + list(args)
-        stdin = kwargs.get("stdin", "")
-        stdout, stderr = StringIO(), StringIO()
-        d = threads.deferToThread(runner.runner, argv, run_by_human=False,
-                                  stdin=StringIO(stdin),
-                                  stdout=stdout, stderr=stderr)
-        def _done(rc):
-            return rc, stdout.getvalue(), stderr.getvalue()
-        d.addCallback(_done)
-        return d
-
-    def skip_if_cannot_represent_filename(self, u):
-        precondition(isinstance(u, unicode))
-
-        enc = get_filesystem_encoding()
-        if not unicode_platform():
-            try:
-                u.encode(enc)
-            except UnicodeEncodeError:
-                raise unittest.SkipTest("A non-ASCII filename could not be encoded on this platform.")
-
-
 class CLI(CLITestMixin, unittest.TestCase):
     def _dump_cap(self, *args):
         config = debug.DumpCapOptions()
diff --git a/src/allmydata/test/cli/test_cp.py b/src/allmydata/test/cli/test_cp.py
index 50b538d2f..08666ae85 100644
--- a/src/allmydata/test/cli/test_cp.py
+++ b/src/allmydata/test/cli/test_cp.py
@@ -9,7 +9,7 @@ from allmydata.util.encodingutil import (quote_output, get_io_encoding,
                                          unicode_to_output, to_str)
 from allmydata.util.assertutil import _assert
 from ..no_network import GridTestMixin
-from .test_cli import CLITestMixin
+from .common import CLITestMixin
 
 timeout = 480 # deep_check takes 360s on Zandr's linksys box, others take > 240s
 
diff --git a/src/allmydata/test/cli/test_create_alias.py b/src/allmydata/test/cli/test_create_alias.py
index 8d2efe29f..5f4b70a54 100644
--- a/src/allmydata/test/cli/test_create_alias.py
+++ b/src/allmydata/test/cli/test_create_alias.py
@@ -6,7 +6,7 @@ from allmydata.scripts.common import get_aliases
 from allmydata.scripts import cli, runner
 from ..no_network import GridTestMixin
 from allmydata.util.encodingutil import quote_output, get_io_encoding
-from .test_cli import CLITestMixin
+from .common import CLITestMixin
 
 timeout = 480 # deep_check takes 360s on Zandr's linksys box, others take > 240s
 
diff --git a/src/allmydata/test/cli/test_list.py b/src/allmydata/test/cli/test_list.py
index 17b58a817..41ea48b2c 100644
--- a/src/allmydata/test/cli/test_list.py
+++ b/src/allmydata/test/cli/test_list.py
@@ -6,7 +6,7 @@ from allmydata.interfaces import MDMF_VERSION, SDMF_VERSION
 from allmydata.mutable.publish import MutableData
 from ..no_network import GridTestMixin
 from allmydata.util.encodingutil import quote_output, get_io_encoding
-from .test_cli import CLITestMixin
+from .common import CLITestMixin
 
 timeout = 480 # deep_check takes 360s on Zandr's linksys box, others take > 240s
 
diff --git a/src/allmydata/test/cli/test_magic_folder.py b/src/allmydata/test/cli/test_magic_folder.py
index a69b763f4..0c9009769 100644
--- a/src/allmydata/test/cli/test_magic_folder.py
+++ b/src/allmydata/test/cli/test_magic_folder.py
@@ -10,7 +10,7 @@ from allmydata.util.assertutil import precondition
 from allmydata.util import fileutil
 from allmydata.scripts.common import get_aliases
 from ..no_network import GridTestMixin
-from .test_cli import CLITestMixin
+from .common import CLITestMixin
 from allmydata.scripts import magic_folder_cli
 from allmydata.util.fileutil import abspath_expanduser_unicode
 from allmydata.util.encodingutil import unicode_to_argv
diff --git a/src/allmydata/test/cli/test_mv.py b/src/allmydata/test/cli/test_mv.py
index 1c72d79b4..40b1474c3 100644
--- a/src/allmydata/test/cli/test_mv.py
+++ b/src/allmydata/test/cli/test_mv.py
@@ -3,7 +3,7 @@ from twisted.trial import unittest
 from allmydata.util import fileutil
 from ..no_network import GridTestMixin
 from allmydata.scripts import tahoe_mv
-from .test_cli import CLITestMixin
+from .common import CLITestMixin
 
 timeout = 480 # deep_check takes 360s on Zandr's linksys box, others take > 240s
 
diff --git a/src/allmydata/test/cli/test_put.py b/src/allmydata/test/cli/test_put.py
index 1b3189f55..99b6182ac 100644
--- a/src/allmydata/test/cli/test_put.py
+++ b/src/allmydata/test/cli/test_put.py
@@ -8,7 +8,7 @@ from allmydata.scripts import cli
 from ..no_network import GridTestMixin
 from allmydata.util.encodingutil import get_io_encoding, unicode_to_argv
 from allmydata.util.fileutil import abspath_expanduser_unicode
-from .test_cli import CLITestMixin
+from .common import CLITestMixin
 
 timeout = 480 # deep_check takes 360s on Zandr's linksys box, others take > 240s
 

From 1877bd38b9c4e06187b7e95c2cd67870acfb0b14 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 9 Sep 2016 00:25:21 -0700
Subject: [PATCH 05/10] consolidate skip_if_cannot_represent_filename()

Remove duplicate copies of this utility, move it from a mixin/test-class
method to being a simple function in common_util.py
---
 src/allmydata/test/cli/common.py        | 15 +--------------
 src/allmydata/test/cli/test_cli.py      |  4 ++--
 src/allmydata/test/cli/test_cp.py       |  5 +++--
 src/allmydata/test/cli/test_put.py      |  3 ++-
 src/allmydata/test/common_util.py       | 11 +++++++++++
 src/allmydata/test/test_backupdb.py     | 18 ++++--------------
 src/allmydata/test/test_encodingutil.py | 12 ++----------
 7 files changed, 25 insertions(+), 43 deletions(-)

diff --git a/src/allmydata/test/cli/common.py b/src/allmydata/test/cli/common.py
index e41825df6..da9a708dc 100644
--- a/src/allmydata/test/cli/common.py
+++ b/src/allmydata/test/cli/common.py
@@ -1,10 +1,7 @@
 from cStringIO import StringIO
-from twisted.trial import unittest
 from twisted.internet import threads # CLI tests use deferToThread
 from ...util.assertutil import precondition
-from ...util.encodingutil import (unicode_platform,
-                                  get_filesystem_encoding,
-                                  unicode_to_argv)
+from ...util.encodingutil import unicode_to_argv
 from ...scripts import runner
 from ..common_util import ReallyEqualMixin
 
@@ -36,13 +33,3 @@ class CLITestMixin(ReallyEqualMixin):
             return rc, stdout.getvalue(), stderr.getvalue()
         d.addCallback(_done)
         return d
-
-    def skip_if_cannot_represent_filename(self, u):
-        precondition(isinstance(u, unicode))
-
-        enc = get_filesystem_encoding()
-        if not unicode_platform():
-            try:
-                u.encode(enc)
-            except UnicodeEncodeError:
-                raise unittest.SkipTest("A non-ASCII filename could not be encoded on this platform.")
diff --git a/src/allmydata/test/cli/test_cli.py b/src/allmydata/test/cli/test_cli.py
index 932fae373..653a5c776 100644
--- a/src/allmydata/test/cli/test_cli.py
+++ b/src/allmydata/test/cli/test_cli.py
@@ -29,7 +29,7 @@ from allmydata.scripts.common import DEFAULT_ALIAS, get_aliases, get_alias, \
      DefaultAliasMarker
 
 from allmydata.scripts import cli, debug, runner
-from ..common_util import ReallyEqualMixin
+from ..common_util import ReallyEqualMixin, skip_if_cannot_represent_filename
 from ..no_network import GridTestMixin
 from .common import CLITestMixin, parse_options
 from twisted.internet import threads # CLI tests use deferToThread
@@ -493,7 +493,7 @@ class CLI(CLITestMixin, unittest.TestCase):
         filenames = [u'L\u00F4zane', u'Bern', u'Gen\u00E8ve']  # must be NFC
 
         for name in filenames:
-            self.skip_if_cannot_represent_filename(name)
+            skip_if_cannot_represent_filename(name)
 
         basedir = "cli/common/listdir_unicode_good"
         fileutil.make_dirs(basedir)
diff --git a/src/allmydata/test/cli/test_cp.py b/src/allmydata/test/cli/test_cp.py
index 08666ae85..20136e978 100644
--- a/src/allmydata/test/cli/test_cp.py
+++ b/src/allmydata/test/cli/test_cp.py
@@ -10,6 +10,7 @@ from allmydata.util.encodingutil import (quote_output, get_io_encoding,
 from allmydata.util.assertutil import _assert
 from ..no_network import GridTestMixin
 from .common import CLITestMixin
+from ..common_util import skip_if_cannot_represent_filename
 
 timeout = 480 # deep_check takes 360s on Zandr's linksys box, others take > 240s
 
@@ -30,7 +31,7 @@ class Cp(GridTestMixin, CLITestMixin, unittest.TestCase):
         except UnicodeEncodeError:
             raise unittest.SkipTest("A non-ASCII command argument could not be encoded on this platform.")
 
-        self.skip_if_cannot_represent_filename(fn1)
+        skip_if_cannot_represent_filename(fn1)
 
         self.set_up_grid(oneshare=True)
 
@@ -198,7 +199,7 @@ class Cp(GridTestMixin, CLITestMixin, unittest.TestCase):
         except UnicodeEncodeError:
             raise unittest.SkipTest("A non-ASCII command argument could not be encoded on this platform.")
 
-        self.skip_if_cannot_represent_filename(fn1)
+        skip_if_cannot_represent_filename(fn1)
 
         self.set_up_grid(oneshare=True)
 
diff --git a/src/allmydata/test/cli/test_put.py b/src/allmydata/test/cli/test_put.py
index 99b6182ac..c62bdab85 100644
--- a/src/allmydata/test/cli/test_put.py
+++ b/src/allmydata/test/cli/test_put.py
@@ -6,6 +6,7 @@ from allmydata.util import fileutil
 from allmydata.scripts.common import get_aliases
 from allmydata.scripts import cli
 from ..no_network import GridTestMixin
+from ..common_util import skip_if_cannot_represent_filename
 from allmydata.util.encodingutil import get_io_encoding, unicode_to_argv
 from allmydata.util.fileutil import abspath_expanduser_unicode
 from .common import CLITestMixin
@@ -427,7 +428,7 @@ class Put(GridTestMixin, CLITestMixin, unittest.TestCase):
         except UnicodeEncodeError:
             raise unittest.SkipTest("A non-ASCII command argument could not be encoded on this platform.")
 
-        self.skip_if_cannot_represent_filename(u"\u00E0 trier.txt")
+        skip_if_cannot_represent_filename(u"\u00E0 trier.txt")
 
         self.basedir = "cli/Put/immutable_from_file_unicode"
         self.set_up_grid(oneshare=True)
diff --git a/src/allmydata/test/common_util.py b/src/allmydata/test/common_util.py
index 9d6bdf3f4..92e105b4b 100644
--- a/src/allmydata/test/common_util.py
+++ b/src/allmydata/test/common_util.py
@@ -3,10 +3,21 @@ from random import randrange
 
 from twisted.internet import reactor, defer
 from twisted.python import failure
+from twisted.trial import unittest
 
 from allmydata.util import fileutil, log
+from ..util.assertutil import precondition
 from allmydata.util.encodingutil import unicode_platform, get_filesystem_encoding
 
+def skip_if_cannot_represent_filename(u):
+    precondition(isinstance(u, unicode))
+
+    enc = get_filesystem_encoding()
+    if not unicode_platform():
+        try:
+            u.encode(enc)
+        except UnicodeEncodeError:
+            raise unittest.SkipTest("A non-ASCII filename could not be encoded on this platform.")
 
 class DevNullDictionary(dict):
     def __setitem__(self, key, value):
diff --git a/src/allmydata/test/test_backupdb.py b/src/allmydata/test/test_backupdb.py
index 835e25315..979c20016 100644
--- a/src/allmydata/test/test_backupdb.py
+++ b/src/allmydata/test/test_backupdb.py
@@ -4,9 +4,9 @@ from StringIO import StringIO
 from twisted.trial import unittest
 
 from allmydata.util import fileutil
-from allmydata.util.encodingutil import listdir_unicode, get_filesystem_encoding, unicode_platform
-from allmydata.util.assertutil import precondition
+from allmydata.util.encodingutil import listdir_unicode
 from allmydata.scripts import backupdb
+from .common_util import skip_if_cannot_represent_filename
 
 class BackupDB(unittest.TestCase):
     def create(self, dbfile):
@@ -15,16 +15,6 @@ class BackupDB(unittest.TestCase):
         self.failUnless(bdb, "unable to create backupdb from %r" % (dbfile,))
         return bdb
 
-    def skip_if_cannot_represent_filename(self, u):
-        precondition(isinstance(u, unicode))
-
-        enc = get_filesystem_encoding()
-        if not unicode_platform():
-            try:
-                u.encode(enc)
-            except UnicodeEncodeError:
-                raise unittest.SkipTest("A non-ASCII filename could not be encoded on this platform.")
-
     def test_basic(self):
         self.basedir = basedir = os.path.join("backupdb", "create")
         fileutil.make_dirs(basedir)
@@ -222,8 +212,8 @@ class BackupDB(unittest.TestCase):
         self.failIf(r.was_created())
 
     def test_unicode(self):
-        self.skip_if_cannot_represent_filename(u"f\u00f6\u00f6.txt")
-        self.skip_if_cannot_represent_filename(u"b\u00e5r.txt")
+        skip_if_cannot_represent_filename(u"f\u00f6\u00f6.txt")
+        skip_if_cannot_represent_filename(u"b\u00e5r.txt")
 
         self.basedir = basedir = os.path.join("backupdb", "unicode")
         fileutil.make_dirs(basedir)
diff --git a/src/allmydata/test/test_encodingutil.py b/src/allmydata/test/test_encodingutil.py
index 0632bea03..a5152c3e0 100644
--- a/src/allmydata/test/test_encodingutil.py
+++ b/src/allmydata/test/test_encodingutil.py
@@ -71,7 +71,7 @@ from allmydata.util.encodingutil import argv_to_unicode, unicode_to_url, \
     get_io_encoding, get_filesystem_encoding, to_str, from_utf8_or_none, _reload, \
     to_filepath, extend_filepath, unicode_from_filepath, unicode_segments_from
 from allmydata.dirnode import normalize
-
+from .common_util import skip_if_cannot_represent_filename
 from twisted.python import usage
 
 
@@ -265,16 +265,8 @@ class StdlibUnicode(unittest.TestCase):
     """This mainly tests that some of the stdlib functions support Unicode paths, but also that
     listdir_unicode works for valid filenames."""
 
-    def skip_if_cannot_represent_filename(self, u):
-        enc = get_filesystem_encoding()
-        if not unicode_platform():
-            try:
-                u.encode(enc)
-            except UnicodeEncodeError:
-                raise unittest.SkipTest("A non-ASCII filename could not be encoded on this platform.")
-
     def test_mkdir_open_exists_abspath_listdir_expanduser(self):
-        self.skip_if_cannot_represent_filename(lumiere_nfc)
+        skip_if_cannot_represent_filename(lumiere_nfc)
 
         try:
             os.mkdir(lumiere_nfc)

From 442468f599085c493e9b958eeab40530715fa2a6 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 9 Sep 2016 10:40:39 -0700
Subject: [PATCH 06/10] do_cli(): split out run_cli()

The main part of CLITestMixin.do_cli() was split into a standalone
function named run_cli(), leaving do_cli() as a method which includes a
nodedir in the arguments (for use by GridTestMixin tests which do a lot
of CLI operations against one of their client nodes, for which adding
the extra --nodedir argument would be ugly).
---
 src/allmydata/test/cli/common.py | 36 +++++++++++++++++---------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/src/allmydata/test/cli/common.py b/src/allmydata/test/cli/common.py
index da9a708dc..87b48d6da 100644
--- a/src/allmydata/test/cli/common.py
+++ b/src/allmydata/test/cli/common.py
@@ -14,22 +14,24 @@ def parse_options(basedir, command, args):
 
 class CLITestMixin(ReallyEqualMixin):
     def do_cli(self, verb, *args, **kwargs):
-        precondition(not [True for arg in args if not isinstance(arg, str)],
-                     "arguments to do_cli must be strs -- convert using unicode_to_argv", args=args)
-
-        # client_num is used to execute client CLI commands on a specific client.
+        # client_num is used to execute client CLI commands on a specific
+        # client.
         client_num = kwargs.get("client_num", 0)
+        client_dir = unicode_to_argv(self.get_clientdir(i=client_num))
+        nodeargs = [ "--node-directory", client_dir ]
+        return run_cli(verb, nodeargs=nodeargs, *args, **kwargs)
 
-        nodeargs = [
-            "--node-directory", unicode_to_argv(self.get_clientdir(i=client_num)),
-            ]
-        argv = nodeargs + [verb] + list(args)
-        stdin = kwargs.get("stdin", "")
-        stdout, stderr = StringIO(), StringIO()
-        d = threads.deferToThread(runner.runner, argv, run_by_human=False,
-                                  stdin=StringIO(stdin),
-                                  stdout=stdout, stderr=stderr)
-        def _done(rc):
-            return rc, stdout.getvalue(), stderr.getvalue()
-        d.addCallback(_done)
-        return d
+def run_cli(verb, *args, **kwargs):
+    precondition(not [True for arg in args if not isinstance(arg, str)],
+                 "arguments to do_cli must be strs -- convert using unicode_to_argv", args=args)
+    nodeargs = kwargs.get("nodeargs", [])
+    argv = nodeargs + [verb] + list(args)
+    stdin = kwargs.get("stdin", "")
+    stdout, stderr = StringIO(), StringIO()
+    d = threads.deferToThread(runner.runner, argv, run_by_human=False,
+                              stdin=StringIO(stdin),
+                              stdout=stdout, stderr=stderr)
+    def _done(rc):
+        return rc, stdout.getvalue(), stderr.getvalue()
+    d.addCallback(_done)
+    return d

From 07e4c491f558a86ac3d9decdfe94570f5839a922 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 9 Sep 2016 10:25:11 -0700
Subject: [PATCH 07/10] move run_cli() up to test/common_util.py

---
 src/allmydata/test/cli/common.py  | 20 +-------------------
 src/allmydata/test/common_util.py | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/src/allmydata/test/cli/common.py b/src/allmydata/test/cli/common.py
index 87b48d6da..852dce52c 100644
--- a/src/allmydata/test/cli/common.py
+++ b/src/allmydata/test/cli/common.py
@@ -1,9 +1,6 @@
-from cStringIO import StringIO
-from twisted.internet import threads # CLI tests use deferToThread
-from ...util.assertutil import precondition
 from ...util.encodingutil import unicode_to_argv
 from ...scripts import runner
-from ..common_util import ReallyEqualMixin
+from ..common_util import ReallyEqualMixin, run_cli
 
 def parse_options(basedir, command, args):
     o = runner.Options()
@@ -20,18 +17,3 @@ class CLITestMixin(ReallyEqualMixin):
         client_dir = unicode_to_argv(self.get_clientdir(i=client_num))
         nodeargs = [ "--node-directory", client_dir ]
         return run_cli(verb, nodeargs=nodeargs, *args, **kwargs)
-
-def run_cli(verb, *args, **kwargs):
-    precondition(not [True for arg in args if not isinstance(arg, str)],
-                 "arguments to do_cli must be strs -- convert using unicode_to_argv", args=args)
-    nodeargs = kwargs.get("nodeargs", [])
-    argv = nodeargs + [verb] + list(args)
-    stdin = kwargs.get("stdin", "")
-    stdout, stderr = StringIO(), StringIO()
-    d = threads.deferToThread(runner.runner, argv, run_by_human=False,
-                              stdin=StringIO(stdin),
-                              stdout=stdout, stderr=stderr)
-    def _done(rc):
-        return rc, stdout.getvalue(), stderr.getvalue()
-    d.addCallback(_done)
-    return d
diff --git a/src/allmydata/test/common_util.py b/src/allmydata/test/common_util.py
index 92e105b4b..cd0f6b123 100644
--- a/src/allmydata/test/common_util.py
+++ b/src/allmydata/test/common_util.py
@@ -1,13 +1,15 @@
 import os, signal, sys, time
 from random import randrange
+from cStringIO import StringIO
 
-from twisted.internet import reactor, defer
+from twisted.internet import reactor, defer, threads
 from twisted.python import failure
 from twisted.trial import unittest
 
 from allmydata.util import fileutil, log
 from ..util.assertutil import precondition
 from allmydata.util.encodingutil import unicode_platform, get_filesystem_encoding
+from ..scripts import runner
 
 def skip_if_cannot_represent_filename(u):
     precondition(isinstance(u, unicode))
@@ -19,6 +21,21 @@ def skip_if_cannot_represent_filename(u):
         except UnicodeEncodeError:
             raise unittest.SkipTest("A non-ASCII filename could not be encoded on this platform.")
 
+def run_cli(verb, *args, **kwargs):
+    precondition(not [True for arg in args if not isinstance(arg, str)],
+                 "arguments to do_cli must be strs -- convert using unicode_to_argv", args=args)
+    nodeargs = kwargs.get("nodeargs", [])
+    argv = nodeargs + [verb] + list(args)
+    stdin = kwargs.get("stdin", "")
+    stdout, stderr = StringIO(), StringIO()
+    d = threads.deferToThread(runner.runner, argv, run_by_human=False,
+                              stdin=StringIO(stdin),
+                              stdout=stdout, stderr=stderr)
+    def _done(rc):
+        return rc, stdout.getvalue(), stderr.getvalue()
+    d.addCallback(_done)
+    return d
+
 class DevNullDictionary(dict):
     def __setitem__(self, key, value):
         return

From 7193bff48bb7c8a1c364f18480d28f0534372013 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 9 Sep 2016 10:41:20 -0700
Subject: [PATCH 08/10] tests: use shared run_cli()/do_cli()

A couple of test classes which defined their own flavors were changed to
use the common one.
---
 src/allmydata/test/cli/test_cli.py    | 24 ++-----
 src/allmydata/test/cli/test_create.py | 29 +++++----
 src/allmydata/test/test_deepcheck.py  | 91 ++++++---------------------
 src/allmydata/test/test_runner.py     | 18 +++---
 src/allmydata/test/test_system.py     | 52 +++++----------
 5 files changed, 62 insertions(+), 152 deletions(-)

diff --git a/src/allmydata/test/cli/test_cli.py b/src/allmydata/test/cli/test_cli.py
index 653a5c776..dafbf6329 100644
--- a/src/allmydata/test/cli/test_cli.py
+++ b/src/allmydata/test/cli/test_cli.py
@@ -29,10 +29,10 @@ from allmydata.scripts.common import DEFAULT_ALIAS, get_aliases, get_alias, \
      DefaultAliasMarker
 
 from allmydata.scripts import cli, debug, runner
-from ..common_util import ReallyEqualMixin, skip_if_cannot_represent_filename
+from ..common_util import (ReallyEqualMixin, skip_if_cannot_represent_filename,
+                           run_cli)
 from ..no_network import GridTestMixin
 from .common import CLITestMixin, parse_options
-from twisted.internet import threads # CLI tests use deferToThread
 from twisted.python import usage
 
 from allmydata.util.encodingutil import listdir_unicode, get_io_encoding
@@ -705,21 +705,9 @@ class Ln(GridTestMixin, CLITestMixin, unittest.TestCase):
 
 
 class Admin(unittest.TestCase):
-    def do_cli(self, *args, **kwargs):
-        argv = list(args)
-        stdin = kwargs.get("stdin", "")
-        stdout, stderr = StringIO(), StringIO()
-        d = threads.deferToThread(runner.runner, argv, run_by_human=False,
-                                  stdin=StringIO(stdin),
-                                  stdout=stdout, stderr=stderr)
-        def _done(res):
-            return stdout.getvalue(), stderr.getvalue()
-        d.addCallback(_done)
-        return d
-
     def test_generate_keypair(self):
-        d = self.do_cli("admin", "generate-keypair")
-        def _done( (stdout, stderr) ):
+        d = run_cli("admin", "generate-keypair")
+        def _done( (rc, stdout, stderr) ):
             lines = [line.strip() for line in stdout.splitlines()]
             privkey_bits = lines[0].split()
             pubkey_bits = lines[1].split()
@@ -738,8 +726,8 @@ class Admin(unittest.TestCase):
 
     def test_derive_pubkey(self):
         priv1,pub1 = keyutil.make_keypair()
-        d = self.do_cli("admin", "derive-pubkey", priv1)
-        def _done( (stdout, stderr) ):
+        d = run_cli("admin", "derive-pubkey", priv1)
+        def _done( (rc, stdout, stderr) ):
             lines = stdout.split("\n")
             privkey_line = lines[0].strip()
             pubkey_line = lines[1].strip()
diff --git a/src/allmydata/test/cli/test_create.py b/src/allmydata/test/cli/test_create.py
index 9e06fa255..c10271a8b 100644
--- a/src/allmydata/test/cli/test_create.py
+++ b/src/allmydata/test/cli/test_create.py
@@ -1,54 +1,53 @@
 import os
-from StringIO import StringIO
 from twisted.trial import unittest
-from allmydata.scripts import runner
+from twisted.internet import defer
 from allmydata.util import configutil
+from ..common_util import run_cli
 
 class Config(unittest.TestCase):
-    def do_cli(self, *args):
-        argv = list(args)
-        stdout, stderr = StringIO(), StringIO()
-        rc = runner.runner(argv, run_by_human=False,
-                           stdout=stdout, stderr=stderr)
-        return rc, stdout.getvalue(), stderr.getvalue()
-
     def read_config(self, basedir):
         tahoe_cfg = os.path.join(basedir, "tahoe.cfg")
         config = configutil.get_config(tahoe_cfg)
         return config
 
+    @defer.inlineCallbacks
     def test_client(self):
         basedir = self.mktemp()
-        rc, out, err = self.do_cli("create-client", basedir)
+        rc, out, err = yield run_cli("create-client", basedir)
         cfg = self.read_config(basedir)
         self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), True)
 
+    @defer.inlineCallbacks
     def test_client_hide_ip(self):
         basedir = self.mktemp()
-        rc, out, err = self.do_cli("create-client", "--hide-ip", basedir)
+        rc, out, err = yield run_cli("create-client", "--hide-ip", basedir)
         cfg = self.read_config(basedir)
         self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), False)
 
+    @defer.inlineCallbacks
     def test_node(self):
         basedir = self.mktemp()
-        rc, out, err = self.do_cli("create-node", basedir)
+        rc, out, err = yield run_cli("create-node", basedir)
         cfg = self.read_config(basedir)
         self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), True)
 
+    @defer.inlineCallbacks
     def test_node_hide_ip(self):
         basedir = self.mktemp()
-        rc, out, err = self.do_cli("create-node", "--hide-ip", basedir)
+        rc, out, err = yield run_cli("create-node", "--hide-ip", basedir)
         cfg = self.read_config(basedir)
         self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), False)
 
+    @defer.inlineCallbacks
     def test_introducer(self):
         basedir = self.mktemp()
-        rc, out, err = self.do_cli("create-introducer", basedir)
+        rc, out, err = yield run_cli("create-introducer", basedir)
         cfg = self.read_config(basedir)
         self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), True)
 
+    @defer.inlineCallbacks
     def test_introducer_hide_ip(self):
         basedir = self.mktemp()
-        rc, out, err = self.do_cli("create-introducer", "--hide-ip", basedir)
+        rc, out, err = yield run_cli("create-introducer", "--hide-ip", basedir)
         cfg = self.read_config(basedir)
         self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), False)
diff --git a/src/allmydata/test/test_deepcheck.py b/src/allmydata/test/test_deepcheck.py
index fd9db7ec1..725f43e84 100644
--- a/src/allmydata/test/test_deepcheck.py
+++ b/src/allmydata/test/test_deepcheck.py
@@ -1,15 +1,12 @@
 
 import os, simplejson, urllib
-from cStringIO import StringIO
 from twisted.trial import unittest
 from twisted.internet import defer
-from twisted.internet import threads # CLI tests use deferToThread
 from allmydata.immutable import upload
 from allmydata.mutable.common import UnrecoverableFileError
 from allmydata.mutable.publish import MutableData
 from allmydata.util import idlib
 from allmydata.util import base32
-from allmydata.scripts import runner
 from allmydata.interfaces import ICheckResults, ICheckAndRepairResults, \
      IDeepCheckResults, IDeepCheckAndRepairResults
 from allmydata.monitor import Monitor, OperationCancelledError
@@ -18,19 +15,13 @@ from twisted.web.client import getPage
 
 from allmydata.test.common import ErrorMixin, _corrupt_mutable_share_data, \
      ShouldFailMixin
-from allmydata.test.common_util import StallMixin
+from .common_util import StallMixin, run_cli
 from allmydata.test.no_network import GridTestMixin
+from .cli.common import CLITestMixin
 
 timeout = 2400 # One of these took 1046.091s on Zandr's ARM box.
 
 class MutableChecker(GridTestMixin, unittest.TestCase, ErrorMixin):
-    def _run_cli(self, argv):
-        stdout, stderr = StringIO(), StringIO()
-        # this can only do synchronous operations
-        assert argv[0] == "debug"
-        runner.runner(argv, run_by_human=False, stdout=stdout, stderr=stderr)
-        return stdout.getvalue()
-
     def test_good(self):
         self.basedir = "deepcheck/MutableChecker/good"
         self.set_up_grid()
@@ -130,7 +121,8 @@ class MutableChecker(GridTestMixin, unittest.TestCase, ErrorMixin):
         return d
 
 
-class DeepCheckBase(GridTestMixin, ErrorMixin, StallMixin, ShouldFailMixin):
+class DeepCheckBase(GridTestMixin, ErrorMixin, StallMixin, ShouldFailMixin,
+                    CLITestMixin):
 
     def web_json(self, n, **kwargs):
         kwargs["output"] = "json"
@@ -727,17 +719,6 @@ class DeepCheckWebGood(DeepCheckBase, unittest.TestCase):
 
         return d
 
-    def _run_cli(self, argv, stdin=""):
-        #print "CLI:", argv
-        stdout, stderr = StringIO(), StringIO()
-        d = threads.deferToThread(runner.runner, argv, run_by_human=False,
-                                  stdin=StringIO(stdin),
-                                  stdout=stdout, stderr=stderr)
-        def _done(res):
-            return stdout.getvalue(), stderr.getvalue()
-        d.addCallback(_done)
-        return d
-
     def do_test_cli_good(self, ignored):
         d = defer.succeed(None)
         d.addCallback(lambda ign: self.do_cli_manifest_stream1())
@@ -757,11 +738,8 @@ class DeepCheckWebGood(DeepCheckBase, unittest.TestCase):
         self.failUnless(base32.b2a(self.large.get_storage_index()) in lines)
 
     def do_cli_manifest_stream1(self):
-        basedir = self.get_clientdir(0)
-        d = self._run_cli(["--node-directory", basedir,
-                           "manifest",
-                           self.root_uri])
-        def _check((out,err)):
+        d = self.do_cli("manifest", self.root_uri)
+        def _check((rc,out,err)):
             self.failUnlessEqual(err, "")
             lines = [l for l in out.split("\n") if l]
             self.failUnlessEqual(len(lines), 8)
@@ -785,12 +763,8 @@ class DeepCheckWebGood(DeepCheckBase, unittest.TestCase):
         return d
 
     def do_cli_manifest_stream2(self):
-        basedir = self.get_clientdir(0)
-        d = self._run_cli(["--node-directory", basedir,
-                           "manifest",
-                           "--raw",
-                           self.root_uri])
-        def _check((out,err)):
+        d = self.do_cli("manifest", "--raw", self.root_uri)
+        def _check((rc,out,err)):
             self.failUnlessEqual(err, "")
             # this should be the same as the POST t=stream-manifest output
             self._check_streamed_manifest(out)
@@ -798,24 +772,16 @@ class DeepCheckWebGood(DeepCheckBase, unittest.TestCase):
         return d
 
     def do_cli_manifest_stream3(self):
-        basedir = self.get_clientdir(0)
-        d = self._run_cli(["--node-directory", basedir,
-                           "manifest",
-                           "--storage-index",
-                           self.root_uri])
-        def _check((out,err)):
+        d = self.do_cli("manifest", "--storage-index", self.root_uri)
+        def _check((rc,out,err)):
             self.failUnlessEqual(err, "")
             self._check_manifest_storage_index(out)
         d.addCallback(_check)
         return d
 
     def do_cli_manifest_stream4(self):
-        basedir = self.get_clientdir(0)
-        d = self._run_cli(["--node-directory", basedir,
-                           "manifest",
-                           "--verify-cap",
-                           self.root_uri])
-        def _check((out,err)):
+        d = self.do_cli("manifest", "--verify-cap", self.root_uri)
+        def _check((rc,out,err)):
             self.failUnlessEqual(err, "")
             lines = [l for l in out.split("\n") if l]
             self.failUnlessEqual(len(lines), 3)
@@ -826,12 +792,8 @@ class DeepCheckWebGood(DeepCheckBase, unittest.TestCase):
         return d
 
     def do_cli_manifest_stream5(self):
-        basedir = self.get_clientdir(0)
-        d = self._run_cli(["--node-directory", basedir,
-                           "manifest",
-                           "--repair-cap",
-                           self.root_uri])
-        def _check((out,err)):
+        d = self.do_cli("manifest", "--repair-cap", self.root_uri)
+        def _check((rc,out,err)):
             self.failUnlessEqual(err, "")
             lines = [l for l in out.split("\n") if l]
             self.failUnlessEqual(len(lines), 3)
@@ -842,11 +804,8 @@ class DeepCheckWebGood(DeepCheckBase, unittest.TestCase):
         return d
 
     def do_cli_stats1(self):
-        basedir = self.get_clientdir(0)
-        d = self._run_cli(["--node-directory", basedir,
-                           "stats",
-                           self.root_uri])
-        def _check3((out,err)):
+        d = self.do_cli("stats", self.root_uri)
+        def _check3((rc,out,err)):
             lines = [l.strip() for l in out.split("\n") if l]
             self.failUnless("count-immutable-files: 1" in lines)
             self.failUnless("count-mutable-files: 1" in lines)
@@ -862,12 +821,8 @@ class DeepCheckWebGood(DeepCheckBase, unittest.TestCase):
         return d
 
     def do_cli_stats2(self):
-        basedir = self.get_clientdir(0)
-        d = self._run_cli(["--node-directory", basedir,
-                           "stats",
-                           "--raw",
-                           self.root_uri])
-        def _check4((out,err)):
+        d = self.do_cli("stats", "--raw", self.root_uri)
+        def _check4((rc,out,err)):
             data = simplejson.loads(out)
             self.failUnlessEqual(data["count-immutable-files"], 1)
             self.failUnlessEqual(data["count-immutable-files"], 1)
@@ -983,20 +938,14 @@ class DeepCheckWebBad(DeepCheckBase, unittest.TestCase):
 
         return d
 
-    def _run_cli(self, argv):
-        stdout, stderr = StringIO(), StringIO()
-        # this can only do synchronous operations
-        assert argv[0] == "debug"
-        runner.runner(argv, run_by_human=False, stdout=stdout, stderr=stderr)
-        return stdout.getvalue()
-
     def _delete_some_shares(self, node):
         self.delete_shares_numbered(node.get_uri(), [0,1])
 
+    @defer.inlineCallbacks
     def _corrupt_some_shares(self, node):
         for (shnum, serverid, sharefile) in self.find_uri_shares(node.get_uri()):
             if shnum in (0,1):
-                self._run_cli(["debug", "corrupt-share", sharefile])
+                yield run_cli("debug", "corrupt-share", sharefile)
 
     def _delete_most_shares(self, node):
         self.delete_shares_numbered(node.get_uri(), range(1,10))
diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index b60d83689..e75713394 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -1,5 +1,4 @@
 import os.path, re, sys, subprocess
-from cStringIO import StringIO
 
 from twisted.trial import unittest
 
@@ -15,6 +14,7 @@ from allmydata.client import Client
 from allmydata.test import common_util
 import allmydata
 from allmydata import __appname__
+from .common_util import run_cli
 
 
 timeout = 240
@@ -180,11 +180,7 @@ class CreateNode(unittest.TestCase):
         fileutil.make_dirs(basedir)
         return basedir
 
-    def run_tahoe(self, argv):
-        out,err = StringIO(), StringIO()
-        rc = runner.runner(argv, stdout=out, stderr=err)
-        return rc, out.getvalue(), err.getvalue()
-
+    @inlineCallbacks
     def do_create(self, kind, *args):
         basedir = self.workdir("test_" + kind)
         command = "create-" + kind
@@ -193,7 +189,7 @@ class CreateNode(unittest.TestCase):
 
         n1 = os.path.join(basedir, command + "-n1")
         argv = ["--quiet", command, "--basedir", n1] + list(args)
-        rc, out, err = self.run_tahoe(argv)
+        rc, out, err = yield run_cli(*argv)
         self.failUnlessEqual(err, "")
         self.failUnlessEqual(out, "")
         self.failUnlessEqual(rc, 0)
@@ -213,7 +209,7 @@ class CreateNode(unittest.TestCase):
                 self.failUnless("\nreserved_space = 1G\n" in content)
 
         # creating the node a second time should be rejected
-        rc, out, err = self.run_tahoe(argv)
+        rc, out, err = yield run_cli(*argv)
         self.failIfEqual(rc, 0, str((out, err, rc)))
         self.failUnlessEqual(out, "")
         self.failUnless("is not empty." in err)
@@ -226,7 +222,7 @@ class CreateNode(unittest.TestCase):
         # test that the non --basedir form works too
         n2 = os.path.join(basedir, command + "-n2")
         argv = ["--quiet", command] + list(args) + [n2]
-        rc, out, err = self.run_tahoe(argv)
+        rc, out, err = yield run_cli(*argv)
         self.failUnlessEqual(err, "")
         self.failUnlessEqual(out, "")
         self.failUnlessEqual(rc, 0)
@@ -236,7 +232,7 @@ class CreateNode(unittest.TestCase):
         # test the --node-directory form
         n3 = os.path.join(basedir, command + "-n3")
         argv = ["--quiet", "--node-directory", n3, command] + list(args)
-        rc, out, err = self.run_tahoe(argv)
+        rc, out, err = yield run_cli(*argv)
         self.failUnlessEqual(err, "")
         self.failUnlessEqual(out, "")
         self.failUnlessEqual(rc, 0)
@@ -247,7 +243,7 @@ class CreateNode(unittest.TestCase):
             # test that the output (without --quiet) includes the base directory
             n4 = os.path.join(basedir, command + "-n4")
             argv = [command] + list(args) + [n4]
-            rc, out, err = self.run_tahoe(argv)
+            rc, out, err = yield run_cli(*argv)
             self.failUnlessEqual(err, "")
             self.failUnlessIn(" created in ", out)
             self.failUnlessIn(n4, out)
diff --git a/src/allmydata/test/test_system.py b/src/allmydata/test/test_system.py
index 57af07139..3d4004e6e 100644
--- a/src/allmydata/test/test_system.py
+++ b/src/allmydata/test/test_system.py
@@ -1,10 +1,8 @@
 
 import os, re, sys, time, simplejson
-from cStringIO import StringIO
 
 from twisted.trial import unittest
 from twisted.internet import defer
-from twisted.internet import threads # CLI tests use deferToThread
 from twisted.application import service
 
 import allmydata
@@ -20,7 +18,6 @@ from allmydata.util import log, base32
 from allmydata.util.encodingutil import quote_output, unicode_to_argv
 from allmydata.util.fileutil import abspath_expanduser_unicode
 from allmydata.util.consumer import MemoryConsumer, download_to_data
-from allmydata.scripts import runner
 from allmydata.stats import StatsGathererService
 from allmydata.interfaces import IDirectoryNode, IFileNode, \
      NoSuchChildError, NoSharesError
@@ -39,6 +36,7 @@ from .common import TEST_RSA_KEY_SIZE
 # TODO: move this to common or common_util
 from allmydata.test.test_runner import RunBinTahoeMixin
 from . import common_util as testutil
+from .common_util import run_cli
 
 LARGE_DATA = """
 This is some data to publish to the remote grid.., which needs to be large
@@ -1064,6 +1062,7 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase):
             return d1
         d.addCallback(_create_mutable)
 
+        @defer.inlineCallbacks
         def _test_debug(res):
             # find a share. It is important to run this while there is only
             # one slot in the grid.
@@ -1073,11 +1072,8 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase):
                     % filename)
             log.msg(" for clients[%d]" % client_num)
 
-            out,err = StringIO(), StringIO()
-            rc = runner.runner(["debug", "dump-share", "--offsets",
-                                filename],
-                               stdout=out, stderr=err)
-            output = out.getvalue()
+            rc,output,err = yield run_cli("debug", "dump-share", "--offsets",
+                                          filename)
             self.failUnlessEqual(rc, 0)
             try:
                 self.failUnless("Mutable slot found:\n" in output)
@@ -1862,6 +1858,7 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase):
 
         return d
 
+    @defer.inlineCallbacks
     def _test_runner(self, res):
         # exercise some of the diagnostic tools in runner.py
 
@@ -1887,11 +1884,8 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase):
                       % self.basedir)
         log.msg("test_system.SystemTest._test_runner using %r" % filename)
 
-        out,err = StringIO(), StringIO()
-        rc = runner.runner(["debug", "dump-share", "--offsets",
-                            unicode_to_argv(filename)],
-                           stdout=out, stderr=err)
-        output = out.getvalue()
+        rc,output,err = yield run_cli("debug", "dump-share", "--offsets",
+                                      unicode_to_argv(filename))
         self.failUnlessEqual(rc, 0)
 
         # we only upload a single file, so we can assert some things about
@@ -1917,23 +1911,18 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase):
         sharedir, shnum = os.path.split(filename)
         storagedir, storage_index_s = os.path.split(sharedir)
         storage_index_s = str(storage_index_s)
-        out,err = StringIO(), StringIO()
         nodedirs = [self.getdir("client%d" % i) for i in range(self.numclients)]
-        cmd = ["debug", "find-shares", storage_index_s] + nodedirs
-        rc = runner.runner(cmd, stdout=out, stderr=err)
+        rc,out,err = yield run_cli("debug", "find-shares", storage_index_s,
+                                   *nodedirs)
         self.failUnlessEqual(rc, 0)
-        out.seek(0)
-        sharefiles = [sfn.strip() for sfn in out.readlines()]
+        sharefiles = [sfn.strip() for sfn in out.splitlines()]
         self.failUnlessEqual(len(sharefiles), 10)
 
         # also exercise the 'catalog-shares' tool
-        out,err = StringIO(), StringIO()
         nodedirs = [self.getdir("client%d" % i) for i in range(self.numclients)]
-        cmd = ["debug", "catalog-shares"] + nodedirs
-        rc = runner.runner(cmd, stdout=out, stderr=err)
+        rc,out,err = yield run_cli("debug", "catalog-shares", *nodedirs)
         self.failUnlessEqual(rc, 0)
-        out.seek(0)
-        descriptions = [sfn.strip() for sfn in out.readlines()]
+        descriptions = [sfn.strip() for sfn in out.splitlines()]
         self.failUnlessEqual(len(descriptions), 30)
         matching = [line
                     for line in descriptions
@@ -1981,10 +1970,10 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase):
         f.write(private_uri)
         f.close()
 
+        @defer.inlineCallbacks
         def run(ignored, verb, *args, **kwargs):
-            stdin = kwargs.get("stdin", "")
-            newargs = nodeargs + [verb] + list(args)
-            return self._run_cli(newargs, stdin=stdin)
+            rc,out,err = yield run_cli(verb, *args, nodeargs=nodeargs, **kwargs)
+            defer.returnValue((out,err))
 
         def _check_ls((out,err), expected_children, unexpected_children=[]):
             self.failUnlessEqual(err, "")
@@ -2345,17 +2334,6 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase):
         d.addCallback(_check_ls)
         return d
 
-    def _run_cli(self, argv, stdin=""):
-        #print "CLI:", argv
-        stdout, stderr = StringIO(), StringIO()
-        d = threads.deferToThread(runner.runner, argv, run_by_human=False,
-                                  stdin=StringIO(stdin),
-                                  stdout=stdout, stderr=stderr)
-        def _done(res):
-            return stdout.getvalue(), stderr.getvalue()
-        d.addCallback(_done)
-        return d
-
     def _test_checker(self, res):
         ut = upload.Data("too big to be literal" * 200, convergence=None)
         d = self._personal_node.add_file(u"big file", ut)

From 97c29a3c0b91d3304755fc908e39585b051e7bdc Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 9 Sep 2016 10:06:00 -0700
Subject: [PATCH 09/10] test_runner: factor out parse_cli() helper

---
 src/allmydata/test/common_util.py | 12 +++++++
 src/allmydata/test/test_runner.py | 60 +++++++++++++------------------
 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/src/allmydata/test/common_util.py b/src/allmydata/test/common_util.py
index cd0f6b123..af152b2c3 100644
--- a/src/allmydata/test/common_util.py
+++ b/src/allmydata/test/common_util.py
@@ -36,6 +36,18 @@ def run_cli(verb, *args, **kwargs):
     d.addCallback(_done)
     return d
 
+def parse_cli(*argv):
+    # This parses the CLI options (synchronously), and throws
+    # usage.UsageError if something went wrong.
+
+    # As a temporary side-effect, if the arguments can be parsed correctly,
+    # it also executes the command. This side-effect will be removed when
+    # runner.py is refactored. After the refactoring, this will return the
+    # Options object, and this method can be used for success testing, not
+    # just failure testing.
+    runner.runner(argv, run_by_human=False)
+    assert False, "eek, I can't be used for success testing yet"
+
 class DevNullDictionary(dict):
     def __setitem__(self, key, value):
         return
diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index e75713394..e8468ca46 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -9,12 +9,11 @@ from twisted.internet.defer import inlineCallbacks, returnValue
 from allmydata.util import fileutil, pollmixin
 from allmydata.util.encodingutil import unicode_to_argv, unicode_to_output, \
     get_filesystem_encoding
-from allmydata.scripts import runner
 from allmydata.client import Client
 from allmydata.test import common_util
 import allmydata
 from allmydata import __appname__
-from .common_util import run_cli
+from .common_util import parse_cli, run_cli
 
 
 timeout = 240
@@ -253,18 +252,14 @@ class CreateNode(unittest.TestCase):
             self.failUnless(os.path.exists(os.path.join(n4, tac)))
 
         # make sure it rejects too many arguments
-        argv = [command, "basedir", "extraarg"]
-        self.failUnlessRaises(usage.UsageError,
-                              runner.runner, argv,
-                              run_by_human=False)
+        self.failUnlessRaises(usage.UsageError, parse_cli,
+                              command, "basedir", "extraarg")
 
         # when creating a non-client, there is no default for the basedir
         if not is_client:
             argv = [command]
-            self.failUnlessRaises(usage.UsageError,
-                                  runner.runner, argv,
-                                  run_by_human=False)
-
+            self.failUnlessRaises(usage.UsageError, parse_cli,
+                                  command)
 
     def test_node(self):
         self.do_create("node")
@@ -281,49 +276,42 @@ class CreateNode(unittest.TestCase):
 
     def test_subcommands(self):
         # no arguments should trigger a command listing, via UsageError
-        self.failUnlessRaises(usage.UsageError,
-                              runner.runner,
-                              [],
-                              run_by_human=False)
+        self.failUnlessRaises(usage.UsageError, parse_cli,
+                              )
 
+    @inlineCallbacks
     def test_stats_gatherer_good_args(self):
-        rc = runner.runner(["create-stats-gatherer", "--hostname=foo",
-                            self.mktemp()])
+        rc,out,err = yield run_cli("create-stats-gatherer", "--hostname=foo",
+                                   self.mktemp())
         self.assertEqual(rc, 0)
-        rc = runner.runner(["create-stats-gatherer", "--location=tcp:foo:1234",
-                            "--port=tcp:1234", self.mktemp()])
+        rc,out,err = yield run_cli("create-stats-gatherer",
+                                   "--location=tcp:foo:1234",
+                                   "--port=tcp:1234", self.mktemp())
         self.assertEqual(rc, 0)
 
+
     def test_stats_gatherer_bad_args(self):
+        def _test(args):
+            argv = args.split()
+            self.assertRaises(usage.UsageError, parse_cli, *argv)
+
         # missing hostname/location/port
-        argv = "create-stats-gatherer D"
-        self.assertRaises(usage.UsageError, runner.runner, argv.split(),
-                          run_by_human=False)
+        _test("create-stats-gatherer D")
 
         # missing port
-        argv = "create-stats-gatherer --location=foo D"
-        self.assertRaises(usage.UsageError, runner.runner, argv.split(),
-                          run_by_human=False)
+        _test("create-stats-gatherer --location=foo D")
 
         # missing location
-        argv = "create-stats-gatherer --port=foo D"
-        self.assertRaises(usage.UsageError, runner.runner, argv.split(),
-                          run_by_human=False)
+        _test("create-stats-gatherer --port=foo D")
 
         # can't provide both
-        argv = "create-stats-gatherer --hostname=foo --port=foo D"
-        self.assertRaises(usage.UsageError, runner.runner, argv.split(),
-                          run_by_human=False)
+        _test("create-stats-gatherer --hostname=foo --port=foo D")
 
         # can't provide both
-        argv = "create-stats-gatherer --hostname=foo --location=foo D"
-        self.assertRaises(usage.UsageError, runner.runner, argv.split(),
-                          run_by_human=False)
+        _test("create-stats-gatherer --hostname=foo --location=foo D")
 
         # can't provide all three
-        argv = "create-stats-gatherer --hostname=foo --location=foo --port=foo D"
-        self.assertRaises(usage.UsageError, runner.runner, argv.split(),
-                          run_by_human=False)
+        _test("create-stats-gatherer --hostname=foo --location=foo --port=foo D")
 
 class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin,
               RunBinTahoeMixin):

From 802cfc87fe8c9343f39e4ef80100e0d61817292a Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 9 Sep 2016 14:25:10 -0700
Subject: [PATCH 10/10] CLI: allow dispatch functions to return Deferred

In addition, CLI functions are allowed to use sys.exit() instead of
always needing to return the exit code as an integer.

runner.py now knows about the blocking httplib calls in scripts/cli and
scripts/magic_folder, and uses deferToThread() to invoke them. Those
functions cannot return a Deferred: when rewrite them to use twisted.web
or treq, we'll remove this deferToThread call.

Option parsing was split out to a separate function for testing. We now
use twisted.internet.task.react() to start the reactor, which required
changing the way runner.py is tested.

closes ticket:2826
---
 src/allmydata/scripts/runner.py    | 77 +++++++++++++++++-------------
 src/allmydata/test/cli/test_cli.py | 23 ++++++---
 src/allmydata/test/common_util.py  | 30 ++++++------
 3 files changed, 76 insertions(+), 54 deletions(-)

diff --git a/src/allmydata/scripts/runner.py b/src/allmydata/scripts/runner.py
index d512eca16..131aa35d6 100644
--- a/src/allmydata/scripts/runner.py
+++ b/src/allmydata/scripts/runner.py
@@ -3,6 +3,7 @@ import os, sys
 from cStringIO import StringIO
 
 from twisted.python import usage
+from twisted.internet import defer, task, threads
 
 from allmydata.scripts.common import get_default_nodedir
 from allmydata.scripts import debug, create_node, startstop_node, cli, \
@@ -89,23 +90,17 @@ create_dispatch = {}
 for module in (create_node, stats_gatherer):
     create_dispatch.update(module.dispatch)
 
-def runner(argv,
-           run_by_human=True,
-           stdin=None, stdout=None, stderr=None):
-
-    assert sys.version_info < (3,), ur"Tahoe-LAFS does not run under Python 3. Please use Python 2.7.x."
-
-    stdin  = stdin  or sys.stdin
-    stdout = stdout or sys.stdout
-    stderr = stderr or sys.stderr
+def parse_options(argv, config=None):
+    if not config:
+        config = Options()
+    config.parseOptions(argv) # may raise usage.error
+    return config
 
+def parse_or_exit_with_explanation(argv, stdout=sys.stdout):
     config = Options()
-
     try:
-        config.parseOptions(argv)
+        parse_options(argv, config=config)
     except usage.error, e:
-        if not run_by_human:
-            raise
         c = config
         while hasattr(c, 'subOptions'):
             c = c.subOptions
@@ -115,14 +110,15 @@ def runner(argv,
         except Exception:
             msg = repr(e)
         print >>stdout, "%s:  %s\n" % (sys.argv[0], quote_output(msg, quotemarks=False))
-        return 1
+        sys.exit(1)
+    return config
 
+def dispatch(config,
+             stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr):
     command = config.subCommand
     so = config.subOptions
-
     if config['quiet']:
         stdout = StringIO()
-
     so.stdout = stdout
     so.stderr = stderr
     so.stdin = stdin
@@ -136,29 +132,46 @@ def runner(argv,
     elif command in admin.dispatch:
         f = admin.dispatch[command]
     elif command in cli.dispatch:
-        f = cli.dispatch[command]
+        # these are blocking, and must be run in a thread
+        f0 = cli.dispatch[command]
+        f = lambda so: threads.deferToThread(f0, so)
     elif command in magic_folder_cli.dispatch:
-        f = magic_folder_cli.dispatch[command]
+        # same
+        f0 = magic_folder_cli.dispatch[command]
+        f = lambda so: threads.deferToThread(f0, so)
     else:
         raise usage.UsageError()
 
-    rc = f(so)
-    return rc
-
+    d = defer.maybeDeferred(f, so)
+    # the calling convention for CLI dispatch functions is that they either:
+    # 1: succeed and return rc=0
+    # 2: print explanation to stderr and return rc!=0
+    # 3: raise an exception that should just be printed normally
+    # 4: return a Deferred that does 1 or 2 or 3
+    def _raise_sys_exit(rc):
+        sys.exit(rc)
+    d.addCallback(_raise_sys_exit)
+    return d
 
 def run():
-    try:
-        if sys.platform == "win32":
-            from allmydata.windows.fixups import initialize
-            initialize()
+    assert sys.version_info < (3,), ur"Tahoe-LAFS does not run under Python 3. Please use Python 2.7.x."
 
-        rc = runner(sys.argv[1:])
-    except Exception:
-        import traceback
-        traceback.print_exc()
-        rc = 1
-
-    sys.exit(rc)
+    if sys.platform == "win32":
+        from allmydata.windows.fixups import initialize
+        initialize()
+    d = defer.maybeDeferred(parse_or_exit_with_explanation, sys.argv[1:])
+    d.addCallback(dispatch)
+    def _show_exception(f):
+        # when task.react() notices a non-SystemExit exception, it does
+        # log.err() with the failure and then exits with rc=1. We want this
+        # to actually print the exception to stderr, like it would do if we
+        # weren't using react().
+        if f.check(SystemExit):
+            return f # dispatch function handled it
+        f.printTraceback(file=sys.stderr)
+        sys.exit(1)
+    d.addErrback(_show_exception)
+    task.react(lambda _reactor: d) # doesn't return: calls sys.exit(rc)
 
 if __name__ == "__main__":
     run()
diff --git a/src/allmydata/test/cli/test_cli.py b/src/allmydata/test/cli/test_cli.py
index dafbf6329..c21856036 100644
--- a/src/allmydata/test/cli/test_cli.py
+++ b/src/allmydata/test/cli/test_cli.py
@@ -5,6 +5,7 @@ import urllib, sys
 
 from twisted.trial import unittest
 from twisted.python.monkey import MonkeyPatcher
+from twisted.internet import task
 
 import allmydata
 from allmydata.util import fileutil, hashutil, base32, keyutil
@@ -511,9 +512,9 @@ class CLI(CLITestMixin, unittest.TestCase):
         exc = Exception("canary")
         ns = Namespace()
 
-        ns.runner_called = False
-        def call_runner(args):
-            ns.runner_called = True
+        ns.parse_called = False
+        def call_parse_or_exit(args):
+            ns.parse_called = True
             raise exc
 
         ns.sys_exit_called = False
@@ -521,13 +522,23 @@ class CLI(CLITestMixin, unittest.TestCase):
             ns.sys_exit_called = True
             self.failUnlessEqual(exitcode, 1)
 
-        patcher = MonkeyPatcher((runner, 'runner', call_runner),
+        def fake_react(f):
+            d = f("reactor")
+            # normally this Deferred would be errbacked with SystemExit, but
+            # since we mocked out sys.exit, it will be fired with None. So
+            # it's safe to drop it on the floor.
+            del d
+
+        patcher = MonkeyPatcher((runner, 'parse_or_exit_with_explanation',
+                                 call_parse_or_exit),
                                 (sys, 'argv', ["tahoe"]),
                                 (sys, 'exit', call_sys_exit),
-                                (sys, 'stderr', stderr))
+                                (sys, 'stderr', stderr),
+                                (task, 'react', fake_react),
+                                )
         patcher.runWithPatches(runner.run)
 
-        self.failUnless(ns.runner_called)
+        self.failUnless(ns.parse_called)
         self.failUnless(ns.sys_exit_called)
         self.failUnlessIn(str(exc), stderr.getvalue())
 
diff --git a/src/allmydata/test/common_util.py b/src/allmydata/test/common_util.py
index af152b2c3..982b8a3ec 100644
--- a/src/allmydata/test/common_util.py
+++ b/src/allmydata/test/common_util.py
@@ -2,7 +2,7 @@ import os, signal, sys, time
 from random import randrange
 from cStringIO import StringIO
 
-from twisted.internet import reactor, defer, threads
+from twisted.internet import reactor, defer
 from twisted.python import failure
 from twisted.trial import unittest
 
@@ -28,25 +28,23 @@ def run_cli(verb, *args, **kwargs):
     argv = nodeargs + [verb] + list(args)
     stdin = kwargs.get("stdin", "")
     stdout, stderr = StringIO(), StringIO()
-    d = threads.deferToThread(runner.runner, argv, run_by_human=False,
-                              stdin=StringIO(stdin),
-                              stdout=stdout, stderr=stderr)
+    d = defer.succeed(argv)
+    d.addCallback(runner.parse_or_exit_with_explanation, stdout=stdout)
+    d.addCallback(runner.dispatch,
+                  stdin=StringIO(stdin),
+                  stdout=stdout, stderr=stderr)
     def _done(rc):
-        return rc, stdout.getvalue(), stderr.getvalue()
-    d.addCallback(_done)
+        return 0, stdout.getvalue(), stderr.getvalue()
+    def _err(f):
+        f.trap(SystemExit)
+        return f.value.code, stdout.getvalue(), stderr.getvalue()
+    d.addCallbacks(_done, _err)
     return d
 
 def parse_cli(*argv):
-    # This parses the CLI options (synchronously), and throws
-    # usage.UsageError if something went wrong.
-
-    # As a temporary side-effect, if the arguments can be parsed correctly,
-    # it also executes the command. This side-effect will be removed when
-    # runner.py is refactored. After the refactoring, this will return the
-    # Options object, and this method can be used for success testing, not
-    # just failure testing.
-    runner.runner(argv, run_by_human=False)
-    assert False, "eek, I can't be used for success testing yet"
+    # This parses the CLI options (synchronously), and returns the Options
+    # argument, or throws usage.UsageError if something went wrong.
+    return runner.parse_options(argv)
 
 class DevNullDictionary(dict):
     def __setitem__(self, key, value):