Merge lp:~nataliabidart/ubuntuone-dev-tools/add-recorder into lp:ubuntuone-dev-tools

[Natalia Bidart]

- Provide a Recorder class, useful for building Fakes, and a class decorator
  to record calls to existent class' methods.

[dobey (dobey)]

Set to Work in Progress as it's not clear this is 'finished' yet. Also, please commit --fixes for the bug # before setting back to Needs Review. Thanks.

[Alejandro J. Cura (alecu)]

Looks fine, tests pass.

review: Approve

[dobey (dobey)]

My first thought is, can we move this to a new module under testing/ instead? I think it might also be good to move the current skip decorators out of the testcases/__init__ and into a new module under testing/ instead as well, and having this be a new module there instead of another huge block of code in testcases/__init__ would be a good start to that end.

What do you think?

review: Needs Information

[Natalia Bidart (nataliabidart)]

> My first thought is, can we move this to a new module under testing/ instead?
> I think it might also be good to move the current skip decorators out of the
> testcases/__init__ and into a new module under testing/ instead as well, and
> having this be a new module there instead of another huge block of code in
> testcases/__init__ would be a good start to that end.
>
> What do you think?

I think it makes sense, a proposal can be:

ubuntuone.devtools.testing.recording

[dobey (dobey)]

So I just had a brilliant idea for some naming here.

ubuntuone.devtools.testing.validation for the module

CallRecorder for the current "Recorder" class name.

