[rfc] testtools test fixtures

[Martin Pool]

For context see

* "[Launchpad-dev] New feature in Launchpad TestCase base class"
* https://code.launchpad.net/~mbp/bzr/testsubjects-old/+merge/24188
* https://code.launchpad.net/~mbp/bzr/fixtures/+merge/25337

I'd (we'd?) like to add a concept of "test fixture" to testtools (the
library that provides the base of our TestCase), so that we can
separate out some aspects of test setup.  Here are some rough notes
from a conversation with Ian and Andrew about it.

I'd like to just start pulling things out into fixtures, and that's
probably worth doing in parallel with this, but I'd like to eventually
make them follow consistent patterns across testtools and bzrlib.

-- 
Martin
-------------- next part --------------
TestFixtures
------------

TestFixtures give a way to separate out the creation of objects used by a test.

This allows:

 * Automatic cleanup of test fixtures when the test completes.

 * Reuse of test fixture code across different tests.

 * Construction of multiple similar fixtures within a single test.

 * The choice of which particular values are used by the fixture is normally
   abstracted-away by the fixture, but the test can provide some parameters
   giving constraints on the value.

 * Making it easy to specify the aspects of the fixture that you care
   about while ignoring the rest.

 * Tests to use "challenging" test data, not inadvertently easy things:
   for instance that they have a directory with unicode names, or a branch
   with representative contents of various kinds.  If the fixture setup is 
   done inline people tend to choose easy values.

 * Test data to adapt to suit the environment: for example, using Unicode 
   filenames if and only if they're supported.

 * Perhaps, as a debugging tool, to allow developers to tweak the
   constructed values to be harder or easier.

These can be seen as an implementation of the *Delegated Setup* and *Creation
Method* xUnit patterns.  It can be used to implement the *Parameterized
Anonymous Creation Method*, meaning that the caller can optionally specify
some parameters about the object created and it generates distinct values for
each call, isolated from other tests.

This is an extended version of the creation patterns in `getUniqueString` and 
`getUniqueInteger` described above.  Those methods accumulate state in the
TestCase.

Fixtures can provide a setUp and tearDown method.  Normally anything expensive
(beyond just saving constructor parameters into members) should be deferred
until `setUp` is called.

    from testtools.fixture import Fixture
 
    class BranchFixture(Fixture):

        def setUp(self):
            # make a branch in a temporary directory
            pass

        def tearDown(self):
            pass

