Merge lp:~vila/bzr/646133-selftest-as-root into lp:bzr/2.1

[Vincent Ladeuil]

Skip chmod bits dependent tests when running as root

This fix the remaining failures in the test suite for 2.1 when running as root (bug #646133).
Some tests are creating files with 000 mode bits or 400 to exercise the code that tries to read or write them. 'root' doesn't care, so there is no point trying to run them.

I also fixed an old bug in test_selftest which has already been fixed in later releases. This is not strictly related but... doesn't deserve a dedicated merge proposal either.

[John A Meinel (jameinel)]

Just to clarify, we need to run as root because the buildd's run everything as root. (seems silly/foolish).

Anyway, the change looks good to me.

review: Approve

[Vincent Ladeuil (vila)]

sent to pqm by email

[Martin Pool (mbp)]

That looks ok. I wonder what happens on Windows? It probably says
False even if you are an administrator, though these particular chmod
tests probably won't run there anyhow?

Could you mention this in testing.txt too please?

[Vincent Ladeuil (vila)]

windows raise an AttributeError when calling getuid() so it *will* return False.
Good point about testing.txt, I'll do that before landing.

[Vincent Ladeuil (vila)]

meh, already merged... Well, I'll do that when merging in bzr.dev then.

[John A Meinel (jameinel)]

Um, check your code again. It will return *True* on Windows.

+ def _probe(self):
+ try:
+ uid = os.getuid()
+ except AttributeError:
+ # If there is no uid, chances are there is no root either
+ return True
+ return uid != 0

But then again, notice that you are using a negative "NotRunningAsRoot".

[Vincent Ladeuil (vila)]

Meh, lol, yeah I didn't re-read the code and replied to Martin, I think we all agree the behaviour is the expected one on windows: not_running_as_root is always True. Now whether it's better to be wrong when you're right or right when you're wrong is left as an exercise for the philosopher ;)

Some of the tests I modified *also* check for the supports_posix_readonly feature which see *OR* are already skipped on windows.

=== modified file 'NEWS'
--- NEWS	2010-09-17 19:58:09 +0000
+++ NEWS	2010-09-23 16:41:25 +0000
@@ -19,6 +19,9 @@
 Bug Fixes
 *********
 
+* Skip the tests that requires respecting the chmod bits when running as root.
+  (Vincent Ladeuil, #646133)
+
 Improvements
 ************
 
 
=== modified file 'bzrlib/tests/features.py'
--- bzrlib/tests/features.py	2010-02-17 17:11:16 +0000
+++ bzrlib/tests/features.py	2010-09-23 16:41:25 +0000
@@ -14,11 +14,27 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
+import os
 
 from bzrlib import tests
 from bzrlib.symbol_versioning import deprecated_in
 
 
+class _NotRunningAsRoot(tests.Feature):
+
+    def _probe(self):
+        try:
+            uid = os.getuid()
+        except AttributeError:
+            # If there is no uid, chances are there is no root either
+            return True
+        return uid != 0
+
+    def feature_name(self):
+        return 'Not running as root'
+
+
+not_running_as_root = _NotRunningAsRoot()
 apport = tests.ModuleAvailableFeature('apport')
 ApportFeature = tests._CompatabilityThunkFeature('bzrlib.tests.features',
     'ApportFeature', 'bzrlib.tests.features.apport', deprecated_in((2,1,0)))
 
=== modified file 'bzrlib/tests/per_lock/test_lock.py'
--- bzrlib/tests/per_lock/test_lock.py	2009-07-07 09:00:59 +0000
+++ bzrlib/tests/per_lock/test_lock.py	2010-09-23 16:41:25 +0000
@@ -21,7 +21,7 @@
     osutils,
     )
 
-from bzrlib.tests import UnicodeFilenameFeature
+from bzrlib.tests import (features, UnicodeFilenameFeature)
 from bzrlib.tests.per_lock import TestCaseWithLock
 
 
@@ -63,6 +63,7 @@
 
         But we shouldn't be able to take a write lock.
         """
+        self.requireFeature(features.not_running_as_root)
         osutils.make_readonly('a-file')
         # Make sure the file is read-only (on all platforms)
         self.assertRaises(IOError, open, 'a-file', 'rb+')
 
=== modified file 'bzrlib/tests/test_lockdir.py'
--- bzrlib/tests/test_lockdir.py	2010-09-10 05:13:57 +0000
+++ bzrlib/tests/test_lockdir.py	2010-09-23 16:41:25 +0000
@@ -39,7 +39,7 @@
     LockNotHeld,
     )
 from bzrlib.lockdir import LockDir
-from bzrlib.tests import TestCaseWithTransport
+from bzrlib.tests import (features, TestCaseWithTransport)
 from bzrlib.trace import note
 
 # These tests sometimes use threads to test the behaviour of lock files with
@@ -669,6 +669,7 @@
         ld1.unlock()
 
     def test_lock_permission(self):
+        self.requireFeature(features.not_running_as_root)
         if not osutils.supports_posix_readonly():
             raise tests.TestSkipped('Cannot induce a permission failure')
         ld1 = self.get_lock()
 
=== modified file 'bzrlib/tests/test_osutils.py'
--- bzrlib/tests/test_osutils.py	2010-05-27 04:00:01 +0000
+++ bzrlib/tests/test_osutils.py	2010-09-23 16:41:25 +0000
@@ -33,6 +33,7 @@
     win32utils,
     )
 from bzrlib.tests import (
+    features,
     file_utils,
     test__walkdirs_win32,
     )
@@ -1067,6 +1068,7 @@
         if sys.platform == 'win32':
             raise tests.TestNotApplicable(
                 "readdir IOError not tested on win32")
+        self.requireFeature(features.not_running_as_root)
         os.mkdir("test-unreadable")
         os.chmod("test-unreadable", 0000)
         # must chmod it back so that it can be removed
 
=== modified file 'bzrlib/tests/test_selftest.py'
--- bzrlib/tests/test_selftest.py	2010-02-17 17:11:16 +0000
+++ bzrlib/tests/test_selftest.py	2010-09-23 16:41:25 +0000
@@ -609,12 +609,13 @@
                 l.attempt_lock()
         test = TestDanglingLock('test_function')
         result = test.run()
+        total_failures = result.errors + result.failures
         if self._lock_check_thorough:
-            self.assertEqual(1, len(result.errors))
+            self.assertEqual(1, len(total_failures))
         else:
             # When _lock_check_thorough is disabled, then we don't trigger a
             # failure
-            self.assertEqual(0, len(result.errors))
+            self.assertEqual(0, len(total_failures))
 
 
 class TestTestCaseWithTransport(tests.TestCaseWithTransport):