@record_calls for the decorator name, if we really need it at all? Do you have an example of how it would be useful in tests only, vs having to define a new wrapper class in the tests which this would wrap anyway? It seems not very useful to have, after thinking about it. Also, the way it works can create problems with class hierarchy when multiple inheritance is used in code (particularly thinking about how TwistedTestCase stuff works as an example, and the problems we've had in the past with it).

Also, do we want the assert methods to be on the BaseTestCase, or do we want a new CallRecorderTestCase for them? Also, standard style for assert calls is to do assertCalled, assertNotCalled, etc… here no?

Unmerged revisions

63. By Natalia Bidart on 2012-03-23

- Attaching bug #963265.

62. By Natalia Bidart on 2012-03-23

- Making the _next_result attribute to be a dict so it supports different
results for every method.

61. By Natalia Bidart on 2012-03-23

- Prefer using class_ instead of klass to conform pep8 better.

60. By Natalia Bidart on 2012-03-23

Forgotten file.

59. By Natalia Bidart on 2012-03-23

- Provide a Recorder class, useful for building Fakes, and a class decorator
  to record existent classes.

=== modified file 'ubuntuone/devtools/testcases/__init__.py'
--- ubuntuone/devtools/testcases/__init__.py	2011-12-08 18:58:43 +0000
+++ ubuntuone/devtools/testcases/__init__.py	2012-03-23 17:43:19 +0000
@@ -23,6 +23,7 @@
 import shutil
 import sys
 
+from collections import defaultdict
 from functools import wraps
 
 from twisted.trial.unittest import TestCase, SkipTest
@@ -104,6 +105,79 @@
 # pylint: enable=C0103
 
 
+class Recorder(object):
+    """A class that records every call clients made to it.
+
+    You can use this class to either:
+
+    - along with the 'add_recorder' decorator, have an existing class to
+      behave as usual but every call made to it will be recorded in the
+      '_called' attribute.
+
+    - quickly create a fake to mimic another class behavior. To do so, you
+      should set _handle_attr_error to True, and define a set of predefined
+      results for each function and assign that to _results.
+
+    Aditionally, every method name listed under '_no_wrap' will not be wrap
+    with the recorder machinery.
+
+    """
+
+    _no_wrap = ['_called']  # do not record calls to these
+    _handle_attr_error = False  # propagate AttributeError
+    _results = {}  # return a given result for a given method
+
+    def __init__(self, *args, **kwargs):
+        super(Recorder, self).__init__()
+        self._called = defaultdict(list)
+
+    def __getattribute__(self, attr_name):
+        """Override so we can record calls to members."""
+        attr_getter = super(Recorder, self).__getattribute__
+        try:
+            # try to get the real result
+            result = attr_getter(attr_name)
+        except AttributeError:
+            if not self._handle_attr_error:
+                raise
+
+            # fake the result if non-existent - useful for faking a class
+            result = lambda *a, **kw: self._results.get(attr_name, None)
+            super(Recorder, self).__setattr__(attr_name, result)
+
+        if attr_name in attr_getter('_no_wrap'):
+            # do not wrap any attributes listed in self.no_wrap
+            return result
+
+        called = attr_getter('_called')
+
+        def wrap_me(f):
+            """Wrap 'f' so we record when 'f' is called."""
+
+            @wraps(f)
+            def inner(*a, **kw):
+                """Keep track of calls to 'f', execute it and return result."""
+                called[attr_name].append((a, kw))
+                return f(*a, **kw)
+
+            return inner
+
+        # if the original result is a callable, wrap it with the recorder
+        if callable(result):
+            return wrap_me(result)
+        else:
+            return result
+
+
+def add_recorder(class_):
+    """Class decorator to wrap 'class_' and record every call made to it."""
+
+    class Recorded(Recorder, class_):
+        """Record every call."""
+
+    return Recorded
+
+
 class BaseTestCase(TestCase):
     """Base TestCase with helper methods to handle temp dir.
 
@@ -164,3 +238,30 @@
         if os.path.exists(parent):
             os.chmod(parent, 0755)
         os.makedirs(path)
+
+    # Access to a protected member _called of a client class
+    # pylint: disable=W0212
+
+    def assert_method_was_called(self, target, method, *args, **kwargs):
+        """Check that 'target.method(*args, **kwargs)' was called."""
+        self.assertTrue(getattr(target, '_called', None) is not None,
+            '%r must have a _called attribute to perform the test.' % target)
+        self.assertTrue(method in target._called,
+            'method %r must be called on %r.' % (method, target))
+        self.assertEqual(target._called[method], [(args, kwargs)])
+
+    def assert_method_was_not_called(self, target, method):
+        """Check that 'target.method' was never called."""
+        self.assertTrue(getattr(target, '_called', None) is not None,
+            '%r must have a _called attribute to perform the test.' % target)
+        self.assertTrue(method not in target._called,
+            'method %r must not be called on %r.' % (method, target))
+
+    def assert_nothing_called(self, target):
+        """Check that no method was called over 'target'."""
+        self.assertTrue(getattr(target, '_called', None) is not None,
+            '%r must have a _called attribute to perform the test.' % target)
+        self.assertEqual(target._called, {},
+            'No method at all must be called on %r.' % target)
+
+    # pylint: enable=W0212

=== added file 'ubuntuone/devtools/testcases/tests/test_recorder.py'
--- ubuntuone/devtools/testcases/tests/test_recorder.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/devtools/testcases/tests/test_recorder.py	2012-03-23 17:43:19 +0000
@@ -0,0 +1,122 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright 2012 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test the Recorder test case."""
+
+from twisted.internet import defer
+
+from ubuntuone.devtools.testcases import add_recorder, BaseTestCase, Recorder
+
+
+class RecordMe(object):
+    """A dummy class to test the add_recorder decorator."""
+
+    def __init__(self):
+        self._private = object()
+        self.somethings = []
+
+    def test_me(self):
+        """Dummy call."""
+        return 'foo'
+
+    def something(self, *a, **kw):
+        """Dummy call."""
+        self.somethings.append((a, kw))
+
+
+class AddRecorderTestCase(BaseTestCase):
+    """Test suite for the add_recorder class decorator."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        yield super(AddRecorderTestCase, self).setUp()
+        self.obj = add_recorder(RecordMe)()
+
+    def test_non_callables(self):
+        """The non-callables attributes are not modified."""
+        self.assertEqual(self.obj.somethings, [])
+        self.assert_nothing_called(self.obj)
+
+    def test_original_result(self):
+        """The original result is returned."""
+        result = self.obj.test_me()
+
+        self.assertEqual(RecordMe().test_me(), result)
+        self.assert_method_was_called(self.obj, 'test_me')
+        self.assert_method_was_not_called(self.obj, 'something')
+
+    def test_result_is_none(self):
+        """If the original result is None, nothing breaks."""
+        args = (1, object(), [object(), ValueError()])
+        kwargs = dict(foo='bar', baz=TypeError())
+
+        result = self.obj.something(*args, **kwargs)
+
+        self.assertTrue(result is None)
+        self.assertEqual(len(self.obj.somethings), 1)
+        self.assertEqual(self.obj.somethings, [(args, kwargs)])
+
+        self.assert_method_was_called(self.obj, 'something', *args, **kwargs)
+        self.assert_method_was_not_called(self.obj, 'test_me')
+
+    def test_attribute_error(self):
+        """AttributeError is propagated."""
+        # pylint: disable=W0108
+        self.assertRaises(AttributeError, lambda: self.obj.bar())
+
+    def test_no_wrap(self):
+        """Names listed under self._no_wrap should not be wrapped."""
+        self.patch(self.obj, '_no_wrap', ['test_me'])
+
+        result = self.obj.test_me()
+
+        self.assertEqual(RecordMe().test_me(), result)
+        self.assert_nothing_called(self.obj)
+
+    def test_can_access_private_attrs(self):
+        """Private attributes can also be accessed."""
+        # pylint: disable=W0212
+        self.assertTrue(self.obj._private, RecordMe()._private)
+
+
+class AddRecorderNoAttrErrorTestCase(AddRecorderTestCase):
+    """Test suite for the add_recorder when handle_attr_error is set."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        self.patch(Recorder, '_handle_attr_error', True)
+        yield super(AddRecorderNoAttrErrorTestCase, self).setUp()
+
+    def test_attribute_error(self):
+        """AttributeError is not propagated."""
+        result = self.obj.bar()
+
+        self.assertTrue(result is None)
+
+    def test_next_result(self):
+        """Handling attribute error, self._results[method] is returned."""
+        expected = object()
+        self.patch(self.obj, '_results', {'foobarbaz': expected})
+
+        result = self.obj.foobarbaz(1, 2, 3, 4)
+        self.assertTrue(result is expected)
+
+    def test_next_result_not_in_results(self):
+        """Handling attribute error, None is returned if no given result."""
+        self.patch(self.obj, '_results', {})
+
+        result = self.obj.foobarbaz(1, 2, 3, 4)
+        self.assertTrue(result is None)