testtools-dev team mailing list archive

[Martin Pool <martinpool@xxxxxxxxx>]

Martin Pool has proposed merging lp:~mbp/testtools/try-import-error into lp:testtools.

Requested reviews:
  testtools developers (testtools-dev)

For more details, see:
https://code.launchpad.net/~mbp/testtools/try-import-error/+merge/63074

jml pointed out try_import and try_imports. I would like to use this in Bazaar to clean up some similar existing code.  We want to log the fact that a module isn't available because it might indicate an installation problem.  This adds a callback to observe the error.
-- 
https://code.launchpad.net/~mbp/testtools/try-import-error/+merge/63074
Your team testtools developers is requested to review the proposed merge of lp:~mbp/testtools/try-import-error into lp:testtools.

=== modified file 'testtools/helpers.py'
--- testtools/helpers.py	2011-03-22 15:25:38 +0000
+++ testtools/helpers.py	2011-06-01 06:49:27 +0000
@@ -6,7 +6,7 @@
     ]
 
 
-def try_import(name, alternative=None):
+def try_import(name, alternative=None, error_callback=None):
     """Attempt to import ``name``.  If it fails, return ``alternative``.
 
     When supporting multiple versions of Python or optional dependencies, it
@@ -16,29 +16,37 @@
         ``os.path.join``.
     :param alternative: The value to return if no module can be imported.
         Defaults to None.
+    :param error_callback: If non-None, a callable that is passed the ImportError
+        when the module cannot be loaded.
     """
     module_segments = name.split('.')
+    last_error = None
     while module_segments:
         module_name = '.'.join(module_segments)
         try:
             module = __import__(module_name)
-        except ImportError:
+        except ImportError, e:
+            last_error = e
             module_segments.pop()
             continue
         else:
             break
     else:
+        if last_error is not None and error_callback is not None:
+            error_callback(last_error)
         return alternative
     nonexistent = object()
     for segment in name.split('.')[1:]:
         module = getattr(module, segment, nonexistent)
         if module is nonexistent:
+            if last_error is not None and error_callback is not None:
+                error_callback(last_error)
             return alternative
     return module
 
 
 _RAISE_EXCEPTION = object()
-def try_imports(module_names, alternative=_RAISE_EXCEPTION):
+def try_imports(module_names, alternative=_RAISE_EXCEPTION, error_callback=None):
     """Attempt to import modules.
 
     Tries to import the first module in ``module_names``.  If it can be
@@ -50,12 +58,14 @@
     :param module_names: A sequence of module names to try to import.
     :param alternative: The value to return if no module can be imported.
         If unspecified, we raise an ImportError.
+    :param error_callback: If None, called with the ImportError for *each* 
+        module that fails to load.
     :raises ImportError: If none of the modules can be imported and no
         alternative value was specified.
     """
     module_names = list(module_names)
     for module_name in module_names:
-        module = try_import(module_name)
+        module = try_import(module_name, error_callback=error_callback)
         if module:
             return module
     if alternative is _RAISE_EXCEPTION:

=== modified file 'testtools/tests/test_helpers.py'
--- testtools/tests/test_helpers.py	2011-01-22 17:56:00 +0000
+++ testtools/tests/test_helpers.py	2011-06-01 06:49:27 +0000
@@ -1,4 +1,4 @@
-# Copyright (c) 2010 testtools developers. See LICENSE for details.
+# Copyright (c) 2010-2011 testtools developers. See LICENSE for details.
 
 from testtools import TestCase
 from testtools.helpers import (
@@ -8,7 +8,35 @@
 from testtools.matchers import (
     Equals,
     Is,
+    Not,
     )
+    
+    
+def check_error_callback(test, function, arg, expected_error_count, 
+    expect_result):
+    """General test template for error_callback argument.
+    
+    :param test: Test case instance.
+    :param function: Either try_import or try_imports.
+    :param arg: Name or names to import.
+    :param expected_error_count: Expected number of calls to the callback.
+    :param expect_result: Boolean for whether a module should
+        ultimately be returned or not.
+    """
+    cb_calls = []
+    def cb(e):
+        test.assertIsInstance(e, ImportError)
+        cb_calls.append(e)
+    try:
+        result = function(arg, error_callback=cb)
+    except ImportError, e:
+        test.assertFalse(expect_result)
+    else:
+        if expect_result:
+            test.assertThat(result, Not(Is(None)))
+        else:
+            test.assertThat(result, Is(None))
+    test.assertEquals(len(cb_calls), expected_error_count)
 
 
 class TestTryImport(TestCase):
@@ -51,7 +79,20 @@
         result = try_import('os.path.join')
         import os
         self.assertThat(result, Is(os.path.join))
-
+        
+    def test_error_callback(self):
+        # the error callback is called on failures.
+        check_error_callback(self, try_import, 'doesntexist', 1, False)
+
+    def test_error_callback_missing_module_member(self):
+        # the error callback is called on failures to find an object 
+        # inside an existing module.
+        check_error_callback(self, try_import, 'os.nonexistent', 1, False)
+
+    def test_error_callback_not_on_success(self):
+        # the error callback is not called on success.
+        check_error_callback(self, try_import, 'os.path', 0, True)
+        
 
 class TestTryImports(TestCase):
 
@@ -99,6 +140,18 @@
         result = try_imports(['os.doesntexist', 'os.path'])
         import os
         self.assertThat(result, Is(os.path))
+        
+    def test_error_callback(self):
+        # One error for every class that doesn't exist.
+        check_error_callback(self, try_imports, 
+            ['os.doesntexist', 'os.notthiseither'],
+            2, False)
+        check_error_callback(self, try_imports, 
+            ['os.doesntexist', 'os.notthiseither', 'os'],
+            2, True)
+        check_error_callback(self, try_imports, 
+            ['os.path'],
+            0, True)
 
 
 def test_suite():