[MERGE] fixes for selftest on win32 (part 2)

John Arbash Meinel john at arbash-meinel.com
Mon Mar 12 22:38:24 GMT 2007


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

Alexander Belchenko wrote:
> Next portion of fixes for selftest on win32:
> 
>  2332 Alexander Belchenko       2007-03-11
>       test_sftp_transport: fix 1 test, skip 1 test; now all tests pass on win32
> 
>  2331 Alexander Belchenko       2007-03-11
>       LockError produce unprintable exception on Python 2.5 because it try to override
> StandardError.message attribute
> 
>  2330 Alexander Belchenko       2007-03-11
>       TestCase.assertFileEqual: separate open/read file operations
>       to achieve more precise locating the problems on win32
> 
>  2329 Alexander Belchenko       2007-03-08
>       test intertree_implementations: skip tests with symlinks on platforms that don't have symlinks
> support
> 
>  2328 Alexander Belchenko       2007-03-08
>       test_dirstate: skip tests with symlinks on platforms that don't have symlinks support
> 
>  2327 Alexander Belchenko       2007-03-08
>       numbered dirs: printErrorList show test number for NUMBERED_DIRS
> 
> [µ]
> 

- ------------------------------------------------------------------------

# Bazaar revision bundle v0.9
#
# message:
#   merge bzr.dev
# committer: Alexander Belchenko <bialix at ukr.net>
# date: Mon 2007-03-12 00:07:34.062000036 +0200

=== modified file bzrlib/errors.py
- --- bzrlib/errors.py
+++ bzrlib/errors.py
@@ -647,7 +647,7 @@

 class LockError(BzrError):

- -    _fmt = "Lock error: %(message)s"
+    _fmt = "Lock error: %(msg)s"

     internal_error = True

@@ -657,7 +657,7 @@
     #
     # New code should prefer to raise specific subclasses
     def __init__(self, message):
- -        self.message = message
+        self.msg = message


^- Was this specifically needed? You seem to just be changing the
variable, without actually changing the effect. But then again, children
might be doing something different.



...

v- This should be in a try/finally
f = file(path, 'r')
try:
  s = f.read()
finally:
  f.close()
self.assertEqualDiff(content, s)

@@ -1692,7 +1695,10 @@
         """Fail if path does not contain 'content'."""
         self.failUnlessExists(path)
         # TODO: jam 20060427 Shouldn't this be 'rb'?
- -        self.assertEqualDiff(content, open(path, 'r').read())
+        f = file(path, 'r')
+        s = f.read()
+        f.close()
+        self.assertEqualDiff(content, s)

     def failUnlessExists(self, path):
         """Fail unless path, which may be abs or relative, exists."""



v- Rather than directly importing 'TestSkipped' you can use
'tests.TestSkipped', since 'tests' is already imported.

 from bzrlib import errors, tests, workingtree_4
- -from bzrlib.osutils import file_kind
+from bzrlib.osutils import file_kind, has_symlinks
+from bzrlib.tests import TestSkipped
 from bzrlib.tests.intertree_implementations import TestCaseWithTwoTrees

 # TODO: test the include_root option.
@@ -955,6 +956,8 @@
         return self.mutable_trees_to_test_trees(tree1, tree2)

# I'm okay with skipping them for now. I think the idea was that we
would hopefully find a way to keep symlinks in the inventory (similar to
how we currently handle the executable bit).

     def test_versioned_symlinks(self):
+        if not has_symlinks():
+            raise TestSkipped("No symlink support")
         tree1, tree2 = self.make_trees_with_symlinks()
         root_id = tree1.path2id('')
         tree1.lock_read()
@@ -979,6 +982,8 @@
                 want_unversioned=True))

     def test_versioned_symlinks_specific_files(self):
+        if not has_symlinks():
+            raise TestSkipped("No symlink support")
         tree1, tree2 = self.make_trees_with_symlinks()
         root_id = tree1.path2id('')
         tree1.lock_read()




...

v- # bzr doesn't support fake symlinks on windows, yet.

