[merge] win32- more test suite fixes

Aaron Bentley aaron.bentley at utoronto.ca
Fri Jun 30 17:13:19 BST 2006


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

John Arbash Meinel wrote:
> In other parts of the code, we were expecting to get ENOENT if we tried
> to rename the current working directory.

It looks like you actually mean 'we were expecting to get ENOENT if we
tried to rename *a non-existent file* to the current working directory'.

> fancy_rename() gets EPERM,
> because it tries to rename the target out of the way, before trying to
> rename the from into position.
> So I change _win32_rename() so that it stats the from first, forcing ENOENT.

Adding an unconditional extra syscall seems like a bad idea for
something so heavily used.  Can't we avoid doing this unless we get an
EPERM?

> the test test_fileids_affected_by was expecting os.chmod() to work.
> So I switched it to use TreeTransform.set_executability() which does
> work on all platforms.

Sounds good.

> Also, the same test thought it was handling when certain characters were
> invalid on the filesystem, but it really wasn't. So I fixed that.
> (It wasn't setting self.branch but never actually checked that it wasn't
> properly set up)


> TreeTransform wasn't properly closing a file descriptor, so I put it in
>  a try/finally.

=== modified file 'bzrlib/merge.py'
- --- bzrlib/merge.py	2006-06-20 03:57:11 +0000
+++ bzrlib/merge.py	2006-06-29 20:44:40 +0000
@@ -394,10 +394,7 @@
             except UnsupportedOperation:
                 pass
         finally:
- -            try:
- -                self.tt.finalize()
- -            except:
- -                pass
+            self.tt.finalize()
             working_tree.unlock()
             self.pb.clear()

What bugs me is that if we enter the finally block due to an exception,
and then self.tt.finalize throws, we'll never see the original
exception.  I'm fine with doing it in this case, but we should find a
more general solution.

I don't understand what the following changes are all about:

@@ -67,6 +71,7 @@
         # J changes: 'b-file-id-2006-01-01-defg'
         # K changes: 'c-funky<file-id> quiji%bo'

+        self.branch = None
         main_wt = self.make_branch_and_tree('main')
         main_branch = main_wt.branch
         self.build_tree(["main/a","main/b","main/c"])
@@ -100,7 +105,7 @@
         d2 = main_branch.bzrdir.clone('branch2')
         branch2_branch = d2.open_branch()
         bt2 = d2.open_workingtree()
- -        os.chmod("branch2/b",0770)
+        set_executability(bt2, 'b', True)
         bt2.commit("branch2, Commit one", rev_id="rev-J")

         #-------- end J -----------
@@ -134,22 +139,27 @@
         self.branch = main_branch

     def test_fileids_altered_between_two_revs(self):
+        if self.branch is None:
+            # Could not create the branching structure
+            # for this repository format
+            return
+
         def foo(old, new):
             print
set(self.branch.repository.get_ancestry(new)).difference(set(self.branch.repository.get_ancestry(old)))

