test isolation: default ignores & repository acquisition

[Robert Collins]

On Wed, 2009-09-16 at 13:22 +1000, Martin Pool wrote:
> 2009/9/16 Robert Collins <robertc at robertcollins.net>:
> > We've got two significant things with test isolation that cause some
> > overhead/work in the test suite.
> >
> > Firstly is the defaut ignores. These are written to in ~/.bazaar/ignore,
> > if the file doesn't exist. Because of this, every test that invokes
> > directly or indirectly 'bzrlib.ignore.get_user_ignores' must have an
> > overridden, created homedir. While we could make one homedir and check
> > it hasn't been written to, I think I'd rather make a hook for this,
> > which can control obtaining these ignores. Then the test suite
> > infrastructure can by default install a hook to satisfy these from the
> > constants rather than from disk. Tests that test the management of
> > default ignores would reset that hook and use an overridden homedir.
> > Any thoughts on this?
> 
> It seems like this is not so much something you want to hook, as
> something that should be outside of the scope of what's being tested.
> So I wonder if we should handle that by hoisting it to say
> initialize_per_user_data, run from run_bzr but not lower levels.

We could look at doing that. At the moment its called via working tree
methods, and config methods.

> > Secondly, the repository acquisition logic, which reads up containing
> > directories to find shared repositories and/or branching policy triggers
> > reads outside the test suite, if we don't have a manually created
> > repository there. I'd like to get to the point of only having a
> > containing repository when we actually want to test the full stack. I
> > can't think of an easy catch-all for this particular issue, but perhaps
> > I've missed something? Anyhow, I think the approach I'm going to drive
> > for is to only create the containing catchall repository on
> > TestCaseWithTransport.
> 
> Well, another way would be to install some kind of persistent process
> wide chroot-like feature while running the tests that forbids all
> access outside of the test temp dir.

It was in writing such a chroot-like feature that I discovered how
pervasive this lookup is :). We'd need to look like a chroot to all the
low level code: file()/mkdir/etc, for this approach to work: A mismatch
between LocalTransport and other code would cause lots of failures.

I think only a small number of tests actually want/need to do 'bzr
branch <target>' where target will have a working tree. Most will just
want 'a branch with some history', and as such can have the details
abstracted away.

> > I propose the following test classes:
> ...
> >  - TestCase
> 
> I (still) think that changing the test base class is a poor way to
> describe this, and that we have tended to get it wrong in the past,
> either by people inheriting from something too broad and expensive or
> something too narrow and insufficiently isolated.  Also, the different
> things you might want here (a home dir vs a memory transport) are
> somewhat orthogonal and that maps poorly onto inheritance.

I agree that the base class isn't a great way to describe the need for
different features. However, for safety and isolation I want our topmost
base class to prevent/inhibit/cause-errors actions outside the test
environment. We could add a dependency on testresources, if you like,
which has all the machinery to bring up and tear down infrastructure
that tests need. It would be a very small amount of work to write
always-dirty resources that provide HomeDir, Safety nets and so on.

Compare that with my proposal though, which adds a single class to the
classes we already have: its probably much less overhead to rearrange
what we have a little and focus on changing the style later.

> What I would prefer is just methods on the bzr base TestCase that say
> "I need a home dir" or "give me a transport" and then they construct
> the thing as it's needed.  They can of course do that from setUp of a
> particular set of tests. Since we have addCleanup I don't see any
> reason people need it done per class.

There is no strong implication that several base TestCase classes would
be anything other than custom setUp methods pre-canned for users. We
already know that many tests only need X, or Y, and providing that X or
Y are currently done via inheritance.

That said, I think that a single big TestCase is pretty ugly, and its an
unfortunate consequence of UnitTest's current design. Having multiple
'core' classes lets us avoid having a truely mega TestCase.

There are a couple of driving factors towards having all the methods on
the topmost class:
 - assertions & object state
 - helper functions & object state

For the former, something like hamcrest looks really nice to me. For the
latter, well thats what I wrote testresources for :).

Using one TestCase base class and then putting different permutations of
setup stuff into setUp of child classes isn't really any different to
having several different TestCase base classes which children can choose
from: in fact, the very first time that two different children in the
former style have the same setUp, you'll end up morphing into the latter
style (many base classes), unless you deliberately preserve the
duplication.

-Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090916/2930d64d/attachment-0002.pgp