Review feedback - make the trace happen around make and clean not just before, and pass structed info not a formatted string.

2 files changed, 12 insertions, 6 deletions

diff --git a/lib/testresources/__init__.py b/lib/testresources/__init__.py
index adccecd..dc373dd 100644
--- a/lib/testresources/__init__.py
+++ b/lib/testresources/__init__.py
@@ -208,14 +208,15 @@ class TestResource(object):
         self._uses = 0
         self._currentResource = None
         self.resources = list(getattr(self.__class__, "resources", []))
-        self._trace = trace_function or (lambda x:"")
+        self._trace = trace_function or (lambda x,y,z:"")
 
     def _clean_all(self, resource):
         """Clean the dependencies from resource, and then resource itself."""
-        self._trace("clean: %s\n" % self)
+        self._trace("clean", "start", self)
         self.clean(resource)
         for name, manager in self.resources:
             manager.finishedWith(getattr(resource, name))
+        self._trace("clean", "stop", self)
 
     def clean(self, resource):
         """Override this to class method to hook into resource removal."""
@@ -279,13 +280,14 @@ class TestResource(object):
 
     def _make_all(self):
         """Make the dependencies of this resource and this resource."""
-        self._trace("make: %s\n" % self)
+        self._trace("make", "start", self)
         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)
+        self._trace("make", "stop", self)
         return result
 
     def make(self, dependency_resources):
diff --git a/lib/testresources/tests/test_test_resource.py b/lib/testresources/tests/test_test_resource.py
index 83abfbe..392f891 100644
--- a/lib/testresources/tests/test_test_resource.py
+++ b/lib/testresources/tests/test_test_resource.py
@@ -278,10 +278,14 @@ class TestTestResource(testtools.TestCase):
 
     def testTraceFunction(self):
         output = []
-        resource_manager = MockResource(trace_function=output.append)
-        expected = ["make: %s\n" % resource_manager]
+        def trace(operation, phase, mgr):
+            output.append((operation, phase, mgr))
+        resource_manager = MockResource(trace_function=trace)
+        expected = [("make", "start", resource_manager),
+            ("make", "stop", resource_manager)]
         r = resource_manager.getResource()
         self.assertEqual(expected, output)
-        expected.append("clean: %s\n" % resource_manager)
+        expected.extend([("clean", "start", resource_manager),
+            ("clean", "stop", resource_manager)])
         resource_manager.finishedWith(r)
         self.assertEqual(expected, output)