Merge lp:~nataliabidart/magicicada-server/no-more-storms-1 into lp:magicicada-server

[Natalia Bidart]

- Transform many server tests that were using the old deferred school into inlineCallbacks tests.
- Fixed some docstrings.
- Ensure src/server/server.py does not import from backends, to ensure total decoupling from the twisted main thread.
- Replaced many hard-coded "user id 0" with the actual id of self.usr0, so later when the ID gets generated automatically, tests do not depend on a hard coded value.

[lp:~nataliabidart/magicicada-server/no-more-storms-1]

49. By Natalia Bidart on 2016-02-26

Merged no-more-storms-0 into no-more-storms-1.

50. By Natalia Bidart on 2016-02-26

Merged trunk in.

[Facundo Batista (facundo)]

src/server/tests/test_sharing.py:28:1: F401 'dbmanager' imported but unused
src/server/tests/test_sharing.py:29:1: F401 'StorageObject' imported but unused
make: *** [lint] Error 1

review: Needs Fixing

[lp:~nataliabidart/magicicada-server/no-more-storms-1]

51. By Natalia Bidart on 2016-02-27

Merged trunk in.

52. By Natalia Bidart on 2016-02-27

Merged trunk into no-more-storms-1.

[Facundo Batista (facundo)]

I'm consistently getting this:

$ ./test src/backends/filesync/tests/test_gateway.py -t StorageUserGatewayTestCase.test_get_photo_directories
backends.filesync.tests.test_gateway
  StorageUserGatewayTestCase
    test_get_photo_directories ... Traceback (most recent call last):
  File "/usr/lib/python2.7/unittest/case.py", line 327, in run
    testMethod()
  File "/home/facundo/devel/reps/magicicada/server/review_no-more-storms-1/src/backends/filesync/tests/test_gateway.py", line 1784, in test_get_photo_directories
    set([(d.id, d.vol_type) for d in expected_dirs]))
  File "/usr/lib/python2.7/unittest/case.py", line 511, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python2.7/unittest/case.py", line 794, in assertSetEqual
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/lib/python2.7/unittest/case.py", line 408, in fail
    raise self.failureException(msg)
exceptions.AssertionError: Items in the first set but not the second:
('7c36e5fa-37be-4386-8651-3e10e1f44241', u'udf')
('bb841d58-2aaa-4ccb-bc0c-3587ad9dbc00', u'udf')
('9ec2e502-7e74-4e0e-8ca2-5feb20df7a9e', u'udf')
Items in the second set but not the first:
(UUID('bb841d58-2aaa-4ccb-bc0c-3587ad9dbc00'), u'udf')
(UUID('7c36e5fa-37be-4386-8651-3e10e1f44241'), u'root')
(UUID('9ec2e502-7e74-4e0e-8ca2-5feb20df7a9e'), u'udf')
[FAIL]

===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/usr/lib/python2.7/unittest/case.py", line 327, in run
    testMethod()
  File "/home/facundo/devel/reps/magicicada/server/review_no-more-storms-1/src/backends/filesync/tests/test_gateway.py", line 1784, in test_get_photo_directories
    set([(d.id, d.vol_type) for d in expected_dirs]))
  File "/usr/lib/python2.7/unittest/case.py", line 511, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python2.7/unittest/case.py", line 794, in assertSetEqual
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/lib/python2.7/unittest/case.py", line 408, in fail
    raise self.failureException(msg)
exceptions.AssertionError: Items in the first set but not the second:
('7c36e5fa-37be-4386-8651-3e10e1f44241', u'udf')
('bb841d58-2aaa-4ccb-bc0c-3587ad9dbc00', u'udf')
('9ec2e502-7e74-4e0e-8ca2-5feb20df7a9e', u'udf')
Items in the second set but not the first:
(UUID('bb841d58-2aaa-4ccb-bc0c-3587ad9dbc00'), u'udf')
(UUID('7c36e5fa-37be-4386-8651-3e10e1f44241'), u'root')
(UUID('9ec2e502-7e74-4e0e-8ca2-5feb20df7a9e'), u'udf')

backends.filesync.tests.test_gateway.StorageUserGatewayTestCase.test_get_photo_directories

[Facundo Batista (facundo)]

Go, girl

review: Approve

