Merge pull request #565 from tahoe-lafs/2989.tempdir-cleanup

Add some tempfile cleanup to the test suite

Fixes: ticket:2989

src/allmydata/test/common.py

@@ -1,4 +1,9 @@
 import os, random, struct
+import tempfile
+from tempfile import mktemp
+from functools import partial
+from unittest import case as _case
+
 import treq
 
 from zope.interface import implementer
@@ -7,8 +12,9 @@ from testtools import (
     TestCase,
 )
 from testtools.twistedsupport import (
-    AsynchronousDeferredRunTest,
     SynchronousDeferredRunTest,
+    AsynchronousDeferredRunTest,
+    AsynchronousDeferredRunTestForBrokenTwisted,
 )
 
 from twisted.internet import defer
@@ -36,7 +42,7 @@ import allmydata.test.common_util as testutil
 from allmydata.immutable.upload import Uploader
 
 from .eliotutil import (
-    eliot_logged_test,
+    EliotLoggedRunTest,
 )
 
 
@@ -845,10 +851,29 @@ class _TestCaseMixin(object):
     * Each test method will be run in a unique Eliot action context which
       identifies the test and collects all Eliot log messages emitted by that
       test (including setUp and tearDown messages).
+    * trial-compatible mktemp method
+    * unittest2-compatible assertRaises helper
+    * Automatic cleanup of tempfile.tempdir mutation (pervasive through the
+      Tahoe-LAFS test suite).
     """
-    @eliot_logged_test
-    def run(self, result):
-        return super(TestCase, self).run(result)
+    def setUp(self):
+        # Restore the original temporary directory.  Node ``init_tempdir``
+        # mangles it and many tests manage to get that method called.
+        self.addCleanup(
+            partial(setattr, tempfile, "tempdir", tempfile.tempdir),
+        )
+        return super(_TestCaseMixin, self).setUp()
+
+    class _DummyCase(_case.TestCase):
+        def dummy(self):
+            pass
+    _dummyCase = _DummyCase("dummy")
+
+    def mktemp(self):
+        return mktemp()
+
+    def assertRaises(self, *a, **kw):
+        return self._dummyCase.assertRaises(*a, **kw)
 
 
 class SyncTestCase(_TestCaseMixin, TestCase):
@@ -856,7 +881,9 @@ class SyncTestCase(_TestCaseMixin, TestCase):
     A ``TestCase`` which can run tests that may return an already-fired
     ``Deferred``.
     """
-    run_tests_with = SynchronousDeferredRunTest
+    run_tests_with = EliotLoggedRunTest.make_factory(
+        SynchronousDeferredRunTest,
+    )
 
 
 class AsyncTestCase(_TestCaseMixin, TestCase):
@@ -864,4 +891,20 @@ class AsyncTestCase(_TestCaseMixin, TestCase):
     A ``TestCase`` which can run tests that may return a Deferred that will
     only fire if the global reactor is running.
     """
-    run_tests_with = AsynchronousDeferredRunTest
+    run_tests_with = EliotLoggedRunTest.make_factory(
+        AsynchronousDeferredRunTest.make_factory(timeout=60.0),
+    )
+
+
+class AsyncBrokenTestCase(_TestCaseMixin, TestCase):
+    """
+    A ``TestCase`` like ``AsyncTestCase`` but which spins the reactor a little
+    longer than apparently necessary to clean out lingering unaccounted for
+    event sources.
+
+    Tests which require this behavior are broken and should be fixed so they
+    pass with ``AsyncTestCase``.
+    """
+    run_tests_with = EliotLoggedRunTest.make_factory(
+        AsynchronousDeferredRunTestForBrokenTwisted.make_factory(timeout=60.0),
+    )

src/allmydata/test/common_util.py

@@ -83,8 +83,8 @@ def flip_one_bit(s, offset=0, size=None):
 
 class ReallyEqualMixin:
     def failUnlessReallyEqual(self, a, b, msg=None):
-        self.failUnlessEqual(a, b, msg=msg)
-        self.failUnlessEqual(type(a), type(b), msg="a :: %r, b :: %r, %r" % (a, b, msg))
+        self.assertEqual(a, b, msg)
+        self.assertEqual(type(a), type(b), "a :: %r, b :: %r, %r" % (a, b, msg))
 
 
 class NonASCIIPathMixin:

src/allmydata/test/eliotutil.py

@@ -2,7 +2,18 @@
 Tools aimed at the interaction between tests and Eliot.
 """
 