- -        self.assertEqual(
+        self.assertDictsEqual(
             {'b-file-id-2006-01-01-defg':set(['rev-J']),
              'c-funky<file-id> quiji%bo':set(['rev-K'])
              },

self.branch.repository.fileids_altered_by_revision_ids(["rev-J","rev-K"]))

- -        self.assertEqual(
+        self.assertDictsEqual(
             {'b-file-id-2006-01-01-defg': set(['rev-<D>']),
              'file-d': set(['rev-F']),
              },

self.branch.repository.fileids_altered_by_revision_ids(['rev-<D>',
'rev-F']))

- -        self.assertEqual(
+        self.assertDictsEqual(
             {
              'b-file-id-2006-01-01-defg': set(['rev-<D>', 'rev-G',
'rev-J']),
              'c-funky<file-id> quiji%bo': set(['rev-K']),
@@ -158,7 +168,7 @@
             self.branch.repository.fileids_altered_by_revision_ids(
                 ['rev-<D>', 'rev-G', 'rev-F', 'rev-K', 'rev-J']))

- -        self.assertEqual(
+        self.assertDictsEqual(
             {'a-file-id-2006-01-01-abcd': set(['rev-B']),
              'b-file-id-2006-01-01-defg': set(['rev-<D>', 'rev-G',
'rev-J']),
              'c-funky<file-id> quiji%bo': set(['rev-K']),
@@ -168,22 +178,30 @@
                 ['rev-G', 'rev-F', 'rev-C', 'rev-B', 'rev-<D>',
'rev-K', 'rev-J']))

     def test_fileids_altered_by_revision_ids(self):
- -        self.assertEqual(
+        if self.branch is None:
+            # See earlier comment about not being able
+            # to run this test with older formats
+            return
+        self.assertDictsEqual(
             {'a-file-id-2006-01-01-abcd':set(['rev-A']),
              'b-file-id-2006-01-01-defg': set(['rev-A']),
              'c-funky<file-id> quiji%bo': set(['rev-A']),
              },

self.branch.repository.fileids_altered_by_revision_ids(["rev-A"]))
- -        self.assertEqual(
+        self.assertDictsEqual(
             {'a-file-id-2006-01-01-abcd':set(['rev-B'])
              },

self.branch.repository.fileids_altered_by_revision_ids(["rev-B"]))
- -        self.assertEqual(
+        self.assertDictsEqual(
             {'b-file-id-2006-01-01-defg':set(['rev-<D>'])
              },

self.branch.repository.fileids_altered_by_revision_ids(["rev-<D>"]))

     def test_fileids_involved_full_compare(self):
+        if self.branch is None:
+            # See earlier comment about not being able
+            # to run this test with older formats
+            return
         # this tests that the result of each fileid_involved calculation
         # along a revision history selects only the fileids selected by
         # comparing the trees - no less, and no more. This is correct
@@ -213,6 +231,7 @@
     def setUp(self):
         super(TestFileIdInvolvedSuperset, self).setUp()

+        self.branch = None
         main_wt = self.make_branch_and_tree('main')
         main_branch = main_wt.branch
         self.build_tree(["main/a","main/b","main/c"])
@@ -228,17 +247,21 @@
         branch2_bzrdir = main_branch.bzrdir.sprout("branch2")
         branch2_branch = branch2_bzrdir.open_branch()
         branch2_wt = branch2_bzrdir.open_workingtree()
- -        os.chmod("branch2/b",0770)
+        set_executability(branch2_wt, 'b', True)
         branch2_wt.commit("branch2, Commit one", rev_id="rev-J")

         self.merge(branch2_branch, main_wt)
- -        os.chmod("main/b",0660)
+        set_executability(main_wt, 'b', True)
         main_wt.commit("merge branch1, rev-22",  rev_id="rev-G")

         # end G
         self.branch = main_branch

     def test_fileid_involved_full_compare2(self):
+        if self.branch is None:
+            # See earlier comment about not being able
+            # to run this test with older formats
+            return

Shouldn't this be a TestSkipped?

=== modified file 'bzrlib/transform.py'
- --- bzrlib/transform.py	2006-06-26 19:06:55 +0000
+++ bzrlib/transform.py	2006-06-29 20:46:29 +0000
@@ -259,11 +259,13 @@
         New file takes the permissions of any existing file with that id,
         unless mode_id is specified.
         """
- -        f = file(self._limbo_name(trans_id), 'wb')
- -        unique_add(self._new_contents, trans_id, 'file')
- -        for segment in contents:
- -            f.write(segment)
- -        f.close()
+        f = open(self._limbo_name(trans_id), 'wb')
+        try:
+            unique_add(self._new_contents, trans_id, 'file')
+            for segment in contents:
+                f.write(segment)
+        finally:
+            f.close()

If unique_add fails, then we'll exit this scope with no entry in
self._new_contents, and so we won't get cleaned up properly in finalize.
How about this:

    f = open(self._limbo_name(trans_id), 'wb')
    try:
        try:
            unique_add(self._new_contents, trans_id, 'file')
            for segment in contents:
                f.write(segment)
        finally:
            f.close()
    except:
        os.unlink(self._limbo_name(trans_id))
        raise

        f.close()

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFEpU2f0F+nu1YWqI0RAjE2AJ9PxpoNGkwrNEMR/CLpK5O2BBOPbwCfco5/
HVIb1mpxkT+uXINHn8kC6B4=
=Fdk3
-----END PGP SIGNATURE-----




More information about the bazaar mailing list