test isolation: default ignores & repository acquisition

[Martin Pool]

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.

> 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.

>
> I propose the following test classes:
> TestCase:
>  - isolates environment variables
>  - has facilities for transports and servers, defaulting to
>   all memory and no local disk
>  - hooks and isolates the default ignores
> TestCaseWithTempDir(TestCase):
>  - adds a temporary directory on disk
> TestCaseWithHomeDir(TestCaseWithTempDir):
>  - adds a fake HOME
> TestCaseWithMemoryTransport(TestCaseWithHomeDir):
>  - adds a 'safety net repository'
> TestCaseWithTransport(TestCaseWithMemoryTransport):
>  - sets the vfs transport to local disk
>
> Then tests can choose what they need to use:
>  - use OS locks in the repository
>  - TestCaseWithTransport
>  - clone to a working tree
>  - TestCaseWithMemoryTransport
>  - write to home/config settings
>  - TestCaseWithHomeDir
>  - use actual files [$TEMPDIR doesn't count]
>  - TestCaseWithTempDir
>  - anything else
>  - 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.

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.

-- 
Martin <http://launchpad.net/~mbp/>