-from functools import wraps
+__all__ = [
+    "RUN_TEST",
+    "EliotLoggedRunTest",
+    "eliot_logged_test",
+]
+
+from functools import (
+    wraps,
+    partial,
+)
+
+import attr
 
 from eliot import (
     ActionType,
@@ -105,3 +116,50 @@ def eliot_logged_test(f):
             return d
 
     return run_and_republish
+
+
+@attr.s
+class EliotLoggedRunTest(object):
+    """
+    A *RunTest* implementation which surrounds test invocation with an
+    Eliot-based action.
+
+    This *RunTest* composes with another for convenience.
+
+    :ivar case: The test case to run.
+
+    :ivar handlers: Pass-through for the wrapped *RunTest*.
+    :ivar last_resort: Pass-through for the wrapped *RunTest*.
+
+    :ivar _run_tests_with_factory: A factory for the other *RunTest*.
+    """
+    _run_tests_with_factory = attr.ib()
+    case = attr.ib()
+    handlers = attr.ib(default=None)
+    last_resort = attr.ib(default=None)
+
+    @classmethod
+    def make_factory(cls, delegated_run_test_factory):
+        return partial(cls, delegated_run_test_factory)
+
+    @property
+    def eliot_logger(self):
+        return self.case.eliot_logger
+
+    @eliot_logger.setter
+    def eliot_logger(self, value):
+        self.case.eliot_logger = value
+
+    def addCleanup(self, f):
+        return self.case.addCleanup(f)
+
+    def id(self):
+        return self.case.id()
+
+    @eliot_logged_test
+    def run(self, result=None):
+        return self._run_tests_with_factory(
+            self.case,
+            self.handlers,
+            self.last_resort,
+        ).run(result)

src/allmydata/test/test_introducer.py

@@ -5,7 +5,10 @@ import json
 from socket import socket, AF_INET
 from mock import Mock, patch
 
-from twisted.trial import unittest
+from testtools.matchers import (
+    Is,
+)
+
 from twisted.internet import defer, address
 from twisted.python import log
 from twisted.python.filepath import FilePath
@@ -34,6 +37,11 @@ from allmydata.client import (
 )
 from allmydata.util import pollmixin, keyutil, idlib, fileutil, iputil, yamlutil
 import allmydata.test.common_util as testutil
+from .common import (
+    SyncTestCase,
+    AsyncTestCase,
+    AsyncBrokenTestCase,
+)
 
 fcntl = requireModule("fcntl")
 
@@ -41,7 +49,7 @@ class LoggingMultiService(service.MultiService):
     def log(self, msg, **kw):
         log.msg(msg, **kw)
 
-class Node(testutil.SignalMixin, testutil.ReallyEqualMixin, unittest.TestCase):
+class Node(testutil.SignalMixin, testutil.ReallyEqualMixin, AsyncTestCase):
 
     def test_backwards_compat_import(self):
         # for old introducer .tac files
@@ -143,7 +151,7 @@ class ServiceMixin(object):
         d.addCallback(flushEventualQueue)
         return d
 
-class Introducer(ServiceMixin, unittest.TestCase, pollmixin.PollMixin):
+class Introducer(ServiceMixin, AsyncTestCase):
     def test_create(self):
         ic = IntroducerClient(None, "introducer.furl", u"my_nickname",
                               "my_version", "oldest_version", {}, fakeseq,
@@ -175,7 +183,7 @@ def make_ann_t(ic, furl, privkey, seqnum):
     ann_t = sign_to_foolscap(ann_d, privkey)
     return ann_t
 
-class Client(unittest.TestCase):
+class Client(AsyncTestCase):
     def test_duplicate_receive_v2(self):
         ic1 = IntroducerClient(None,
                                "introducer.furl", u"my_nickname",
@@ -284,7 +292,7 @@ class Client(unittest.TestCase):
         d.addCallback(_then5)
         return d
 
-class Server(unittest.TestCase):
+class Server(AsyncTestCase):
     def test_duplicate(self):
         i = IntroducerService()
         ic1 = IntroducerClient(None,
@@ -434,7 +442,7 @@ class SystemTestMixin(ServiceMixin, pollmixin.PollMixin):
         tub.setServiceParent(self.parent)
         self.central_portnum = listenOnUnused(tub, portnum)
 
-class Queue(SystemTestMixin, unittest.TestCase):
+class Queue(SystemTestMixin, AsyncTestCase):
     def test_queue_until_connected(self):
         self.basedir = "introducer/QueueUntilConnected/queued"
         os.makedirs(self.basedir)
@@ -484,7 +492,7 @@ class Queue(SystemTestMixin, unittest.TestCase):
         return d
 
 
-class SystemTest(SystemTestMixin, unittest.TestCase):
+class SystemTest(SystemTestMixin, AsyncTestCase):
 
     def do_system_test(self):
         self.create_tub()
@@ -643,13 +651,13 @@ class SystemTest(SystemTestMixin, unittest.TestCase):
             ir = introweb.IntroducerRoot(self.parent)
             self.parent.nodeid = "NODEID"
             text = ir.renderSynchronously().decode("utf-8")
-            self.failUnlessIn(NICKNAME % "0", text) # a v2 client
-            self.failUnlessIn(NICKNAME % "1", text) # another v2 client
+            self.assertIn(NICKNAME % "0", text) # a v2 client
+            self.assertIn(NICKNAME % "1", text) # another v2 client
             for i in range(NUM_STORAGE):
-                self.failUnlessIn(printable_serverids[i], text,
+                self.assertIn(printable_serverids[i], text,
                                   (i,printable_serverids[i],text))
                 # make sure there isn't a double-base32ed string too
-                self.failIfIn(idlib.nodeid_b2a(printable_serverids[i]), text,
+                self.assertNotIn(idlib.nodeid_b2a(printable_serverids[i]), text,
                               (i,printable_serverids[i],text))
             log.msg("_check1 done")
         d.addCallback(_check1)
@@ -779,7 +787,7 @@ class FakeRemoteReference:
     def getPeer(self): return address.IPv4Address("TCP", "remote.example.com",
                                                   3456)
 
-class ClientInfo(unittest.TestCase):
+class ClientInfo(AsyncTestCase):
     def test_client_v2(self):
         introducer = IntroducerService()
         tub = introducer_furl = None
@@ -801,7 +809,7 @@ class ClientInfo(unittest.TestCase):
         self.failUnlessEqual(s0.nickname, NICKNAME % u"v2")
         self.failUnlessEqual(s0.version, "my_version")
 
-class Announcements(unittest.TestCase):
+class Announcements(AsyncTestCase):
     def test_client_v2_signed(self):
         introducer = IntroducerService()
         tub = introducer_furl = None
@@ -818,7 +826,7 @@ class Announcements(unittest.TestCase):
         introducer.remote_publish_v2(ann_t0, canary0)
         a = introducer.get_announcements()
         self.failUnlessEqual(len(a), 1)
-        self.failUnlessIdentical(a[0].canary, canary0)
+        self.assertThat(a[0].canary, Is(canary0))
         self.failUnlessEqual(a[0].index, ("storage", pks))
         self.failUnlessEqual(a[0].announcement["app-versions"], app_versions)
         self.failUnlessEqual(a[0].nickname, u"nick-v2")
@@ -922,7 +930,7 @@ class Announcements(unittest.TestCase):
         self.assertEqual(c2.storage_broker.get_all_serverids(),
                          frozenset([pub1, pub2]))
 
-class ClientSeqnums(unittest.TestCase):
+class ClientSeqnums(AsyncBrokenTestCase):
 
     @defer.inlineCallbacks
     def test_client(self):
@@ -983,7 +991,7 @@ class TooNewServer(IntroducerService):
                 "application-version": "greetings from the crazy future",
                 }
 
-class NonV1Server(SystemTestMixin, unittest.TestCase):
+class NonV1Server(SystemTestMixin, AsyncTestCase):
     # if the client connects to a server that doesn't provide the 'v2'
     # protocol, it is supposed to provide a useful error instead of a weird
     # exception.
@@ -1022,7 +1030,7 @@ class NonV1Server(SystemTestMixin, unittest.TestCase):
         d.addCallback(_done)
         return d
 
-class DecodeFurl(unittest.TestCase):
+class DecodeFurl(SyncTestCase):
     def test_decode(self):
         # make sure we have a working base64.b32decode. The one in
         # python2.4.[01] was broken.
@@ -1032,7 +1040,7 @@ class DecodeFurl(unittest.TestCase):
         nodeid = b32decode(m.group(1).upper())
         self.failUnlessEqual(nodeid, "\x9fM\xf2\x19\xcckU0\xbf\x03\r\x10\x99\xfb&\x9b-\xc7A\x1d")
 
-class Signatures(unittest.TestCase):
+class Signatures(SyncTestCase):
     def test_sign(self):
         ann = {"key1": "value1"}
         sk_s,vk_s = keyutil.make_keypair()