Merge lp:~jml/testtools/extract-factory into lp:~testtools-committers/testtools/trunk

[Jonathan Lange]

This branch follows the advice that I recall receiving from Rob ages ago: it extracts a separate factory class out of TestCase.

It's fairly straightforward, but there are a few questions that came to mind while I was doing the work:

testtools/factory.py:11:
  XXX: Are we happy with the name ObjectFactory?

testtools/factory.py:13:
  XXX: Is this a good opportunity to change the getUniqueInteger and
  getUniqueString methods here to be get_unique_integer and get_unique_string?

testtools/testcase.py:196:
  XXX: We could have a class variable factory_factory (except with a
  better name) and then instead write:
      self.factory = self.factory_factory(self)
  Which would allow others to plugin in their factories more easily,
  rather than using the TestCaseWithFactory mixin pattern. Just a
  thought.

Would be interested in opinions.

jml

[Robert Collins (lifeless)]

Does the factory need to be exposed at all?

On 15/12/2011 10:19 PM, "Jonathan Lange" <email address hidden> wrote:

Jonathan Lange has proposed merging lp:~jml/testtools/extract-factory into
lp:testtools.

Requested ...
Your team testtools developers is subscribed to branch lp:testtools.

=== modified file 'NEWS'
--- NEWS 2011-12-05 15:33:37 +0000
+++ NEWS 2011-12-15 11:18:22 +0000
@@ -14,6 +14,14 @@
  ``MatchesAll`` with keyword arguments, then this change might affect your
  test results. (Jonathan Lange)

+* ``TestCase`` now has a ``factory`` attribute, set to an instance of
+ ``ObjectFactory``. It now uses this instance for generating unique
strings
+ and integers. (Jonathan Lange)
+
+* ``TestCase.getUniqueInteger`` and ``TestCase.getUniqueString`` are now
+ deprecated. Use ``TestCase.factory.getUniqueInteger`` and
+ ``TestCase.factory.getUniqueString`` instead. (Jonathan Lange)
+
 Improvements
 ------------

=== modified file 'doc/for-test-authors.rst'
--- doc/for-test-authors.rst 2011-12-07 11:32:45 +0000
+++ doc/for-test-authors.rst 2011-12-15 11:18:22 +0000
@@ -1240,17 +1240,19 @@
 fine. However, sometimes it's useful to be able to create arbitrary
objects
 at will, without having to make up silly sample data.

-To help with this, ``testtools.TestCase`` implements creation methods
called
-``getUniqueString`` and ``getUniqueInteger``. They return strings and
-integers that are unique within the context of the test that can be used to
-assemble more complex objects. Here's a basic example where
-``getUniqueString`` is used instead of saying "foo" or "bar" or whatever::
+To help with this, ``testtools.TestCase`` comes with an ``ObjectFactory``
that
+you can access as the ``factory`` attribute within your test
+case. ``ObjectFactory`` implements creation methods called
``getUniqueString``
+and ``getUniqueInteger``. They return strings and integers that are unique
+within the context of the test that can be used to assemble more complex
+objects. Here's a basic example where ``getUniqueString`` is used instead
of
+saying "foo" or "bar" or whatever::

  class SomeTest(TestCase):

      def test_full_name(self):
- first_name = self.getUniqueString()
- last_name = self.getUniqueString()
+ first_name = self.factory.getUniqueString()
+ last_name = self.factory.getUniqueString()
          p = Person(first_name, last_name)
          self.assertEqual(p.full_name, "%s %s" % (first_name, last_name))

@@ -1260,13 +1262,15 @@
  class TestCoupleLogic(TestCase):

      def make_arbitrary_person(self):
- return Person(self.getUniqueString(), self.getUniqueString())
+ return Person(
+ self.factory.getUniqueString(),
+ self.factory.getUniqueString())

      def test_get_invitation(self):
          a = self.make_arbitrary_person()
          b = self.make_arbitrary_person()
          couple = Couple(a, b)