- -        # be represented on windows.
+        # bzr yet don't have support for fake symlinks on windows
+        if not has_symlinks():
+            raise TestSkipped("No symlink support")
         os.symlink('target', 'a link')
         stat = os.lstat('a link')
         expected_entries = [
@@ -1201,7 +1207,8 @@
     def test_update_entry_symlink(self):
         """Update entry should read symlinks."""
         if not osutils.has_symlinks():
- -            return # PlatformDeficiency / TestSkipped
+            # PlatformDeficiency / TestSkipped
+            raise TestSkipped("No symlink support")
         state, entry = self.get_state_with_a()
         state.save()
         self.assertEqual(dirstate.DirState.IN_MEMORY_UNMODIFIED,
@@ -1286,6 +1293,7 @@
         This should not be called if this platform does not have symlink
         support.
         """
+        # caller should care about skipping test on platforms without
symlinks
         os.symlink('path/to/foo', 'a')

         stat_value = os.lstat('a')
@@ -1320,7 +1328,8 @@

     def test_update_missing_symlink(self):
         if not osutils.has_symlinks():
- -            return # PlatformDeficiency / TestSkipped
+            # PlatformDeficiency / TestSkipped
+            raise TestSkipped("No symlink support")
         state, entry = self.get_state_with_a()
         packed_stat = self.create_and_test_symlink(state, entry)
         os.remove('a')
@@ -1341,7 +1350,8 @@
     def test_update_file_to_symlink(self):
         """File becomes a symlink"""
         if not osutils.has_symlinks():
- -            return # PlatformDeficiency / TestSkipped
+            # PlatformDeficiency / TestSkipped
+            raise TestSkipped("No symlink support")
         state, entry = self.get_state_with_a()
         self.create_and_test_file(state, entry)
         os.remove('a')
@@ -1357,7 +1367,8 @@
     def test_update_dir_to_symlink(self):
         """Directory becomes a symlink"""
         if not osutils.has_symlinks():
- -            return # PlatformDeficiency / TestSkipped
+            # PlatformDeficiency / TestSkipped
+            raise TestSkipped("No symlink support")
         state, entry = self.get_state_with_a()
         self.create_and_test_dir(state, entry)
         os.rmdir('a')
@@ -1365,6 +1376,8 @@

     def test_update_symlink_to_file(self):
         """Symlink becomes a file"""
+        if not has_symlinks():
+            raise TestSkipped("No symlink support")
         state, entry = self.get_state_with_a()
         self.create_and_test_symlink(state, entry)
         os.remove('a')
@@ -1372,6 +1385,8 @@

     def test_update_symlink_to_dir(self):
         """Symlink becomes a directory"""
+        if not has_symlinks():
+            raise TestSkipped("No symlink support")
         state, entry = self.get_state_with_a()
         self.create_and_test_symlink(state, entry)
         os.remove('a')



@@ -105,6 +106,8 @@
     """Test the SFTP transport with homedir based relative paths."""

     def test__remote_path(self):

v- I don't think this test should be skipped. We can use:

if sys.platform == 'win32':
    # On win32 an initial '/' is inserted before the drive letter.
    test_dir = '/' + self.test_dir
else:
    test_dir = self.test_dir
...

+        if sys.platform == 'win32':
+            raise TestSkipped('This test require unix-like absolute path')
         t = self.get_transport()
         # try what is currently used:
         # remote path = self._abspath(relpath)

=== modified file bzrlib/transport/sftp.py
- --- bzrlib/transport/sftp.py
+++ bzrlib/transport/sftp.py
@@ -1106,7 +1106,10 @@

     def get_url(self):
         """See bzrlib.transport.Server.get_url."""
- -        return self._get_sftp_url(urlutils.escape(self._homedir[1:]))
+        if sys.platform == 'win32':
+            return self._get_sftp_url(urlutils.escape(self._homedir))
+        else:
+            return self._get_sftp_url(urlutils.escape(self._homedir[1:]))

^- I think this one would be clearer as

homedir = self._homedir
if sys.platform != 'win32':
    # Remove the initial '/' on all platforms but win32
    homedir = homedir[1:]
return self._get_sftp_url(urlutils.escape(homedir))


Otherwise +1 from me.

I noticed your last commit message is "now all tests pass on win32". So
figured I would do a full test run and verify that. (Of course, this
takes a while, so I won't know just yet).

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFF9dZgJdeBCYSNAAMRAhmyAJ9L77FrXcBVREDEQaZ/UGQKtDSrKACgiZE/
qsR2ovmtmUQtZDoC9Asjku0=
=cSlW
-----END PGP SIGNATURE-----



More information about the bazaar mailing list