Move ShouldFailMixin back to common_util

There were originally two versions of this, one in common and another in
common_util. We moved both into common_py3 but then removed the one from
common, so here we move back to common_util, while allowing imports from
common to avoid a noisy changeset.

src/allmydata/test/common.py

@@ -88,8 +88,7 @@ from ..crypto import (
 from .eliotutil import (
     EliotLoggedRunTest,
 )
-# Backwards compatibility imports:
+from .common_util import ShouldFailMixin  # noqa: F401
-from .common_py3 import ShouldFailMixin  # noqa: F401
 
 
 TEST_RSA_KEY_SIZE = 522

src/allmydata/test/common_py3.py

@@ -18,8 +18,7 @@ import os
 import time
 import signal
 
-from twisted.internet import defer, reactor
+from twisted.internet import reactor
-from twisted.python import failure
 from twisted.trial import unittest
 
 from ..util.assertutil import precondition
@@ -73,51 +72,6 @@ class SignalMixin(object):
         return super(SignalMixin, self).tearDown()
 
 
-class ShouldFailMixin(object):
-
-    def shouldFail(self, expected_failure, which, substring,
-                   callable, *args, **kwargs):
-        """Assert that a function call raises some exception. This is a
-        Deferred-friendly version of TestCase.assertRaises() .
-
-        Suppose you want to verify the following function:
-
-         def broken(a, b, c):
-             if a < 0:
-                 raise TypeError('a must not be negative')
-             return defer.succeed(b+c)
-
-        You can use:
-            d = self.shouldFail(TypeError, 'test name',
-                                'a must not be negative',
-                                broken, -4, 5, c=12)
-        in your test method. The 'test name' string will be included in the
-        error message, if any, because Deferred chains frequently make it
-        difficult to tell which assertion was tripped.
-
-        The substring= argument, if not None, must appear in the 'repr'
-        of the message wrapped by this Failure, or the test will fail.
-        """
-
-        assert substring is None or isinstance(substring, (bytes, unicode))
-        d = defer.maybeDeferred(callable, *args, **kwargs)
-        def done(res):
-            if isinstance(res, failure.Failure):
-                res.trap(expected_failure)
-                if substring:
-                    self.failUnless(substring in str(res),
-                                    "%s: substring '%s' not in '%s'"
-                                    % (which, substring, str(res)))
-                # return the Failure for further analysis, but in a form that
-                # doesn't make the Deferred chain think that we failed.
-                return [res]
-            else:
-                self.fail("%s was supposed to raise %s, not get '%s'" %
-                          (which, expected_failure, res))
-        d.addBoth(done)
-        return d
-
-
 class ReallyEqualMixin(object):
     def failUnlessReallyEqual(self, a, b, msg=None):
         self.assertEqual(a, b, msg)

src/allmydata/test/common_util.py

@@ -5,6 +5,7 @@ from random import randrange
 from six.moves import StringIO
 
 from twisted.internet import reactor, defer
+from twisted.python import failure
 from twisted.trial import unittest
 
 from ..util.assertutil import precondition
@@ -12,8 +13,9 @@ from ..scripts import runner
 from allmydata.util.encodingutil import get_io_encoding
 # Imported for backwards compatibility:
 from future.utils import bord, bchr, binary_type
+from past.builtins import unicode
 from .common_py3 import (
-    SignalMixin, skip_if_cannot_represent_filename, ReallyEqualMixin, ShouldFailMixin
+    SignalMixin, skip_if_cannot_represent_filename, ReallyEqualMixin
 )
 
 
@@ -85,6 +87,51 @@ class StallMixin(object):
         return d
 
 
+class ShouldFailMixin(object):
+
+    def shouldFail(self, expected_failure, which, substring,
+                   callable, *args, **kwargs):
+        """Assert that a function call raises some exception. This is a
+        Deferred-friendly version of TestCase.assertRaises() .
+
+        Suppose you want to verify the following function:
+
+         def broken(a, b, c):
+             if a < 0:
+                 raise TypeError('a must not be negative')
+             return defer.succeed(b+c)
+
+        You can use:
+            d = self.shouldFail(TypeError, 'test name',
+                                'a must not be negative',
+                                broken, -4, 5, c=12)
+        in your test method. The 'test name' string will be included in the
+        error message, if any, because Deferred chains frequently make it
+        difficult to tell which assertion was tripped.
+
+        The substring= argument, if not None, must appear in the 'repr'
+        of the message wrapped by this Failure, or the test will fail.
+        """
+
+        assert substring is None or isinstance(substring, (bytes, unicode))
+        d = defer.maybeDeferred(callable, *args, **kwargs)
+        def done(res):
+            if isinstance(res, failure.Failure):
+                res.trap(expected_failure)
+                if substring:
+                    self.failUnless(substring in str(res),
+                                    "%s: substring '%s' not in '%s'"
+                                    % (which, substring, str(res)))
+                # return the Failure for further analysis, but in a form that
+                # doesn't make the Deferred chain think that we failed.
+                return [res]
+            else:
+                self.fail("%s was supposed to raise %s, not get '%s'" %
+                          (which, expected_failure, res))
+        d.addBoth(done)
+        return d
+
+
 class TestMixin(SignalMixin):
     def setUp(self):
         return super(TestMixin, self).setUp()

src/allmydata/test/test_happiness.py

@@ -23,7 +23,7 @@ from hypothesis.strategies import text, sets
 from allmydata.immutable import happiness_upload
 from allmydata.util.happinessutil import servers_of_happiness, \
     shares_by_server, merge_servers
-from allmydata.test.common_py3 import ShouldFailMixin
+from allmydata.test.common import ShouldFailMixin
 
 
 class HappinessUploadUtils(unittest.TestCase):

src/allmydata/test/test_storage.py

@@ -51,8 +51,8 @@ from allmydata.test.no_network import NoNetworkServer
 from allmydata.storage_client import (
     _StorageServer,
 )
-from .common import LoggingServiceParent
+from .common import LoggingServiceParent, ShouldFailMixin
-from .common_py3 import FakeCanary, ShouldFailMixin
+from .common_py3 import FakeCanary
 
 
 class UtilTests(unittest.TestCase):

src/allmydata/test/test_upload.py

@@ -28,12 +28,12 @@ from allmydata.util import log, base32
 from allmydata.util.assertutil import precondition
 from allmydata.util.deferredutil import DeferredListShouldSucceed
 from allmydata.test.no_network import GridTestMixin
-from allmydata.test.common_py3 import ShouldFailMixin
 from allmydata.storage_client import StorageFarmBroker
 from allmydata.storage.server import storage_index_to_dir
 from allmydata.client import _Client
 from .common import (
     EMPTY_CLIENT_CONFIG,
+    ShouldFailMixin,
 )
 from functools import reduce