Merge pull request twisted#668 from GaretJax/8983-garetjax-twist-reactor

Author: GaretJax Reviewer: exarkun, glyph Fixes: ticket:8983 The `twist` script now respects the --reactor option.
koncybernet · Jan 18, 2017 · ffb0d00 · ffb0d00
2 parents aeeffc7 + eea528d
commit ffb0d00
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 21 deletions.
diff --git a/src/twisted/application/twist/_options.py b/src/twisted/application/twist/_options.py
@@ -30,13 +30,14 @@ class TwistOptions(Options):
     Command line options for C{twist}.
     """
 
+    defaultReactorName = "default"
     defaultLogLevel = LogLevel.info
 
 
     def __init__(self):
         Options.__init__(self)
 
-        self["reactorName"] = "default"
+        self["reactorName"] = self.defaultReactorName
         self["logLevel"] = self.defaultLogLevel
         self["logFile"] = stdout
 
@@ -59,7 +60,15 @@ def opt_reactor(self, name):
         The name of the reactor to use.
         (options: {options})
         """
-        self["reactorName"] = name
+        # Actually actually actually install the reactor right at this very
+        # moment, before any other code (for example, a sub-command plugin)
+        # runs and accidentally imports and installs the default reactor.
+        try:
+            self["reactor"] = self.installReactor(name)
+        except NoSuchReactor:
+            raise UsageError("Unknown reactor: {}".format(name))
+        else:
+            self["reactorName"] = name
 
     opt_reactor.__doc__ = dedent(opt_reactor.__doc__).format(
         options=", ".join(
@@ -68,15 +77,15 @@ def opt_reactor(self, name):
     )
 
 
-    def installReactor(self):
+    def installReactor(self, name):
         """
         Install the reactor.
         """
-        name = self["reactorName"]
-        try:
-            self["reactor"] = installReactor(name)
-        except NoSuchReactor:
-            raise UsageError("Unknown reactor: {}".format(name))
+        if name == self.defaultReactorName:
+            from twisted.internet import reactor
+            return reactor
+        else:
+            return installReactor(name)
 
 
     def opt_log_level(self, levelName):
@@ -154,11 +163,13 @@ def selectDefaultLogObserver(self):
 
 
     def parseOptions(self, options=None):
-        self.installReactor()
         self.selectDefaultLogObserver()
 
         Options.parseOptions(self, options=options)
 
+        if "reactor" not in self:
+            self["reactor"] = self.installReactor(self["reactorName"])
+
 
     @property
     def plugins(self):

diff --git a/src/twisted/application/twist/test/test_options.py b/src/twisted/application/twist/test/test_options.py
@@ -7,6 +7,7 @@
 
 from sys import stdout, stderr
 
+from twisted.internet import reactor
 from twisted.copyright import version
 from twisted.python.usage import UsageError
 from twisted.logger import LogLevel, textFileLogObserver, jsonFileLogObserver
@@ -96,23 +97,28 @@ def test_version(self):
 
     def test_reactor(self):
         """
-        L{TwistOptions.opt_reactor} sets the reactor name.
+        L{TwistOptions.installReactor} installs the chosen reactor and sets
+        the reactor name.
         """
+        self.patchInstallReactor()
+
         options = TwistOptions()
-        options.opt_reactor("fission")
+        options.opt_reactor("fusion")
 
-        self.assertEquals(options["reactorName"], "fission")
+        self.assertEqual(set(self.installedReactors), set(["fusion"]))
+        self.assertEquals(options["reactorName"], "fusion")
 
 
-    def test_installReactor(self):
+    def test_installCorrectReactor(self):
         """
-        L{TwistOptions.installReactor} installs the chosen reactor.
+        L{TwistOptions.installReactor} installs the chosen reactor after the
+        command line options have been parsed.
         """
         self.patchInstallReactor()
 
         options = TwistOptions()
-        options.opt_reactor("fusion")
-        options.installReactor()
+        options.subCommand = "test-subcommand"
+        options.parseOptions(["--reactor=fusion"])
 
         self.assertEqual(set(self.installedReactors), set(["fusion"]))
 
@@ -125,9 +131,16 @@ def test_installReactorBogus(self):
         self.patchInstallReactor()
 
         options = TwistOptions()
-        options.opt_reactor("coal")
+        self.assertRaises(UsageError, options.opt_reactor, "coal")
 
-        self.assertRaises(UsageError, options.installReactor)
+
+    def test_installReactorDefault(self):
+        """
+        L{TwistOptions.installReactor} returns the currently installed reactor
+        when the default reactor name is specified.
+        """
+        options = TwistOptions()
+        self.assertIdentical(reactor, options.installReactor('default'))
 
 
     def test_logLevelValid(self):
@@ -365,6 +378,8 @@ def test_postOptionsNoSubCommand(self):
         L{TwistOptions.postOptions} raises L{UsageError} is it has no
         sub-command.
         """
+        self.patchInstallReactor()
+
         options = TwistOptions()
 
         self.assertRaises(UsageError, options.postOptions)
diff --git a/src/twisted/application/twist/test/test_twist.py b/src/twisted/application/twist/test/test_twist.py
@@ -13,7 +13,7 @@
 from ...runner._exit import ExitStatus
 from ...runner._runner import Runner, RunnerOptions
 from ...runner.test.test_runner import DummyExit
-from ...twist import _options, _twist
+from ...twist import _twist
 from .._options import TwistOptions
 from .._twist import Twist
 
@@ -45,12 +45,12 @@ def patchInstallReactor(self):
         """
         self.installedReactors = {}
 
-        def installReactor(name):
+        def installReactor(_, name):
             reactor = MemoryReactor()
             self.installedReactors[name] = reactor
             return reactor
 
-        self.patch(_options, "installReactor", installReactor)
+        self.patch(TwistOptions, "installReactor", installReactor)
 
 
     def patchStartService(self):

diff --git a/src/twisted/topfiles/8983.bugfix b/src/twisted/topfiles/8983.bugfix
@@ -0,0 +1 @@
+The twist script now respects the --reactor option.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		The twist script now respects the --reactor option.