- event_name = self.getUniqueString()
+ event_name = self.factory.getUniqueString()
          invitation = couple.get_invitation(event_name)
          self.assertEqual(
              invitation,

=== modified file 'testtools/__init_...

[Jonathan Lange (jml)]

On Thu, Dec 15, 2011 at 1:13 PM, Robert Collins
<email address hidden> wrote:
> Does the factory need to be exposed at all?

As in, does it need to be in the public API?

Not really, but it would allow Launchpad to delete a little bit of
code from its ObjectFactory, and make it easier for other projects to
build their own domain-specific factories.

jml

[Robert Collins (lifeless)]

I meant 'does the factory need to be a test case attribute'. To me, the factory is a multipurpose fixture, and fixtures are very usable (e.g. via scenarios) without being exposed on the testcase by default.

From the point of view of compat with prior testtools releases we can't drop our additional methods, but we can avoid adding an overly specific factory instance on every TestCase object. So putting it on e.g. __factory. That would let the new factory be used by other testing styles (e.g. nose), or evolved separately to TestCase.

Beyond that there is an interesting discussion about whether global factories are a good idea, or whether per test state is a better idea, and a little orthogonally, whether the factory should be a Fixture (which can then provide details etc) or an adhoc API.

So the .factory is the only thing that really needs fixing, I'll mark this approved if you change factory to __factory.

review: Approve

[lp:~jml/testtools/extract-factory]

242. By Jonathan Lange on 2012-05-02

Unexpose TestCase.factory, and recommend that people use ObjectFactory for themselves.

[Jonathan Lange (jml)]

On Wed, May 2, 2012 at 4:56 AM, Robert Collins
<email address hidden> wrote:
> Review: Approve
>
> I meant 'does the factory need to be a test case attribute'. To me, the factory is a multipurpose fixture, and fixtures are very usable (e.g. via scenarios) without being exposed on the testcase by default.
>
> >From the point of view of compat with prior testtools releases we can't drop our additional methods, but we can avoid adding an overly specific factory instance on every TestCase object. So putting it on e.g. __factory. That would let the new factory be used by other testing styles (e.g. nose), or evolved separately to TestCase.
>

OK. Thanks for clarifying.

FWIW, the price of this is making the factory slightly less convenient
to start using.

e.g. from

  x = self.factory.getUniqueString()

To

  factory = ObjectFactory()
  x = factory.getUniqueString()

To perhaps even

  def setUp(self):
    super(MyTestCase, self).setUp()
    self.factory = factory

  def test_something(self):
    x = self.factory.getUniqueString()

The structural correctness is probably worth this cost for now.

> Beyond that there is an interesting discussion about whether global factories are a good idea, or whether per test state is a better idea, and a little orthogonally, whether the factory should be a Fixture (which can then provide details etc) or an adhoc API.

I'd be interested in having that discussion ... later :)

>
> So the .factory is the only thing that really needs fixing, I'll mark this approved if you change factory to __factory.

I really don't like double underscores. But sure, let's get this
landed. Changing to drop one is easy and very low risk.

jml

[Robert Collins (lifeless)]

So I think this became good to merge but we need to port it to git.

Unmerged revisions

242. By Jonathan Lange on 2012-05-02

Unexpose TestCase.factory, and recommend that people use ObjectFactory for themselves.

241. By Jonathan Lange on 2011-12-15

Documentation update.

240. By Jonathan Lange on 2011-12-15

News.

239. By Jonathan Lange on 2011-12-15

Some thoughts on options
Deprecation
Expose ObjectFactory at the top-level.

238. By Jonathan Lange on 2011-12-15

Use ObjectFactory rather than implementing it ourselves.

237. By Jonathan Lange on 2011-12-15

Extract out a factory.

=== modified file 'NEWS'
--- NEWS	2012-05-02 03:41:51 +0000
+++ NEWS	2012-05-02 10:33:21 +0000
@@ -81,6 +81,13 @@
   ``MatchesAll`` with keyword arguments, then this change might affect your
   test results.  (Jonathan Lange)
 
+* ``ObjectFactory`` extracts out the ``getUniqueInteger`` and
+  ``getUniqueString`` behaviour from ``TestCase``.   (Jonathan Lange)
+
+* ``TestCase.getUniqueInteger`` and ``TestCase.getUniqueString`` are now
+  deprecated.  Use ``ObjectFactory.getUniqueInteger`` and
+  ``ObjectFactory.getUniqueString`` instead.  (Jonathan Lange)
+
 Improvements
 ------------
 

=== modified file 'doc/for-test-authors.rst'
--- doc/for-test-authors.rst	2012-01-29 14:08:39 +0000
+++ doc/for-test-authors.rst	2012-05-02 10:33:21 +0000
@@ -1240,17 +1240,19 @@
 fine.  However, sometimes it's useful to be able to create arbitrary objects
 at will, without having to make up silly sample data.
 
-To help with this, ``testtools.TestCase`` implements creation methods called
-``getUniqueString`` and ``getUniqueInteger``.  They return strings and
-integers that are unique within the context of the test that can be used to
-assemble more complex objects.  Here's a basic example where
+To help with this, ``testtools.TestCase`` comes with an ``ObjectFactory`` that
+you can use within your test case. ``ObjectFactory`` implements creation
+methods called ``getUniqueString`` and ``getUniqueInteger``.  They return
+strings and integers that are unique within the context of the test that can
+be used to assemble more complex objects.  Here's a basic example where
 ``getUniqueString`` is used instead of saying "foo" or "bar" or whatever::
 
   class SomeTest(TestCase):
 
       def test_full_name(self):
-          first_name = self.getUniqueString()
-          last_name = self.getUniqueString()
+          factory = ObjectFactory()
+          first_name = factory.getUniqueString()
+          last_name = factory.getUniqueString()
           p = Person(first_name, last_name)
           self.assertEqual(p.full_name, "%s %s" % (first_name, last_name))
 
@@ -1259,14 +1261,20 @@
 
   class TestCoupleLogic(TestCase):
 
+      def setUp(self):
+          super(TestCoupleLogic, self).setUp()
+          self.factory = ObjectFactory()
+
       def make_arbitrary_person(self):
-          return Person(self.getUniqueString(), self.getUniqueString())
+          return Person(
+              self.factory.getUniqueString(),
+              self.factory.getUniqueString())
 
       def test_get_invitation(self):
           a = self.make_arbitrary_person()
           b = self.make_arbitrary_person()
           couple = Couple(a, b)
-          event_name = self.getUniqueString()
+          event_name = self.factory.getUniqueString()
           invitation = couple.get_invitation(event_name)
           self.assertEqual(
               invitation,

=== modified file 'testtools/__init__.py'
--- testtools/__init__.py	2012-04-13 12:33:46 +0000
+++ testtools/__init__.py	2012-05-02 10:33:21 +0000
@@ -12,6 +12,7 @@
     'iterate_tests',
     'MultipleExceptions',
     'MultiTestResult',
+    'ObjectFactory',
     'PlaceHolder',
     'run_test_with',
     'Tagger',
@@ -30,6 +31,7 @@
     'try_imports',
     ]
 
+from testtools.factory import ObjectFactory
 from testtools.helpers import (
     try_import,
     try_imports,

=== added file 'testtools/factory.py'
--- testtools/factory.py	1970-01-01 00:00:00 +0000
+++ testtools/factory.py	2012-05-02 10:33:21 +0000
@@ -0,0 +1,48 @@
+# Copyright (c) 2008-2011 testtools developers. See LICENSE for details.
+
+__all__ = [
+    'ObjectFactory',
+    ]
+
+import itertools
+
+from testtools.compat import advance_iterator
+
+# XXX: Are we happy with the name ObjectFactory?
+
+# XXX: Is this a good opportunity to change the getUniqueInteger and
+# getUniqueString methods here to be get_unique_integer and get_unique_string?
+
+class ObjectFactory(object):
+
+    DEFAULT_PREFIX = 'unique'
+
+    def __init__(self, prefix=None):
+        if prefix is None:
+            prefix = self.DEFAULT_PREFIX
+        self._prefix = prefix
+        self._unique_id_gen = itertools.count(1)
+
+    def getUniqueInteger(self):
+        """Get an integer unique to this test.
+
+        Returns an integer that is guaranteed to be unique to this instance.
+        Use this when you need an arbitrary integer in your test, or as a
+        helper for custom anonymous factory methods.
+        """
+        return advance_iterator(self._unique_id_gen)
+
+    def getUniqueString(self, prefix=None):
+        """Get a string unique to this test.
+
+        Returns a string that is guaranteed to be unique to this instance. Use
+        this when you need an arbitrary string in your test, or as a helper
+        for custom anonymous factory methods.
+
+        :param prefix: The prefix of the string. If not provided, defaults
+            to the id of the tests.
+        :return: A bytestring of '<prefix>-<unique_int>'.
+        """
+        if prefix is None:
+            prefix = self._prefix
+        return '%s-%d' % (prefix, self.getUniqueInteger())

=== modified file 'testtools/testcase.py'
--- testtools/testcase.py	2012-04-03 07:39:43 +0000
+++ testtools/testcase.py	2012-05-02 10:33:21 +0000
@@ -28,6 +28,7 @@
     advance_iterator,
     reraise,
     )
+from testtools.factory import ObjectFactory
 from testtools.matchers import (
     Annotate,
     Contains,
@@ -171,7 +172,6 @@
         runTest = kwargs.pop('runTest', None)
         super(TestCase, self).__init__(*args, **kwargs)
         self._cleanups = []
-        self._unique_id_gen = itertools.count(1)
         # Generators to ensure unique traceback ids.  Maps traceback label to
         # iterators.
         self._traceback_id_gens = {}
@@ -196,6 +196,7 @@
         if sys.version_info < (2, 6):
             # Catch old-style string exceptions with None as the instance
             self.exception_handlers.append((type(None), self._report_error))
+        self.__factory = ObjectFactory(self.id())
 
     def __eq__(self, other):
         eq = getattr(unittest.TestCase, '__eq__', None)
@@ -447,16 +448,20 @@
             raise _UnexpectedSuccess(reason)
 
     def getUniqueInteger(self):
-        """Get an integer unique to this test.
+        """DEPRECATED - Get an integer unique to this test.
+
+        Use ``ObjectFactory.getUniqueInteger()`` instead.
 
         Returns an integer that is guaranteed to be unique to this instance.
         Use this when you need an arbitrary integer in your test, or as a
         helper for custom anonymous factory methods.
         """
-        return advance_iterator(self._unique_id_gen)
+        return self.factory.getUniqueInteger()
 
     def getUniqueString(self, prefix=None):
-        """Get a string unique to this test.
+        """DEPRECATED - Get a string unique to this test.
+
+        Use ``ObjectFactory.getUniqueString()`` instead.
 
         Returns a string that is guaranteed to be unique to this instance. Use
         this when you need an arbitrary string in your test, or as a helper
@@ -466,9 +471,7 @@
             to the id of the tests.
         :return: A bytestring of '<prefix>-<unique_int>'.
         """
-        if prefix is None:
-            prefix = self.id()
-        return '%s-%d' % (prefix, self.getUniqueInteger())
+        return self.factory.getUniqueString(prefix=prefix)
 
     def onException(self, exc_info, tb_label='traceback'):
         """Called when an exception propogates from test code.

=== modified file 'testtools/tests/__init__.py'
--- testtools/tests/__init__.py	2012-04-11 11:00:52 +0000
+++ testtools/tests/__init__.py	2012-05-02 10:33:21 +0000
@@ -12,6 +12,7 @@
         test_content_type,
         test_deferredruntest,
         test_distutilscmd,
+        test_factory,
         test_fixturesupport,
         test_helpers,
         test_matchers,
@@ -30,6 +31,7 @@
         test_content_type,
         test_deferredruntest,
         test_distutilscmd,
+        test_factory,
         test_fixturesupport,
         test_helpers,
         test_matchers,

=== added file 'testtools/tests/test_factory.py'
--- testtools/tests/test_factory.py	1970-01-01 00:00:00 +0000
+++ testtools/tests/test_factory.py	2012-05-02 10:33:21 +0000
@@ -0,0 +1,47 @@
+from testtools import TestCase
+from testtools.factory import ObjectFactory
+from testtools.matchers import Equals
+
+
+class TestFactory(TestCase):
+
+    def test_getUniqueInteger(self):
+        # getUniqueInteger returns an integer that increments each time you
+        # call it.
+        factory = ObjectFactory()
+        one = factory.getUniqueInteger()
+        self.assertEqual(1, one)
+        two = factory.getUniqueInteger()
+        self.assertEqual(2, two)
+
+    def test_getUniqueString_default_prefix(self):
+        # If no other parameters are given, getUniqueString returns a string
+        # starting with the default prefix followed by a unique integer.
+        factory = ObjectFactory()
+        name_one = factory.getUniqueString()
+        self.assertEqual('%s-%d' % (factory.DEFAULT_PREFIX, 1), name_one)
+        name_two = factory.getUniqueString()
+        self.assertEqual('%s-%d' % (factory.DEFAULT_PREFIX, 2), name_two)
+
+    def test_getUniqueString_early_prefix(self):
+        # An optional prefix can be given to the factory.  If so, then all
+        # getUniqueString calls use that as a prefix by default.
+        factory = ObjectFactory(prefix=self.id())
+        name_one = factory.getUniqueString()
+        self.assertEqual('%s-%d' % (self.id(), 1), name_one)
+        name_two = factory.getUniqueString()
+        self.assertEqual('%s-%d' % (self.id(), 2), name_two)
+
+    def test_getUniqueString_late_prefix(self):
+        # If getUniqueString is given an argument, it uses that argument as
+        # the prefix of the unique string, rather than the test id.
+        factory = ObjectFactory()
+        name_one = factory.getUniqueString('foo')
+        self.assertThat(name_one, Equals('foo-1'))
+        name_two = factory.getUniqueString('bar')
+        self.assertThat(name_two, Equals('bar-2'))
+
+
+def test_suite():
+    from unittest import TestLoader
+    return TestLoader().loadTestsFromName(__name__)