From 2b475777d9e945db48c9ef67fb9430cb5c04ae6c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 30 Jan 2019 12:22:26 -0500 Subject: [PATCH 1/5] Return the result of the base stopService It is a Deferred that indicates when things have actually stopped. Failing to return it means callers will think everything stopped synchronously. This is certainly not the case. --- src/allmydata/test/no_network.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/no_network.py b/src/allmydata/test/no_network.py index 28ed9f2e9..28adbf4ea 100644 --- a/src/allmydata/test/no_network.py +++ b/src/allmydata/test/no_network.py @@ -231,7 +231,7 @@ class _NoNetworkClient(_Client): def startService(self): service.MultiService.startService(self) def stopService(self): - service.MultiService.stopService(self) + return service.MultiService.stopService(self) def init_control(self): pass def init_helper(self): From 644fd04d08bfb45d44c80ed17bd3e873b2bce937 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 30 Jan 2019 12:28:19 -0500 Subject: [PATCH 2/5] news fragment --- newsfragments/2966.other | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/2966.other diff --git a/newsfragments/2966.other b/newsfragments/2966.other new file mode 100644 index 000000000..92b5437f6 --- /dev/null +++ b/newsfragments/2966.other @@ -0,0 +1 @@ +The NoNetworkGrid implementation has been somewhat improved. From 89bb68254b937a3dd04752bf101f7239b96c98f2 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 30 Jan 2019 15:55:37 -0500 Subject: [PATCH 3/5] Speed up MagicFolder service shutdown Also work-around a tricky, mysterious failure in the test suite by explicitly flushing the eventual call queue. I don't understand where the call that is landing there comes from or why some other part of the code isn't properly waiting on it. --- src/allmydata/frontends/magic_folder.py | 6 ++++++ src/allmydata/test/test_magic_folder.py | 3 +++ 2 files changed, 9 insertions(+) diff --git a/src/allmydata/frontends/magic_folder.py b/src/allmydata/frontends/magic_folder.py index c125a4976..298d38ef0 100644 --- a/src/allmydata/frontends/magic_folder.py +++ b/src/allmydata/frontends/magic_folder.py @@ -712,6 +712,8 @@ class Uploader(QueueMixin): d = self._notifier.wait_until_stopped() else: d = defer.succeed(None) + # Speed up shutdown + self._processing.cancel() # wait for processing loop to actually exit d.addCallback(lambda ign: self._processing) return d @@ -1198,6 +1200,10 @@ class Downloader(QueueMixin, WriteFileMixin): def stop(self): self._log("stop") self._stopped = True + + # Speed up shutdown + self._processing.cancel() + d = defer.succeed(None) # wait for processing loop to actually exit d.addCallback(lambda ign: self._processing) diff --git a/src/allmydata/test/test_magic_folder.py b/src/allmydata/test/test_magic_folder.py index 6872ed28f..c20477f9c 100644 --- a/src/allmydata/test/test_magic_folder.py +++ b/src/allmydata/test/test_magic_folder.py @@ -1448,6 +1448,9 @@ class SingleMagicFolderTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, Reall self.local_dir = os.path.join(self.basedir, u"local_dir") self.mkdir_nonascii(self.local_dir) + from foolscap.eventual import flushEventualQueue + self.addCleanup(flushEventualQueue) + d = self.create_invite_join_magic_folder(self.alice_nickname, self.local_dir) d.addCallback(self._restart_client) # note: _restart_client ultimately sets self.magicfolder to not-None From f61b51619d224575856b0fa5ad68dc6a2537eb2d Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 30 Jan 2019 19:02:13 -0500 Subject: [PATCH 4/5] Improve the failure mode for this test. Make it show stderr if there is any and stdout if the expected content is missing. --- src/allmydata/test/cli/test_create_alias.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/cli/test_create_alias.py b/src/allmydata/test/cli/test_create_alias.py index 46224de49..68f18236b 100644 --- a/src/allmydata/test/cli/test_create_alias.py +++ b/src/allmydata/test/cli/test_create_alias.py @@ -33,8 +33,8 @@ class CreateAlias(GridTestMixin, CLITestMixin, unittest.TestCase): d = self.do_cli("create-alias", "tahoe") def _done((rc,stdout,stderr)): - self.failUnless("Alias 'tahoe' created" in stdout) - self.failIf(stderr) + self.assertEqual(stderr, "") + self.assertIn("Alias 'tahoe' created", stdout) aliases = get_aliases(self.get_clientdir()) self.failUnless("tahoe" in aliases) self.failUnless(aliases["tahoe"].startswith("URI:DIR2:")) From c764214d0e3a826d04809841d4d953d1c16f9f9e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 31 Jan 2019 09:07:31 -0500 Subject: [PATCH 5/5] A note about this queue flushing. --- src/allmydata/test/test_magic_folder.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/allmydata/test/test_magic_folder.py b/src/allmydata/test/test_magic_folder.py index c20477f9c..a76e1a861 100644 --- a/src/allmydata/test/test_magic_folder.py +++ b/src/allmydata/test/test_magic_folder.py @@ -1448,6 +1448,15 @@ class SingleMagicFolderTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, Reall self.local_dir = os.path.join(self.basedir, u"local_dir") self.mkdir_nonascii(self.local_dir) + # Magic-folder implementation somehow manages to leave a DelayedCall + # in the reactor from the eventual queue by the end of the test. It + # may have something to do with the upload process but it's not + # entirely clear. It's difficult to track things through the eventual + # queue. It is almost certainly the case that some other Deferred + # involved in magic-folder that is already being waited on elsewhere + # *should* encompass this DelayedCall but I wasn't able to figure out + # where that association needs to be made. So, as a work-around, + # explicitly flush the eventual queue at the end of the test, too. from foolscap.eventual import flushEventualQueue self.addCleanup(flushEventualQueue)