[MERGE] Update to bzr.dev.

Andrew Bennetts andrew at canonical.com
Tue Jun 17 09:45:04 BST 2008


I've started reviewing the test changes now.

Robert Collins wrote:
> === modified file 'bzrlib/tests/__init__.py'

> === modified file 'bzrlib/tests/blackbox/test_info.py'

> === modified file 'bzrlib/tests/branch_implementations/test_branch.py'

> === modified file 'bzrlib/tests/bzrdir_implementations/test_bzrdir.py'

> === modified file 'bzrlib/tests/interrepository_implementations/test_fetch.py'

> === modified file 'bzrlib/tests/interrepository_implementations/test_interrepository.py'

> === modified file 'bzrlib/tests/repository_implementations/__init__.py'

> === modified file 'bzrlib/tests/repository_implementations/helpers.py'

> === modified file 'bzrlib/tests/repository_implementations/test_check.py'

These are all good.

> === modified file 'bzrlib/tests/repository_implementations/test_check_reconcile.py'

> +            # (we specify it this way because a store can use arbitrary
> +            # compression pointers in principle.

This comment is missing a close parenthesis.

> === modified file 'bzrlib/tests/repository_implementations/test_commit_builder.py'
[...]
>          tree1, tree2 = self._get_revtrees(tree, [rev1, rev2])
>          self.assertEqual(rev1, tree1.inventory[name + 'id'].revision)
>          self.assertEqual(rev1, tree2.inventory[name + 'id'].revision)
> -        self.assertFileAncestry([rev1], tree, name)
> +        file_id = name + 'id'
> +        expected_graph = {}
> +        expected_graph[(file_id, rev1)] = ()
> +        self.assertFileGraph(expected_graph, tree, (file_id, rev1))

Why couldn't assertFileAncestry have been preserved, at least for the cases that
didn't use the optional alt_ancestry arg (which are the majority)?  The
implementation is simple:

    def assertFileAncestry(self, ancestry, tree, name):
        file_id = name + 'id'
        expected_graph = {}
        parent = ()
        for rev_id in ancestry:
            key = (file_id, rev_id)
            expected_graph[key] = parent
            parent = key
        self.assertFileGraph(expected_graph, tree, parent)

assertFileGraph's API doesn't seem to be an improvement to assertFileAncestry's
to me.  It makes the tests harder to read.

>      def test_last_modified_revision_after_content_file_changes(self):
>          # altering a file changes the last modified.
> @@ -457,10 +462,13 @@
>          rev4 = self.mini_commit(tree1, 'new_' + name, 'new_' + name)
>          tree3, = self._get_revtrees(tree1, [rev4])
>          self.assertEqual(rev4, tree3.inventory[name + 'id'].revision)
> -        # TODO: change this to an assertFileGraph call to check the
> -        # parent order of rev4: it should be rev2, rev3
> -        self.assertFileAncestry([rev1, rev2, rev3, rev4], tree1, name,
> -            [rev1, rev3, rev2, rev4])
> +        file_id = name + 'id'
> +        expected_graph = {}
> +        expected_graph[(file_id, rev1)] = ()
> +        expected_graph[(file_id, rev2)] = ((file_id, rev1),)
> +        expected_graph[(file_id, rev3)] = ((file_id, rev1),)
> +        expected_graph[(file_id, rev4)] = ((file_id, rev2), (file_id, rev3),)
> +        self.assertFileGraph(expected_graph, tree1, (file_id, rev4))

This case is reasonable to use assertFileGraph for I think.

One other comment: check your diff for "import pdb; pdb.set_trace()" :)

-Andrew.




More information about the bazaar mailing list