launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:inprocess-keyserver-cleanup into launchpad:master.

Commit message:
Clean up InProcessKeyServerFixture asynchronously

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/399692

Tests that use `InProcessKeyServerFixture` fail anomalously often on buildbot, and it may be because the fixture's cleanup isn't being done quite right: `port.stopListening` may return a `Deferred`, and nothing was waiting for that to fire.  Rearrange the cleanup code to run asynchronously.  This is rather awkward in places due to the lack of a proper asynchronous fixtures API.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:inprocess-keyserver-cleanup into launchpad:master.

diff --git a/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py b/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
index b521e85..263b592 100644
--- a/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
+++ b/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
@@ -81,11 +81,10 @@ class TestSignableArchiveWithSigningKey(TestCaseWithFactory):
         self.archive_root = getPubConfig(self.archive).archiveroot
         self.suite = "distroseries"
 
-        with InProcessKeyServerFixture() as keyserver:
-            yield keyserver.start()
-            key_path = os.path.join(gpgkeysdir, 'ppa-sample@xxxxxxxxxxxxxxxxx')
-            yield IArchiveGPGSigningKey(self.archive).setSigningKey(
-                key_path, async_keyserver=True)
+        yield self.useFixture(InProcessKeyServerFixture()).start()
+        key_path = os.path.join(gpgkeysdir, 'ppa-sample@xxxxxxxxxxxxxxxxx')
+        yield IArchiveGPGSigningKey(self.archive).setSigningKey(
+            key_path, async_keyserver=True)
 
     def test_signFile_absolute_within_archive(self):
         filename = os.path.join(self.archive_root, "signme")
diff --git a/lib/lp/archivepublisher/tests/test_signing.py b/lib/lp/archivepublisher/tests/test_signing.py
index e59a3dd..b83eef9 100644
--- a/lib/lp/archivepublisher/tests/test_signing.py
+++ b/lib/lp/archivepublisher/tests/test_signing.py
@@ -231,11 +231,10 @@ class TestSigningHelpers(TestCaseWithFactory):
 
     @defer.inlineCallbacks
     def setUpArchiveKey(self):
-        with InProcessKeyServerFixture() as keyserver:
-            yield keyserver.start()
-            key_path = os.path.join(gpgkeysdir, 'ppa-sample@xxxxxxxxxxxxxxxxx')
-            yield IArchiveGPGSigningKey(self.archive).setSigningKey(
-                key_path, async_keyserver=True)
+        yield self.useFixture(InProcessKeyServerFixture()).start()
+        key_path = os.path.join(gpgkeysdir, 'ppa-sample@xxxxxxxxxxxxxxxxx')
+        yield IArchiveGPGSigningKey(self.archive).setSigningKey(
+            key_path, async_keyserver=True)
 
     def setUpUefiKeys(self, create=True, series=None):
         if not series:
diff --git a/lib/lp/archivepublisher/tests/test_sync_signingkeys.py b/lib/lp/archivepublisher/tests/test_sync_signingkeys.py
index 9e4189c..6f1edbd 100644
--- a/lib/lp/archivepublisher/tests/test_sync_signingkeys.py
+++ b/lib/lp/archivepublisher/tests/test_sync_signingkeys.py
@@ -576,10 +576,9 @@ class TestSyncSigningKeysScript(TestCaseWithFactory):
 
     @defer.inlineCallbacks
     def setUpArchiveKey(self, archive, secret_key_path):
-        with InProcessKeyServerFixture() as keyserver:
-            yield keyserver.start()
-            yield IArchiveGPGSigningKey(archive).setSigningKey(
-                secret_key_path, async_keyserver=True)
+        yield self.useFixture(InProcessKeyServerFixture()).start()
+        yield IArchiveGPGSigningKey(archive).setSigningKey(
+            secret_key_path, async_keyserver=True)
 
     @defer.inlineCallbacks
     def test_injectGPG(self):
diff --git a/lib/lp/soyuz/tests/test_archive.py b/lib/lp/soyuz/tests/test_archive.py
index 5c3f3b6..f06ae96 100644
--- a/lib/lp/soyuz/tests/test_archive.py
+++ b/lib/lp/soyuz/tests/test_archive.py
@@ -1869,11 +1869,10 @@ class TestArchiveDependencies(TestCaseWithFactory):
     def test_private_sources_list(self):
         """Entries for private dependencies include credentials."""
         p3a = self.factory.makeArchive(name='p3a', private=True)
-        with InProcessKeyServerFixture() as keyserver:
-            yield keyserver.start()
-            key_path = os.path.join(gpgkeysdir, 'ppa-sample@xxxxxxxxxxxxxxxxx')
-            yield IArchiveGPGSigningKey(p3a).setSigningKey(
-                key_path, async_keyserver=True)
+        yield self.useFixture(InProcessKeyServerFixture()).start()
+        key_path = os.path.join(gpgkeysdir, 'ppa-sample@xxxxxxxxxxxxxxxxx')
+        yield IArchiveGPGSigningKey(p3a).setSigningKey(
+            key_path, async_keyserver=True)
         dependency = self.factory.makeArchive(
             name='dependency', private=True, owner=p3a.owner)
         with person_logged_in(p3a.owner):
diff --git a/lib/lp/testing/keyserver/inprocess.py b/lib/lp/testing/keyserver/inprocess.py
index 592dfae..e937a06 100644
--- a/lib/lp/testing/keyserver/inprocess.py
+++ b/lib/lp/testing/keyserver/inprocess.py
@@ -48,20 +48,31 @@ class InProcessKeyServerFixture(Fixture):
             def setUp(self):
                 super(TestSomething, self).setUp()
                 yield self.useFixture(InProcessKeyServerFixture()).start()
+
+    Unlike other fixtures, `InProcessKeyServerFixture` should not be used as
+    a context manager, because the context manager API does not offer a way
+    to do asynchronous cleanup.
     """
 
     @defer.inlineCallbacks
     def start(self):
         resource = KeyServerResource(self.useFixture(TempDir()).path)
         endpoint = endpoints.serverFromString(reactor, nativeString("tcp:0"))
-        port = yield endpoint.listen(server.Site(resource))
-        self.addCleanup(port.stopListening)
+        self._port = yield endpoint.listen(server.Site(resource))
         config.push("in-process-key-server-fixture", dedent("""
             [gpghandler]
             port: %s
-            """) % port.getHost().port)
+            """) % self._port.getHost().port)
         self.addCleanup(config.pop, "in-process-key-server-fixture")
 
+    @defer.inlineCallbacks
+    def cleanUp(self, *args, **kwargs):
+        # fixtures.callmany.CallMany doesn't support cleanup functions that
+        # return Deferred, so we have to do this manually.
+        yield self._port.stopListening()
+        defer.returnValue(
+            super(InProcessKeyServerFixture, self).cleanUp(*args, **kwargs))
+
     @property
     def url(self):
         """The URL that the web server will be running on."""