=== modified file 'src/server/integtests/test_action_queue.py'
--- src/server/integtests/test_action_queue.py	2015-09-26 13:02:01 +0000
+++ src/server/integtests/test_action_queue.py	2016-02-27 20:39:13 +0000
@@ -773,20 +773,20 @@
             d.addCallback(lambda _: self.wait_for_nirvana(.5))
         return d
 
+    @defer.inlineCallbacks
     def test_connection_lost(self):
         """Try to connect."""
-        d = self.connect()
         dd = defer.Deferred()
-        d.addCallback(lambda _: self.wait_for_nirvana(.5))
-        d.addCallback(lambda _: self.aq.connector.transport.loseConnection())
-        d.addCallback(lambda _: reactor.callLater(1, dd.callback, None))
-        d.addCallback(lambda _: dd)
+
+        yield self.connect()
+        yield self.wait_for_nirvana(.5)
+        yield self.aq.connector.transport.loseConnection()
+        yield reactor.callLater(1, dd.callback, None)
+        yield dd
         # check that the connection lost event is issued:
-        d.addCallback(lambda _: self.assertIn(('SYS_CONNECTION_LOST', {}),
-                                              self.listener.q))
+        yield self.assertIn(('SYS_CONNECTION_LOST', {}), self.listener.q)
         # test that we reconnect:
-        d.addCallback(lambda _: self.wait_for_nirvana(.5))
-        return d
+        yield lambda _: self.wait_for_nirvana(.5)
 
     def test_init(self):
         """Check that, when started, state is INIT."""

=== modified file 'src/server/integtests/test_share_sync.py'
--- src/server/integtests/test_share_sync.py	2016-02-26 00:22:17 +0000
+++ src/server/integtests/test_share_sync.py	2016-02-27 20:39:13 +0000
@@ -33,7 +33,8 @@
 
 
 class TestSharesSync(test_sync.TestSync):
-    """ Base class for shares tests. """
+    """Base class for shares tests."""
+
     called = 0
 
     def handle_SYS_STATE_CHANGED(self, state):

=== modified file 'src/server/server.py'
--- src/server/server.py	2015-10-18 01:35:37 +0000
+++ src/server/server.py	2016-02-27 20:39:13 +0000
@@ -55,7 +55,6 @@
 from metrics import get_meter
 from metrics.metricsconnector import MetricsConnector
 from backends.filesync import errors as dataerror
-from backends.filesync.models import Share, StorageObject
 from backends.filesync.notifier import notifier
 from magicicada import settings
 from ubuntuone.storage.server.logger import configure_logger, TRACE
@@ -1186,8 +1185,8 @@
     # these are the valid access levels and their translation from the
     # protocol message
     _valid_access_levels = {
-        protocol_pb2.CreateShare.VIEW: Share.VIEW,
-        protocol_pb2.CreateShare.MODIFY: Share.MODIFY,
+        protocol_pb2.CreateShare.VIEW: 'View',
+        protocol_pb2.CreateShare.MODIFY: 'Modify',
     }
 
     user_activity = 'create_share'
@@ -2160,11 +2159,9 @@
         if self.source_message.type == protocol_pb2.Message.MAKE_DIR:
             response_type = protocol_pb2.Message.NEW_DIR
             create_method_name = 'make_dir'
-            node_type = StorageObject.DIRECTORY
         elif self.source_message.type == protocol_pb2.Message.MAKE_FILE:
             response_type = protocol_pb2.Message.NEW_FILE
             create_method_name = 'make_file'
-            node_type = StorageObject.FILE
         else:
             raise request.StorageProtocolError(
                 'Can not create from message '
@@ -2188,7 +2185,7 @@
         # save data to be logged on operation end
         extension = self._get_extension(name)
         self.operation_data = 'type=%s vol_id=%s node_id=%s mime=%r ext=%r' % (
-            node_type, share_id, node_id, mimetype, extension)
+            create_method_name, share_id, node_id, mimetype, extension)
 
 
 class FreeSpaceResponse(SimpleRequestResponse):

=== modified file 'src/server/testing/testcase.py'
--- src/server/testing/testcase.py	2015-10-04 15:14:24 +0000
+++ src/server/testing/testcase.py	2016-02-27 20:39:13 +0000
@@ -92,9 +92,8 @@
 
 
 class BaseProtocolTestCase(TwistedTestCase):
