Implement non-optimising resource dependencies/cascading by extending the TestResource interface.

8 files changed, 146 insertions, 21 deletions

diff --git a/NEWS b/NEWS
index 0d1c219..865112e 100644
--- a/NEWS
+++ b/NEWS
@@ -42,6 +42,12 @@ IN DEVELOPMENT
 
     * SampleTestResource has been removed. (Jonathan Lange)
 
+    * TestResource.make has had an API change: it must now accept a
+      dependency_resources parameter which is a dictionary listing the
+      dependencies that will be provided to the resource. This parameter is
+      provided so the resource can access its dependencies during setUp, if
+      needed. (Robert Collins)
+
     * TestResource subclasses should override 'make' and 'clean' where they
       previously overrode '_makeResource' and '_cleanResource'.
       (Jonathan Lange)
diff --git a/README b/README
index be1edd2..69a28a5 100644
--- a/README
+++ b/README
@@ -40,16 +40,22 @@ There are three main components to make testresources work:
 
 1) testresources.TestResource
 
-A TestResource is an object that tests can use, which provides a getResource()
-method that returns an object implementing whichever interface the client
-needs, and which will accept that same object back on its finishedWith()
-method.
+A TestResource is an object that tests can use. Usually a subclass of
+testresources.TestResource, with the getResource method overridden. This method
+should return an object providing the resource that the client needs (which can
+Optionally, the clean() method can be overridden if the resource needs to take
+action to clean up external resources (e.g. threads, disk files, ...).
+The 'resources' list on the TestResource object is used to declare
+dependencies. For instance, a DataBaseResource that needs a TemporaryDirectory
+might be declared with a resources list::
 
-Most importantly, two getResources to the same TestResource with no
-finishedWith call in the middle, should return the same object.
+    class DataBaseResource(TestResource):
+
+        resources = [("scratchdir", TemporaryDirectoryResource())]
 
-    XXX the same object requirement may not be needed - but for expensive
-    resources that is the optimisation goal. -- rbc
+Most importantly, two getResources to the same TestResource with no
+finishedWith call in the middle, will return the same object as long as it has
+not been marked dirty.
 
 The goals for TestResources that cannot finish properly are not yet clear, so
 for now the behaviour will to silently continue.
diff --git a/TODO b/TODO
index 02fbd44..e654fe7 100644
--- a/TODO
+++ b/TODO
@@ -10,6 +10,7 @@ Tasks
 
 * Test exceptions being raised from make and clean
 
+* More docs.
 
 Questions
 =========
@@ -17,7 +18,7 @@ Questions
 * Why does finishedWith take a parameter? Why not use
   TestResource._currentResource?
 
-* How should resources be composed?
+* How should resources be composed? (Partially answered).
 
 * How can testresources be used with layers?
 
diff --git a/doc/example.py b/doc/example.py
index 6e02d5c..d7c0187 100644
--- a/doc/example.py
+++ b/doc/example.py
@@ -27,5 +27,17 @@ class SampleTestResource(TestResource):
     setUpCost = 2
     tearDownCost = 2
 
-    def make(self):
+    def make(self, dependency_resources):
         return "You need to implement your own getResource."
+
+
+class MyResource(object):
+    """My pet resource."""
+
+
+class SampleWithDependencies(TestResource):
+
+    resources = [('foo', SampleTestResource()), ('bar', SampleTestResource())]
+
+    def make(self, dependency_resources):
+        return MyResource()
diff --git a/lib/testresources/__init__.py b/lib/testresources/__init__.py
index 0228421..3db40bd 100644
--- a/lib/testresources/__init__.py
+++ b/lib/testresources/__init__.py
@@ -171,6 +171,14 @@ class TestLoader(unittest.TestLoader):
 class TestResource(object):
     """A resource that can be shared across tests.
 
+    :cvar resources: The same as the resources list on an instance, the default
+        constructor will look for the class instance and copy it. This is a
+        convenience to avoid needing to define __init__ solely to alter the
+        dependencies list.
+    :ivar resources: The resources that this resource needs. Calling
+        neededResources will return the closure of this resource and its needed
+        resources. The resources list is in the same format as resources on a 
+        test case - a list of tuples (attribute_name, resource).
     :ivar setUpCost: The relative cost to construct a resource of this type.
          One good approach is to set this to the number of seconds it normally
          takes to set up the resource.
@@ -186,6 +194,13 @@ class TestResource(object):
         self._dirty = False
         self._uses = 0
         self._currentResource = None
+        self.resources = list(getattr(self.__class__, resources, []))
+
+    def clean_all(self, resource):
+        """Clean the dependencies from resource, and then resource itself."""
+        for name, manager in self.resources:
+            manager.finishedWith(getattr(resource, name))
+        self.clean(resource)
 
     def clean(self, resource):
         """Override this to class method to hook into resource removal."""
@@ -211,7 +226,7 @@ class TestResource(object):
         """
         self._uses -= 1
         if self._uses == 0:
-            self.clean(resource)
+            self.clean_all(resource)
             self._setResource(None)
         elif self._dirty:
             self._resetResource(resource)
@@ -225,20 +240,51 @@ class TestResource(object):
         that it is no longer needed.
         """
         if self._uses == 0:
-            self._setResource(self.make())
+            self._setResource(self.make_all())
         elif self._dirty:
             self._resetResource(self._currentResource)
         self._uses += 1
         return self._currentResource
 
-    def make(self):
-        """Override this to construct resources."""
+    def make_all(self):
+        """Make the dependencies of this resource and this resource."""
+        dependency_resources = {}
+        for name, resource in self.resources:
+            dependency_resources[name] = resource.getResource()
+        result = self.make(dependency_resources)
+        for name, value in dependency_resources.items():
+            setattr(result, name, value)
+        return result
+
+    def make(self, dependency_resources):
+        """Override this to construct resources.
+        
+        :param dependency_resources: A dict mapping name -> resource instance
+            for the resources specified as dependencies.
+        """
         raise NotImplementedError(
             "Override make to construct resources.")
 
+    def neededResources(self):
+        """Return the resources needed for this resource, including self.
+        
+        :return: A list of needed resources, in topological deepest-first
+            order.
+        """
+        seen = set([self])
+        result = []
+        for name, resource in self.resources:
+            for resource in resource.neededResources():
+                if resource in seen:
+                    continue
+                seen.add(resource)
+                result.append(resource)
+        result.append(self)
+        return result
+
     def _resetResource(self, old_resource):
-        self.clean(old_resource)
-        self._setResource(self.make())
+        self.clean_all(old_resource)
+        self._setResource(self.make_all())
 
     def _setResource(self, new_resource):
         """Set the current resource to a new value."""
diff --git a/lib/testresources/tests/test_optimising_test_suite.py b/lib/testresources/tests/test_optimising_test_suite.py
index a707212..2b5ff2c 100644
--- a/lib/testresources/tests/test_optimising_test_suite.py
+++ b/lib/testresources/tests/test_optimising_test_suite.py
@@ -43,7 +43,7 @@ class MakeCounter(testresources.TestResource):
     def clean(self, resource):
         self.cleans += 1
 
-    def make(self):
+    def make(self, dependency_resources):
         self.makes += 1
         return "boo"
 
diff --git a/lib/testresources/tests/test_resourced_test_case.py b/lib/testresources/tests/test_resourced_test_case.py
index 31b29fa..73fffac 100644
--- a/lib/testresources/tests/test_resourced_test_case.py
+++ b/lib/testresources/tests/test_resourced_test_case.py
@@ -35,10 +35,14 @@ class MockResource(testresources.TestResource):
         testresources.TestResource.__init__(self)
         self._resource = resource
 
-    def make(self):
+    def make(self, dependency_resources):
         return self._resource
 
 
+class MockResourceInstance(object):
+    """A resource instance."""
+
+
 class TestResourcedTestCase(testtools.TestCase):
 
     def setUp(self):
@@ -65,6 +69,17 @@ class TestResourcedTestCase(testtools.TestCase):
         self.assertEqual(self.resource, self.resourced_case.foo)
         self.assertEqual('bar_resource', self.resourced_case.bar)
 
+    def testSetUpResourcesSetsUpDependences(self):
+        resource = MockResourceInstance()
+        self.resource_manager = MockResource(resource)
+        self.resourced_case.resources = [('foo', self.resource_manager)]
+        # Give the 'foo' resource access to a 'bar' resource
+        self.resource_manager.resources.append(
+            ('bar', MockResource('bar_resource')))
+        self.resourced_case.setUpResources()
+        self.assertEqual(resource, self.resourced_case.foo)
+        self.assertEqual('bar_resource', self.resourced_case.foo.bar)
+
     def testSetUpUsesResource(self):
         # setUpResources records a use of each declared resource.
         self.resourced_case.resources = [("foo", self.resource_manager)]
@@ -85,6 +100,18 @@ class TestResourcedTestCase(testtools.TestCase):
         self.resourced_case.tearDownResources()
         self.assertEqual(self.resource_manager._uses, 0)
 
+    def testTearDownResourcesStopsUsingDependencies(self):
+        resource = MockResourceInstance()
+        dep1 = MockResource('bar_resource')
+        self.resource_manager = MockResource(resource)
+        self.resourced_case.resources = [('foo', self.resource_manager)]
+        # Give the 'foo' resource access to a 'bar' resource
+        self.resource_manager.resources.append(
+            ('bar', dep1))
+        self.resourced_case.setUpResources()
+        self.resourced_case.tearDownResources()
+        self.assertEqual(dep1._uses, 0)
+
     def testSingleWithSetup(self):
         # setUp and tearDown invoke setUpResources and tearDownResources.
         self.resourced_case.resources = [("foo", self.resource_manager)]
diff --git a/lib/testresources/tests/test_test_resource.py b/lib/testresources/tests/test_test_resource.py
index 59f8729..754ecd3 100644
--- a/lib/testresources/tests/test_test_resource.py
+++ b/lib/testresources/tests/test_test_resource.py
@@ -40,7 +40,7 @@ class MockResource(testresources.TestResource):
     def clean(self, resource):
         self.cleans += 1
 
-    def make(self):
+    def make(self, dependency_resources):
         self.makes += 1
         return "Boo!"
 
@@ -48,7 +48,8 @@ class MockResource(testresources.TestResource):
 class TestTestResource(testtools.TestCase):
 
     def testUnimplementedGetResource(self):
-        # By default, TestResource raises NotImplementedError on getResource.
+        # By default, TestResource raises NotImplementedError on getResource
+        # because make is not defined initially.
         resource_manager = testresources.TestResource()
         self.assertRaises(NotImplementedError, resource_manager.getResource)
 
@@ -64,6 +65,32 @@ class TestTestResource(testtools.TestCase):
         resource_manager = testresources.TestResource()
         self.assertEqual(None, resource_manager._currentResource)
 
+    def testneededResourcesDefault(self):
+        # Calling neededResources on a default TestResource returns the
+        # resource.
+        resource = testresources.TestResource()
+        self.assertEqual([resource], resource.neededResources())
+
+    def testneededResourcesDependenciesFirst(self):
+        # Calling neededResources on a TestResource with dependencies puts the
+        # dependencies first.
+        resource = testresources.TestResource()
+        dep1 = testresources.TestResource()
+        dep2 = testresources.TestResource()
+        resource.resources.append(("dep1", dep1))
+        resource.resources.append(("dep2", dep2))
+        self.assertEqual([dep1, dep2, resource], resource.neededResources())
+
+    def testneededResourcesClosure(self):
+        # Calling neededResources on a TestResource with dependencies includes
+        # the needed resources of the needed resources.
+        resource = testresources.TestResource()
+        dep1 = testresources.TestResource()
+        dep2 = testresources.TestResource()
+        resource.resources.append(("dep1", dep1))
+        dep1.resources.append(("dep2", dep2))
+        self.assertEqual([dep2, dep1, resource], resource.neededResources())
+
     def testDefaultCosts(self):
         # The base TestResource costs 1 to set up and to tear down.
         resource_manager = testresources.TestResource()
@@ -73,7 +100,7 @@ class TestTestResource(testtools.TestCase):
     def testGetResourceReturnsMakeResource(self):
         resource_manager = MockResource()
         resource = resource_manager.getResource()
-        self.assertEqual(resource_manager.make(), resource)
+        self.assertEqual(resource_manager.make({}), resource)
 
     def testGetResourceIncrementsUses(self):
         resource_manager = MockResource()