From caf6bceb8064d87e61f192ee9cace4f96c2e4b53 Mon Sep 17 00:00:00 2001 From: Jonathan Stoppani Date: Mon, 9 Jan 2017 14:46:18 +0100 Subject: [PATCH 1/6] Add a test to fail when the wrong reactor is set --- src/twisted/application/twist/test/test_options.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/twisted/application/twist/test/test_options.py b/src/twisted/application/twist/test/test_options.py index 3f1256444a0..57f831e818d 100644 --- a/src/twisted/application/twist/test/test_options.py +++ b/src/twisted/application/twist/test/test_options.py @@ -117,6 +117,15 @@ def test_installReactor(self): self.assertEqual(set(self.installedReactors), set(["fusion"])) + def test_installCorrectReactor(self): + self.patchInstallReactor() + + options = TwistOptions() + options.parseOptions(["--reactor=fusion"]) + + self.assertEqual(set(self.installedReactors), set(["fusion"])) + + def test_installReactorBogus(self): """ L{TwistOptions.installReactor} raises UsageError if an unknown reactor From b77fcd14e6bd245f2818fb861cdc65a3a3514da4 Mon Sep 17 00:00:00 2001 From: Jonathan Stoppani Date: Mon, 9 Jan 2017 14:50:20 +0100 Subject: [PATCH 2/6] Install the reactor after having parsed the arguments --- src/twisted/application/twist/_options.py | 3 ++- src/twisted/application/twist/test/test_options.py | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/twisted/application/twist/_options.py b/src/twisted/application/twist/_options.py index 544612a5247..9c73127038d 100644 --- a/src/twisted/application/twist/_options.py +++ b/src/twisted/application/twist/_options.py @@ -154,7 +154,6 @@ def selectDefaultLogObserver(self): def parseOptions(self, options=None): - self.installReactor() self.selectDefaultLogObserver() Options.parseOptions(self, options=options) @@ -188,6 +187,8 @@ def subCommands(self): def postOptions(self): + self.installReactor() + Options.postOptions(self) if self.subCommand is None: diff --git a/src/twisted/application/twist/test/test_options.py b/src/twisted/application/twist/test/test_options.py index 57f831e818d..120dc3ea8df 100644 --- a/src/twisted/application/twist/test/test_options.py +++ b/src/twisted/application/twist/test/test_options.py @@ -118,9 +118,14 @@ def test_installReactor(self): def test_installCorrectReactor(self): + """ + L{TwistOptions.installReactor} installs the chosen reactor after the + command line options have been parsed. + """ self.patchInstallReactor() options = TwistOptions() + options.subCommand = "test-subcommand" options.parseOptions(["--reactor=fusion"]) self.assertEqual(set(self.installedReactors), set(["fusion"])) @@ -374,6 +379,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) From 8a684ecdc2b7fffb36fa890c219d0f94bd80b82e Mon Sep 17 00:00:00 2001 From: Jonathan Stoppani Date: Mon, 9 Jan 2017 15:03:13 +0100 Subject: [PATCH 3/6] Add a news fragment --- src/twisted/topfiles/8983.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 src/twisted/topfiles/8983.bugfix diff --git a/src/twisted/topfiles/8983.bugfix b/src/twisted/topfiles/8983.bugfix new file mode 100644 index 00000000000..a3f48af53d2 --- /dev/null +++ b/src/twisted/topfiles/8983.bugfix @@ -0,0 +1 @@ +The twist script now respects the --reactor option. From 640036f9d4772e972101052daca3651255c9c620 Mon Sep 17 00:00:00 2001 From: Jonathan Stoppani Date: Mon, 9 Jan 2017 16:17:37 +0100 Subject: [PATCH 4/6] Install the reactor as soon as possible --- src/twisted/application/twist/_options.py | 27 ++++++++++++------- .../application/twist/test/test_options.py | 19 +++---------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/twisted/application/twist/_options.py b/src/twisted/application/twist/_options.py index 9c73127038d..a7c3887551f 100644 --- 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): @@ -187,8 +196,6 @@ def subCommands(self): def postOptions(self): - self.installReactor() - Options.postOptions(self) if self.subCommand is None: diff --git a/src/twisted/application/twist/test/test_options.py b/src/twisted/application/twist/test/test_options.py index 120dc3ea8df..b5e9c9439d0 100644 --- a/src/twisted/application/twist/test/test_options.py +++ b/src/twisted/application/twist/test/test_options.py @@ -96,25 +96,16 @@ def test_version(self): def test_reactor(self): """ - L{TwistOptions.opt_reactor} sets the reactor name. - """ - options = TwistOptions() - options.opt_reactor("fission") - - self.assertEquals(options["reactorName"], "fission") - - - def test_installReactor(self): - """ - L{TwistOptions.installReactor} installs the chosen reactor. + L{TwistOptions.installReactor} installs the chosen reactor and sets + the reactor name. """ self.patchInstallReactor() options = TwistOptions() options.opt_reactor("fusion") - options.installReactor() self.assertEqual(set(self.installedReactors), set(["fusion"])) + self.assertEquals(options["reactorName"], "fusion") def test_installCorrectReactor(self): @@ -139,9 +130,7 @@ def test_installReactorBogus(self): self.patchInstallReactor() options = TwistOptions() - options.opt_reactor("coal") - - self.assertRaises(UsageError, options.installReactor) + self.assertRaises(UsageError, options.opt_reactor, "coal") def test_logLevelValid(self): From e9f6ecfaec6654fa12f48c01eeb2df3deb2d8c88 Mon Sep 17 00:00:00 2001 From: Jonathan Stoppani Date: Mon, 9 Jan 2017 16:18:40 +0100 Subject: [PATCH 5/6] =?UTF-8?q?Set=20the=20default=20reactor=20if=20the=20?= =?UTF-8?q?options=20wasn=E2=80=99t=20provided?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/twisted/application/twist/_options.py | 3 +++ src/twisted/application/twist/test/test_twist.py | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/twisted/application/twist/_options.py b/src/twisted/application/twist/_options.py index a7c3887551f..2b7d3075155 100644 --- a/src/twisted/application/twist/_options.py +++ b/src/twisted/application/twist/_options.py @@ -167,6 +167,9 @@ def parseOptions(self, options=None): 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_twist.py b/src/twisted/application/twist/test/test_twist.py index f2114c5f052..0c7d7341b5a 100644 --- 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): From fca9e62bba608add1babbf8ab11ee72f9045233d Mon Sep 17 00:00:00 2001 From: Jonathan Stoppani Date: Fri, 13 Jan 2017 00:32:33 +0100 Subject: [PATCH 6/6] Add unit test to check for the installation of the default reactor --- src/twisted/application/twist/test/test_options.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/twisted/application/twist/test/test_options.py b/src/twisted/application/twist/test/test_options.py index b5e9c9439d0..853de9fde6a 100644 --- 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 @@ -133,6 +134,15 @@ def test_installReactorBogus(self): self.assertRaises(UsageError, options.opt_reactor, "coal") + 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): """ L{TwistOptions.opt_log_level} sets the corresponding log level.