-    """
-    Reusable part of ProtocolTestCase
-    """
+    """Reusable part of ProtocolTestCase."""
+
     # do we need to create oops files?
     createOOPSFiles = False
     heartbeat_interval = 0
@@ -408,10 +407,10 @@
         yield super(TestWithDatabase, self).setUp()
 
         users = (
-            (0, u'usr0', 'pass0'),
-            (1, u'usr1', 'pass1'),
+            (0, u'usr0', 'open sesame'),
+            (1, u'usr1', 'friend'),
             (2, u'usr2', 'pass2'),
-            (3, u'usr3', 'pass3'),
+            (3, u'usr3', 'usr3'),
         )
         for user_id, username, password in users:
             user = create_test_user(
@@ -419,6 +418,9 @@
             setattr(self, username, user)
             # set the password in the object just as a test simplifier
             user.password = password
+            dummy_tokens = getattr(self.auth_provider_class, '_allowed', None)
+            if dummy_tokens:
+                dummy_tokens[password] = user.id
 
         # tune the config for this tests
         self.patch(settings.api_server, 'STORAGE_CHUNK_SIZE', 1024 * 64)

=== modified file 'src/server/tests/test_auth.py'
--- src/server/tests/test_auth.py	2015-09-27 16:59:15 +0000
+++ src/server/tests/test_auth.py	2016-02-27 20:39:13 +0000
@@ -26,7 +26,6 @@
 
 from backends.filesync.errors import DoesNotExist
 from ubuntuone.storage.server.auth import (
-    AuthenticationProvider,
     DummyAuthProvider,
     SimpleAuthProvider,
 )
@@ -47,19 +46,13 @@
 
         return auth_d
 
-
-class AuthenticationProviderTests(AuthenticationBaseTestCase):
-    """Fixture for authentication provider tests."""
-
-    auth_provider_class = AuthenticationProvider
-
     @defer.inlineCallbacks
     def setUp(self):
-        yield super(AuthenticationProviderTests, self).setUp()
-        self.provider = self.auth_provider_class(self.service.factory)
-
-
-class DummyProviderTests(AuthenticationProviderTests):
+        yield super(AuthenticationBaseTestCase, self).setUp()
+        self.provider = self.service.factory.auth_provider
+
+
+class DummyProviderTests(AuthenticationBaseTestCase):
     """Tests for the dummy authentication provider."""
 
     auth_provider_class = DummyAuthProvider
@@ -69,7 +62,7 @@
         """The dummy authentication provider succeeds with a valid token."""
         auth_params = {"dummy_token": "open sesame"}
         user = yield self.provider.authenticate(auth_params, None)
-        self.assertEqual(user.id, 0)
+        self.assertEqual(user.id, self.usr0.id)
         # the same user is returned by repeated calls
         user2 = yield self.provider.authenticate(auth_params, None)
         self.assertEqual(user.id, user2.id)
@@ -88,7 +81,7 @@
         self.assertEqual(user, None)
 
 
-class SimpleAuthProviderTests(AuthenticationProviderTests):
+class SimpleAuthProviderTests(AuthenticationBaseTestCase):
     """Tests for the simple authentication provider."""
 
     auth_provider_class = SimpleAuthProvider

=== modified file 'src/server/tests/test_basic.py'
--- src/server/tests/test_basic.py	2015-09-27 21:33:56 +0000
+++ src/server/tests/test_basic.py	2016-02-27 20:39:13 +0000
@@ -409,6 +409,9 @@
     @defer.inlineCallbacks
     def test_status_OK(self):
         """Test the OK status response."""
+        status = self.site.resource.children['status']
+        # override user_id with a existing user
+        status.user_id = self.usr0.id
         response = yield client.getPage(self.url)
         self.assertEqual(response, 'Status OK\n')
 

=== modified file 'src/server/tests/test_content.py'
--- src/server/tests/test_content.py	2016-02-20 17:03:19 +0000
+++ src/server/tests/test_content.py	2016-02-27 20:39:13 +0000
@@ -590,7 +590,7 @@
                     return request
 
                 def read(innerself, cant):
-                    '''If second read, cancel and trigger test.'''
+                    """If second read, cancel and trigger test."""
                     innerself.notifs += 1
                     if innerself.notifs == 2:
                         innerself.request.cancel()
@@ -2099,10 +2099,6 @@
         # tune the config for this tests
         self.patch(settings.api_server, 'STORAGE_CHUNK_SIZE', 1024 * 1024)
 
-    @defer.inlineCallbacks
-    def tearDown(self):
-        yield super(TestChunkedContent, self).tearDown()
-
     def test_putcontent_chunked(self, put_fail=False, get_fail=False):
         """Checks a chunked putcontent."""
         size = int(settings.api_server.STORAGE_CHUNK_SIZE * 1.5)
@@ -2170,13 +2166,13 @@
         return self.callback_test(auth, timeout=10)
 
     def test_putcontent_chunked_putfail(self):
-        '''Assures that chunked putcontent fails with "try again".'''
+        """Assures that chunked putcontent fails with "try again"."""
         d = self.test_putcontent_chunked(put_fail=True)
         self.assertFails(d, 'TRY_AGAIN')
         return d
 
     def test_putcontent_chunked_getfail(self):
-        '''Assures that chunked putcontent fails with "try again".'''
+        """Assures that chunked putcontent fails with "try again"."""
         d = self.test_putcontent_chunked(get_fail=True)
         self.assertFails(d, 'NOT_AVAILABLE')
         return d
@@ -2349,12 +2345,7 @@
             return reactor.callLater(0.1, x)
 
         self._cooperator = task.Cooperator(scheduler=slowScheduler)
-
-    @defer.inlineCallbacks
-    def tearDown(self):
-        """Tear down."""
-        self._cooperator.stop()
-        yield super(TestUploadJob, self).tearDown()
+        self.addCleanup(self._cooperator.stop)
 
     @defer.inlineCallbacks
     def make_upload(self, size):
@@ -2410,7 +2401,6 @@
         size = self.double_size
         deflated_data, hash_value, upload_job = yield self.make_upload(size)
         yield upload_job.connect()
-        all_sent_deferred = defer.Deferred()
 
         # now let's upload some data
         def data_iter(chunk_size=request.MAX_MESSAGE_SIZE):
@@ -2418,10 +2408,8 @@
             for part in range(0, len(deflated_data), chunk_size):
                 yield upload_job.add_data(
                     deflated_data[part:part + chunk_size])
-            all_sent_deferred.callback(True)
 
-        self._cooperator.coiterate(data_iter())
-        yield all_sent_deferred
+        yield self._cooperator.coiterate(data_iter())
         yield upload_job.commit()
 
         # verify node content

=== modified file 'src/server/tests/test_fileops.py'
--- src/server/tests/test_fileops.py	2015-09-27 17:01:04 +0000
+++ src/server/tests/test_fileops.py	2016-02-27 20:39:13 +0000
@@ -67,23 +67,19 @@
 
     def test_move_invalid_character(self):
         """Try to move a dir to a name with invalid characters and fail."""
+
+        @defer.inlineCallbacks
         def auth(client):
-            d = client.dummy_authenticate("open sesame")
-            d.addCallback(lambda r: client.get_root())
-            d.addCallback(self.save_req, 'root_id')
-            d.addCallback(lambda r: client.make_dir(request.ROOT, r, "hola"))
-            d.addCallback(lambda req: client.move(request.ROOT, req.new_id,
-                                                  self._state.root_id,
-                                                  "hola / "))
-
-            def check(failure):
-                """Checks the error returned."""
-                self.assertIsInstance(failure.value,
-                                      request.StorageRequestError)
-                client.test_done(True)
-
-            d.addCallbacks(client.test_fail, check)
-        return self.callback_test(auth)
+            yield client.dummy_authenticate("open sesame")
+            root = yield client.get_root()
+            self.save_req(root, 'root_id')
+            req = yield client.make_dir(request.ROOT, root, "hola")
+            d = client.move(
+                request.ROOT, req.new_id, self._state.root_id, "hola / ")
+            res = yield self.assertFailure(d, request.StorageRequestError)
+            self.assertEqual(str(res), "INVALID_FILENAME")
+
+        return self.callback_test(auth, add_default_callbacks=True)
 
     def test_move_file_overwrite(self):
         """Rename over an existing file."""

=== modified file 'src/server/tests/test_make.py'
--- src/server/tests/test_make.py	2015-09-27 16:59:15 +0000
+++ src/server/tests/test_make.py	2016-02-27 20:39:13 +0000
@@ -25,7 +25,7 @@
 
 from StringIO import StringIO
 
-from twisted.internet import threads, defer
+from twisted.internet import defer
 
 from backends.filesync import errors
 from ubuntuone.storageprotocol import request, volumes
@@ -50,27 +50,23 @@
         return self.callback_test(auth)
 
     def build_mime_test(filename, file_mime):
-        """create test cases for mime type checking."""
+        """Create test cases for mime type checking."""
         def test_mkfile_mime_type(self):
             """Create a file."""
+            @defer.inlineCallbacks
             def auth(client):
                 def check_file(result):
-                    def _check_file():
-                        try:
-                            file = self.usr0.get_node(result.new_id)
-                        except errors.DoesNotExist:
-                            raise ValueError("storage object is missing")
-                        assert(file.mimetype == file_mime)
-                    d = threads.deferToThread(_check_file)
-                    return d
-                d = client.dummy_authenticate("open sesame")
-                d.addCallbacks(lambda r: client.get_root(), client.test_fail)
-                d.addCallbacks(
-                    lambda r: client.make_file(request.ROOT, r, filename),
-                    client.test_fail)
-                d.addCallback(check_file)
-                d.addCallbacks(client.test_done, client.test_fail)
-            return self.callback_test(auth)
+                    try:
+                        f = self.usr0.get_node(result.new_id)
+                    except errors.DoesNotExist:
+                        raise ValueError("storage object is missing")
+                    self.assertEqual(f.mimetype, file_mime)
+
+                yield client.dummy_authenticate("open sesame")
+                root = yield client.get_root()
+                result = yield client.make_file(request.ROOT, root, filename)
+                check_file(result)
+            return self.callback_test(auth, add_default_callbacks=True)
         return test_mkfile_mime_type
 
     test_mkfile_mime1 = build_mime_test("image.png", "image/png")

=== modified file 'src/server/tests/test_oops.py'
--- src/server/tests/test_oops.py	2015-09-18 23:36:43 +0000
+++ src/server/tests/test_oops.py	2016-02-27 20:39:13 +0000
@@ -125,21 +125,19 @@
         """Test that the user id and username is included in the extra data"""
         @defer.inlineCallbacks
         def poisoned_ping(client):
-            try:
-                pd = self.service.factory.protocols[0].wait_for_poison()
-                self.service.factory.protocols[0].poison("ping")
-                message = protocol_pb2.Message()
-                message.id = 5
-                message.type = protocol_pb2.Message.PING
-                client.sendMessage(message)
-                yield pd
+            pd = self.service.factory.protocols[0].wait_for_poison()
+            self.service.factory.protocols[0].poison("ping")
+            message = protocol_pb2.Message()
+            message.id = 5
+            message.type = protocol_pb2.Message.PING
+            client.sendMessage(message)
+            yield pd
 
-                oops_data = self.get_oops_data()
-                self.assertEqual("Service was poisoned with: ping",
-                                 oops_data["value"])
-                self.assertEqual("0,0,usr0", oops_data["username"])
-            except Exception, e:
-                raise e
+            oops_data = self.get_oops_data()
+            self.assertEqual("Service was poisoned with: ping",
+                             oops_data["value"])
+            self.assertEqual("%s,%s,usr0" % (self.usr0.id, self.usr0.id),
+                             oops_data["username"])
 
         def auth(client):
             d = client.dummy_authenticate("open sesame")

=== modified file 'src/server/tests/test_sharing.py'
--- src/server/tests/test_sharing.py	2016-02-27 00:01:05 +0000
+++ src/server/tests/test_sharing.py	2016-02-27 20:39:13 +0000
@@ -44,7 +44,7 @@
         """Test deletion of an offered by me share."""
 
         @defer.inlineCallbacks
-        def _do_delete(client):
+        def do_delete(client):
             """Callback to do delete."""
             # authenticate and create a root to share
             yield client.dummy_authenticate("open sesame")
@@ -55,14 +55,8 @@
             yield client.delete_share(share_id)
             self.assertRaises(errors.DoesNotExist,
                               self.usr1.get_share, share_id)
-            client.test_done()
-
-        def do_delete(client):
-            """Test body callback."""
-            d = _do_delete(client)
-            d.addErrback(client.test_fail)
-
-        return self.callback_test(do_delete)
+
+        return self.callback_test(do_delete, add_default_callbacks=True)
 
     def test_delete_volume(self):
         """Test deletion of an offered to me and accepted share."""
@@ -542,11 +536,11 @@
         subsubdir = subdir.make_subdirectory(u"subsubdir")
         subsubfile = subsubdir.make_file(u"subsubfile")
 
-        share = root2.share(0, u"foo", readonly=True)
+        share = root2.share(self.usr0.id, u"foo", readonly=True)
         share_other = root2.share(self.usr1.id, u"foo", readonly=True)
-        subshare = subdir.share(0, u"foo2", readonly=True)
-        share_owner = root1.share(0, u"foo3", readonly=True)
-        share_modify = rwdir.share(0, u"foo4")
+        subshare = subdir.share(self.usr0.id, u"foo2", readonly=True)
+        share_owner = root1.share(self.usr0.id, u"foo3", readonly=True)
+        share_modify = rwdir.share(self.usr0.id, u"foo4")
         for s in self.usr0.get_shared_to(accepted=False):
             s.accept()
         for s in self.usr1.get_shared_to(accepted=False):
@@ -835,7 +829,7 @@
     def _create_share(self, _, accepted=False):
         """Creates a share, optionally accepted."""
         root = self.usr1.root.load()
-        share = root.share(0, u"sharename", readonly=True)
+        share = root.share(self.usr0.id, u"sharename", readonly=True)
         if accepted:
             self.usr0.get_share(share.id).accept()
         # save the node id to be able to compare it later
@@ -1067,113 +1061,110 @@
 
     def test_notify_shared_node(self):
         """The notif should be sent to other users if the node is shared."""
+
+        d1 = defer.Deferred()
+        d2 = defer.Deferred()
+
+        @defer.inlineCallbacks
         def login1(client):
             """client1 login"""
-            self._state.client1 = client
-            d = client.dummy_authenticate("open sesame")  # for user #0
-            d.addCallback(lambda _: new_client())
-            d.addCallback(make_notification)
+            self.addCleanup(client.transport.loseConnection)
+            yield client.dummy_authenticate("open sesame")  # for user #0
+            # new client
+            reactor.connectTCP('localhost', self.port, factory2)
+            yield d2
+            # get the root
+            root_id = yield client.get_root()
+            # create the share
+            yield client.create_share(
+                root_id, self.usr1.username, u"name", Share.VIEW)
+            # mark the share as accepted by hand, so we don't
+            # have to emulate the whole process just for this test
+            shares = [s for s in self.usr1.get_shared_to()
+                      if s.shared_by_id == self.usr0.id]
+            share = shares[0]
+            share.accept()
+            # also store it in the state for further control
+            self._state.share_id = share.id
+            # create a file in the root
+            yield client.make_file(request.ROOT, root_id, "hola")
 
+        @defer.inlineCallbacks
         def login2(client):
             """client2 login"""
-            self._state.client2 = client
+            self.addCleanup(client.transport.loseConnection)
             client.set_volume_new_generation_callback(on_notification)
-            d = client.dummy_authenticate("friend")  # for user #1
-            d.addCallback(done_auth)
+            yield client.dummy_authenticate("friend")  # for user #1
+            d2.callback(None)
 
         # setup
         factory = FactoryHelper(login1)
         factory2 = FactoryHelper(login2)
-        d1 = defer.Deferred()
-        d2 = defer.Deferred()
         timeout = reactor.callLater(3, d1.errback, Exception("timeout"))
 
-        def new_client():
-            """add the second client"""
-            reactor.connectTCP('localhost', self.port, factory2)
-            return d2
-
         def on_notification(volume, generation):
             """notification arrived, check and cleanup"""
             # check
             self.assertEqual(volume, self._state.share_id)
-
             # cleanup
             factory.timeout.cancel()
             factory2.timeout.cancel()
             timeout.cancel()
             d1.callback(True)
-            self._state.client1.transport.loseConnection()
-            self._state.client2.transport.loseConnection()
-
-        def done_auth(result):
-            """authentication done for client2, we can start making changes"""
-            d2.callback(result)
-
-        def mark_share(_):
-            # mark the share as accepted by hand, so we don't
-            # have to emulate the whole process just for this test
-            shares = [s for s in self.usr1.get_shared_to()
-                      if s.shared_by_id == 0]
-            share = shares[0]
-            share.accept()
-            # also store it in the state for further control
-            self._state.share_id = share.id
-
-        def make_notification(_):
-            """create a change that should create a notification"""
-            # create the share
-            d = self._state.client1.get_root()
-            d.addCallback(self.save_req, 'root_id')
-
-            # create the share
-            d.addCallback(
-                lambda r: self._state.client1.create_share(
-                    r, self.usr1.username, u"name", Share.VIEW))
-            d.addCallback(mark_share)
-
-            # create a file in the root
-            d.addCallback(
-                lambda _: self._state.client1.make_file(
-                    request.ROOT, self._state.root_id, "hola"))
 
         reactor.connectTCP('localhost', self.port, factory)
         return d1
 
     def test_share_node_deleted(self):
-        '''Remove the node that was shared.'''
+        """Remove the node that was shared."""
+
+        d1 = defer.Deferred()
+        d2 = defer.Deferred()
+
+        @defer.inlineCallbacks
         def login1(client):
             """client1 login"""
+            self.addCleanup(client.transport.loseConnection)
             self._state.client1 = client
-            d = client.dummy_authenticate("open sesame")  # for user #0
-            d.addCallback(new_client)
-            d.addCallback(make_notification)
+            yield client.dummy_authenticate("open sesame")  # for user #0
+            reactor.connectTCP('localhost', self.port, factory2)
+            yield d2
+            # create the share
+            root_id = yield client.get_root()
+            # create the dir to share
+            req = yield client.make_dir(request.ROOT, root_id, "hi")
+            # create the share
+            yield client.create_share(
+                req.new_id, self.usr1.username, u"name", Share.VIEW)
+            # mark the share as accepted by hand, so we don't
+            # have to emulate the whole process just for this test
+            share = self.usr1.get_shared_to(accepted=False)[0]
+            share.accept()
+            # also store it in the state for further control
+            self._state.share_id = share.id
+            # remove the shared node
+            yield client.unlink(request.ROOT, req.new_id)
 
+        @defer.inlineCallbacks
         def login2(client):
             """client2 login"""
+            self.addCleanup(client.transport.loseConnection)
             self._state.client2 = client
             client.set_share_change_callback(notif1)
-            d = client.dummy_authenticate("friend")  # for user #1
-            d.addCallback(done_auth)
+            yield client.dummy_authenticate("friend")  # for user #1
+            d2.callback(True)
 
         # setup
         factory = FactoryHelper(login1)
         factory2 = FactoryHelper(login2)
-        d1 = defer.Deferred()
-        d2 = defer.Deferred()
         timeout = reactor.callLater(3, d1.errback, Exception("timeout"))
 
-        def new_client(_):
-            """add the second client"""
-            reactor.connectTCP('localhost', self.port, factory2)
-            return d2
-
         def notif1(share_info):
-            '''First notification, for the created share.'''
+            """First notification, for the created share."""
             self._state.client2.set_share_delete_callback(notif2)
 
         def notif2(share_id):
-            '''Second notification, the tested one.'''
+            """Second notification, the tested one."""
             # find the real share
             self.assertRaises(errors.DoesNotExist,
                               self.usr1.get_share, self._state.share_id)
@@ -1182,50 +1173,13 @@
             factory.timeout.cancel()
             factory2.timeout.cancel()
             timeout.cancel()
-            d1.callback((share_id))
-            self._state.client1.transport.loseConnection()
-            self._state.client2.transport.loseConnection()
-
-        def done_auth(result):
-            """authentication done for client2, we can start making changes"""
-            d2.callback(result)
-
-        def mark_share(_):
-            # mark the share as accepted by hand, so we don't
-            # have to emulate the whole process just for this test
-            share = self.usr1.get_shared_to(accepted=False)[0]
-            share.accept()
-            # also store it in the state for further control
-            self._state.share_id = share.id
-
-        def make_notification(_):
-            """create a change that should create a notification"""
-            # create the share
-            client1 = self._state.client1
-            d = client1.get_root()
-            d.addCallback(self.save_req, 'root_id')
-
-            # create the dir to share
-            d.addCallback(lambda r: client1.make_dir(request.ROOT, r, "hi"))
-            d.addCallback(self.save_req, "req")
-
-            # create the share
-            d.addCallback(
-                lambda _: client1.create_share(
-                    self._state.req.new_id, self.usr1.username,
-                    u"name", Share.VIEW))
-            d.addCallback(mark_share)
-
-            # remove the shared node
-            d.addCallback(
-                lambda _: client1.unlink(request.ROOT, self._state.req.new_id))
+            d1.callback(share_id)
 
         reactor.connectTCP('localhost', self.port, factory)
         return d1
-    test_share_node_deleted.skip = 'LP: #766088'
 
     def test_share_node_overwritten_with_move(self):
-        '''Move something else over the node that was shared.'''
+        """Move something else over the node that was shared."""
         def login1(client):
             """client1 login"""
             self._state.client1 = client
@@ -1253,11 +1207,11 @@
             return d2
 
         def notif1(share_id):
-            '''First notification, for the created share.'''
+            """First notification, for the created share."""
             self._state.client2.set_share_delete_callback(notif2)
 
         def notif2(share_id):
-            '''Second notification, the tested one.'''
+            """Second notification, the tested one."""
             # find the real share
             self.assertRaises(errors.DoesNotExist,
                               self.usr1.get_share, self._state.share_id)

=== modified file 'src/server/tests/test_volumes.py'
--- src/server/tests/test_volumes.py	2015-09-27 17:01:04 +0000
+++ src/server/tests/test_volumes.py	2016-02-27 20:39:13 +0000
@@ -92,8 +92,10 @@
             d.addCallbacks(client.test_done, client.test_fail)
         return self.callback_test(auth)
 
-    def _create_share(self, _, accept=False, dead=False, from_id=1):
+    def _create_share(self, _, accept=False, dead=False, from_id=None):
         """Create the share to me."""
+        if from_id is None:
+            from_id = self.usr1.id
         fromusr = get_storage_user(from_id)
         node = fromusr.root.load()
         share = node.share(self.usr0.id, u"name", readonly=True)
@@ -324,28 +326,29 @@
             self.assertEqual(share.free_bytes,
                              self.usr1.get_quota().free_bytes)
 
+        @defer.inlineCallbacks
         def auth(client):
             """Authenticate and test."""
-            d = client.dummy_authenticate("open sesame")
-            d.addCallback(lambda r: client.get_root())
-            d.addCallback(self.save_req, "root")
+            client.dummy_authenticate("open sesame")
+            root = yield client.get_root()
+            self.save_req(root, "root")
 
             # create two udfs, kill one
-            d.addCallback(lambda _: client.create_udf(u"~/ñ", u"foo"))
-            d.addCallback(self.save_req, "udf")
-            d.addCallback(lambda _: client.create_udf(u"~/moño", u"groovy"))
-            d.addCallback(lambda r: client.delete_volume(r.volume_id))
+            udf = yield client.create_udf(u"~/ñ", u"foo")
+            self.save_req(udf, "udf")
+            result = yield client.create_udf(u"~/moño", u"groovy")
+            yield client.delete_volume(result.volume_id)
 
             # create two shares, one dead (the second one should be the live
             # one because the helper function stores data for comparison)
-            d.addCallback(self._create_share, accept=True, dead=True)
-            d.addCallback(self._create_share, accept=True, from_id=2)
+            self._create_share(None, accept=True, dead=True)
+            self._create_share(None, accept=True, from_id=self.usr2.id)
 
             # list the volumes and check
-            d.addCallback(lambda _: client.list_volumes())
-            d.addCallback(check)
-            d.addCallbacks(client.test_done, client.test_fail)
-        return self.callback_test(auth)
+            req = yield client.list_volumes()
+            check(req)
+
+        return self.callback_test(auth, add_default_callbacks=True)
 
 
 class TestDataWithVolumes(TestWithDatabase):