Normally test fixtures will be constructed near the start of the test method
that uses them, and a reference to them will be held in a local variable of
that test method.  The Fixture object is created by the test code and passed
to `useFixture`, which starts it and arranges for it to be torn down by a
cleanup from the test case::

    class TestSomething(TestCase):
  
        def test_branch_last_revision(self):
            # branch = self.useFixture(BranchFixture())
            self.assertTrue(branch.last_revision()

Open questions
~~~~~~~~~~~~~~

 * Should the fixture be an actual domain/application object, or a factory for
   them?  (If a factory, what do we call the thing it produces?)

   Many domain objects (citation needed) don't need any cleanup: you just
   construct them using appropriate underlying resources and use them.
   They're either just in memory, or they'll be implicitly destroyed when
   the containing object (eg Transport or directory) is destroyed.

 * Should fixtures normally know about the TestCase that's using them?
   Preferably not?  Perhaps we want them to know about the testcase as a
   place to hold default resources they can use?  Perhaps there should be
   a `TestCase.fixtures` dict they hold?

 * Fixtures may need to accumulate some state so that you can for example
   get a series of unique values from them: is this state held in the fixture
   object?  (That would imply it needs to be a kind of factory.)

 * What about if you have dependencies between different fixtures, such as 
   wanting a repository and some branches within it?  Should references to
   them be passed in to the constructor?  (And do you pass the fixture/factory 
   or the actual instance?)

 * Are we interested in sharing expensive resources across multiple tests,
   as is done by TestResources?  Perhaps the Resource could just be a Fixture 
   that holds its own state.  (In that case `tearDown` would just mean 
   "finished one test" not "completely tear down".)

 * What if the test damages the fixture such that the tearDown will fail, and
   this is expected so we're not interested in seeing an error about it? 
   (For example if the fixture's a directory that's deleted during the test,
   or a repository whose format is corrupted.)

Interesting cases
~~~~~~~~~~~~~~~~~

 * Generating some filenames to put into a tree: something like 'name%d' will 
   do, with a counter that runs up over the course of a test.

 * Making a working tree full of some representative content.

 * Generating some history into a branch.

 * "Needs isolation of a particular type": for instance we clear out lots of
   environment variables in the base TestCase class because they *might* be
   used in a subclass, even though in many cases they're probably not.  And
   this would be a reasonable case to have *Suite Fixture Setup*: some types
   of isolation need to only be done once per whole test run, because we're
   concerned about isolating the tests from the user, not the tests from each 
   other.

Parameterized creation
~~~~~~~~~~~~~~~~~~~~~~

bzr tests at the moment have trouble smoothly dialing up the amount of
detail you want to specify about the system under test: if you care about
specifying say file ids in history then you tend to need to specify a lot
more data.

Ideally you would be able to construct any fixture providing no
parameters, and be able to individually add any number of named parameters
to constrain different aspects of it.  ::

  def test_filtering_unsupported(self):
    tf = make_tree_format(supports_content_filtering=False) 
    wt = make_working_tree(format=tf)
    self.assertRaises(Exception, wt.set_content_filter(
        make_content_filter()))

(We could have a decorator that insists on kwargs so people don't write
the obscure ``make_branch(True)``.)

Relationships between fixtures
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 * Same class of fixture being used twice: for example we have two branches 
   and want to pull from one to the other.

 * Constraints between fixtures, eg: I have two branches and for some tests 
   require that they have either the same format or different formats or 
   incompatible formats.

 * Child/parent relationships, such as wanting two temporary directories that 
   are siblings, or two branches within the same repository, or having a
   symlink from one thing to something else.

 * Default relationships: there are many things that need a disk directory to
   work in, and you don't want to be manually assembling complicated stacks of
   fixtures from the ground up every time.  So saying "make me a branch"
   should default to the most reasonable type of transport to store it on.  

If you do need to specify relationships is it ok from that point on to
explicitly specify the relationships or can they be implicit in the TestCase?
Do we want ::

   def test_checkout(self):
       t = make_transport(supports_workingtree=True)
       b = make_branch(t)
       b.make_workingtree()

or ::

   def test_checkout(self):
       self.make_transport(supports_workingtree=True)
       b = self.make_branch() # gets transport from self
       b.make_workingtree()

The second is a lot more like we have now: in fact at present it's even more
implicit inside the test setUp.  Will always passing the related fixture when
you don't want the global default be reasonable?

If fixtures default to creating the whole stack they need, then the
following code will make two temporary directories and two repostiories,
which might be unnecessarily expensive::

    def test_two_branches(self):
        b1 = make_branch()
        b2 = make_branch()

alternatively::

    def test_two_branches_same_repo(self):
        r = make_repository()
        b1 = make_branch_with_history(repository=r)
        b2 = make_branch(repository=r)

and perhaps that's nice and explicit, and lets us make less assumptions
about unspecified relationships.

Perhaps we want a way to say "here's a namespaces containing some default
values for you"? ::

    def test_branches_on_disk(self):
        t = make_transport(decorator=VFATSimulator)
        defaults['transport'] = t
        make_branch_with_history(defaults)

but then why not just say ::

        make_branch_with_history(**defaults)

Cleanups
~~~~~~~~

Given that cleanup is rarely needed we *could* rely on gc to tell us when
to free a temporary directory.  This is after all what the Python
TemporaryFile does.  However relying on gc for real-world actions is a bit
of an antipattern: the timing's unpredictable, errors from it are lost, it
can happen a long time after the test finishes.

So if that's not acceptable we do need a way for fixtures that are created
arbitarily far inside a dependency chain to know which TestCase (or other
ObjectWithCleanups) should be destroying them.


Relationships between fixtures and tests
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

How do fixtures relate to tests?

 * If we generate a sequence of *Related Generated Values* then we
   probably want the values in that sequence to be distinct within the
   lifetime of a particular test, and independent of other tests.

 * If the fixture object must be explicitly destroyed, this should 
   happen during the cleanup of the test.  Many (most?) interesting 
   objects don't require explicit cleanup, but many of them do depend 
   on an underlying fixture such as a temporary directory that must be
   cleaned up.

If we call `useFixture` as the standard protocol to set up fixtures,
everything used as a fixture must support tearDown/setUp, but many
interesting objects either don't support that or don't care if we call it
in the context of a test.  You could add a wrapper, or make all the
fixtures into factories for the things that we actually want but that just
seems like overhead.

Even though the Branch might not need any explicit cleanup or teardown, it
may build on a stack of things some of which, such as a temporary
directory, do need explicit cleanup.

Generated values
~~~~~~~~~~~~~~~~

Ideally you want them to be unique but human-friendly.  If they're just
totally arbitrary it may be hard to tell where the value comes from in the
code.  This is foreshadowed by ::

  parent = getUniqueString("parent") 
  child = getUniqueString("child") 
  dict[parent] = [child]   # for example
  self.assertTrue(child in dict[parent]) 
  # perhaps will be dict={"parent-1": "child-2"}

We need to work out where the source of uniqueness (typically a counter)
that generates these will live.  In the current testtools and in the XUnit
Test Patterns book the counter is directly on the TestCase.

For example if you call `make_branch_with_history` twice, what do you get?
Two distinct branches with identical history?  In bzr the common case is
::

    br1 = make_branch_with_history(branch_length=10)
    br2 = make_branch_with_history(fork_from_branch=br1, fork_from_revno=-3,
        branch_length=10)


Fail-closed isolation
~~~~~~~~~~~~~~~~~~~~~

We want to make things "fail closed" if they're not using a fixture, so that
unisolated code will tend to fail rather than to look at the host system of
the tests.  (For example, we want to avoid that anything depends on the
environment variables, the home directory, or the working directory of the
person running the tests.)  There are a few aspects to this:

 * Code in tests that doesn't start out by creating a fixture should "look
   wrong", at least if it's not just testing fairly simple pure in-memory
   objects or functions.  It shouldn't just create things on disk just assuming
   that they're in an isolated directory.

 * A per-suite setup could disable as many things as possible (chdir to 
   an empty readonly directory, or disable IO) and then let it be turned 
   back on only when the specific fixture is requested.  So that implies that
   you need a fixture to say "I need a working directory" or "I need a home
   directory", in the second case both creating it and setting $HOME.

A story
~~~~~~~

1. Really what I just want to write is::

    def test_a_branch(self):
         branch = make_empty_branch()
         self.assertEquals(branch.last_revno(), 0)

Every single line I have to add to this test is a distraction from the core
statement of the test and obscures the purpose of the test.

(But let's say I do actually want *Explicit Setup* so I prefer to say inline
that this test needs a branch: I don't want that just magically present in the
environment or in setUp.  Why?  Because it makes the context more explicit and
avoids constructing expensive per-test resources that I don't actually want.)

2. But I want to make sure that the branch is torn down when it's not needed any
more.  So we could try ::

    def test_a_branch(self):
         branch = make_empty_branch()
         self.addCleanup(branch.delete)
         self.assertEquals(branch.last_revno(), 0)

But any branch we create for the purposes of testing, it's reasonable to
assume that it should be cleaned up when the test concludes.  So we could
say (as we have in bzr pre 2.2)::

    def make_empty_branch(self):
         branch = Branch(....)
         self.addCleanup(branch.delete)
         return branch

    def test_a_branch(self):
         branch = self.make_empty_branch()
         
We tend to add creation methods close to where they're needed and to have them
store some state but they tend to be poorly reusable because they can only
really be called from the class that declared them.

3. We can move these methods to the base TestCase class but that class then
gets bloated with things that aren't really essential to being a test.  Or we
can have multiple base classes specialized in different ways but then too
tests tend to find themselves in the wrong place.  It seems unnecessary to put
the specific definition of the fixture into the TestCase.

If the tests do anything automatic at setUp then the problems of inheritance
get much worse: you entangle "is able to use this fixture" with "must do this
setup."

One reason we want the test fixture connected to the TestCase object is so
that it can be cleaned up or released when the test finishes.  What else?

4. Actually the branch isn't just created in isolation, but rather it's on a
Transport.  The fully explicit form might be

   def make_empty_branch(self):
        t = make_transport()
        b = make_branch(t)

At the moment bzrlib test cases normally have a transport.  While this is very
commonly used it is also a bit of a special case with some of the problems
discussed above: what kind of transport you get is mostly determined by your
test case; some tests use more expensive transports than they really need.

One other consequence is that there is pressure to have only one transport per
test (or one cluster of related transports) but for some cases it would be
better, faster, and perhaps give better coverage to do different parts on different
objects.  Things that need a working tree can be on disk and others can be in
memory.

So one other reason fixtures tend to be created through the TestCase is that
the TestCase holds some implicit resources for them, such as the transport.
This can be seen as the TestCase holding a set of fixtures that are used by
other fixtures.

5. Sometimes we care about some properties of that transport: some
tests must run on a local disk (because they'll want to make a WorkingTree)
and other times not.




.. vim: ft=rest et sw=4