[MERGE] Add PreviewTree to tree_implementation tests

Andrew Bennetts andrew at canonical.com
Mon Apr 14 01:23:33 BST 2008


Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi all,
> 
> Here's a patch that gets PreviewTree passing all tree_implementation
> tests.  However, it is only testing the case where no files have changed
> from the underlying tree.  The next step will be testing the case where
> all files have changed from the underlying tree.
> 
> PreviewTree does not have an inventory attribute, and hope to avoid
> adding one.  Our API is currently at a stage where reasonable
> alternatives are available for just about every need.  So I have
> disabled the test_inv tests on PreviewTree.
> 
> I have copied the test_inv tests to test_inv_alternatives, and tweaked
> them so that they do not require an inventory attribute (though many
> still require an InventoryEntry).  This at least ensures that
> PreviewTree has equivalent functionality to inventory-based trees.

I don't much like the duplication of test code here.  It's a big maintenance
problem.  At a minimum this sort of thing needs big, unmissable comments in the
docstrings of the two modules saying there's another module that MUST be kept in
sync.  (Even better might be that plus a test that the two modules actually have
an identical set of tests.)

More fundamentally, I don't understand from your description why the current
test_inv can't simply be replaced by your test_inv_alternatives — if the
interface doesn't require an inventory attribute to exist on tree
implementations, and we can do all the tests we want to do without one, why do
we need seperate tests that use an inventory attribute?

-Andrew.




More information about the bazaar mailing list