[MERGE] test framework distinguishes skips

[John Arbash Meinel]

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

Aaron Bentley wrote:
> Here is the revised version.  Changes are:
> 1. Unicode error handling is as it was before, except that
> NonUnicodeFilesystem is raised instead of a generic TestSkipped.
> 2. dependency_failure(LSProfNotPresent) has morphed into
> depends_on('lsprof')
> 3. platform requirments are handled with normal function calls rather
> than decorators.
> 
> Aaron


...


- -class NoDiff(BzrNewError):
+class NoDiff(DependencyNotPresent):
     """Diff is not installed on this machine: %(msg)s"""

     def __init__(self, msg):
- -        super(NoDiff, self).__init__(msg=msg)
- -
- -
- -class NoDiff3(BzrNewError):
+        super(NoDiff, self).__init__('diff', Exception(msg))
+
+
+class NoDiff3(DependencyNotPresent):
     """Diff3 is not installed on this machine."""

+    def __init__(self):
+        DependencyNotPresent.__init__(self, 'diff3', None)
+


It turns out that super() is not safe for exceptions. (It is safe for
TestCase).

Can you fix this for NoDiff?


+class ExpectedFailure(TestSkipped):
+    """Indicates an expected failure ocurred."""
+    def __init__(self, e):
+        TestSkipped.__init__(self)
+        self.e = e
+
+
+class MissingDependency(TestSkipped):
+    """Indicates a test was skipped because of a missing dependency"""
+    def __init__(self, e):
+        TestSkipped.__init__(self)
+        self.e = e
+
+
+class PlatformDefect(TestSkipped):
+    """Indicates a test was skipped because of a platform defect"""
+    def __init__(self, msg):
+        TestSkipped.__init__(self, msg)

You include a copy of the error as part of the exception, but you aren't
overriding the __str__ so that it would get prented.


+class NonUnicodeFilesystem(PlatformDefect):
+    """Indicates a test was skipped because the filesystem is
non-unicode"""
+    def __init__(self):
+        PlatformDefect.__init__(self, "Non-unicode filesystem")

It may be better to say:
'Unable to create unicode filename on filesystem %s' %
(sys.getfilesystemencoding(),)

The problem is that under certain encodings you may be able to represent
some files, but not all.
In the original tests, I included the filename that failed, but that may
not be worth the effort.


 class CommandFailed(Exception):
@@ -542,6 +567,16 @@
             self.fail("%r is an instance of %s rather than %s" % (
                 obj, obj.__class__, kls))

+    def require_symlinks(self):
+        """Call to skip a test on platforms that don't support symlinks."""
+        if not osutils.has_symlinks():
+            raise PlatformDefect("Symlinks are not supported")
+
+    def require_file_modes(self):
+        """Skip a test if the platform doesn't support file modes"""
+        if sys.platform == 'win32':
+            raise PlatformDefect('Unix file modes not supported')
+
     def _startLogFile(self):
         """Send bzr and test log messages to a temporary file.


Transport objects do have a _can_roundtrip_unix_modebits() function,
which is used by some of the test suite. I'm not sure if that is
better/worse than this. (I think it is better)
Tests that want to test real files directly could go through
TreeTransform. We only need one or two tests that actually call chmod().
To test that TreeTransform is doing the right thing, etc.


...

+    @depends_on('diff')
     def test_external_diff(self):
         lines = external_udiff_lines(['boo\n'], ['goo\n'])
         self.check_patch(lines)

+    @depends_on('diff')
     def test_external_diff_no_fileno(self):
         # Make sure that we can handle not having a fileno, even
         # if the diff is large

This seems a little funny that Exception.library == diff. Maybe .library
should be renamed to 'dependency'.

...

I think the rest looks pretty good. It is a big enough change that I
think it would be reasonable to wait for Martin to look at it. But you
have my +1.

John
=:->


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

iD8DBQFErq0NJdeBCYSNAAMRAj65AKCRq/Vm7kx2stnGywDEMsVBQC19IQCbBzyl
bw0XiXXxk7QHgh4WpOZLAqw=
=WwWD
-----END PGP SIGNATURE-----