Indicate failed imports in list-tests.

Test listing now explicitly indicates by printing 'Failed to import' and
exiting (2) when an import has failed rather than only signalling through the
test name. This plus the introduction of a public helper lays the groundwork
for fixing a usability bug in testrepository caused by the interaction between
failed listings and user test filters.

Change-Id: I7fb8cb70c515c54d8febd68ba791c27db0a7a9c4
Closes-Bug: #1245672

NEWS

@@ -19,6 +19,10 @@ Improvements
 * Network tests now bind to 127.0.0.1 to avoid (even temporary) network
   visible ports. (Benedikt Morbach, github #46)
 
+* Test listing now explicitly indicates by printing 'Failed to import' and
+  exiting (2) when an import has failed rather than only signalling through the
+  test name. (Robert Collins, #1245672)
+
 0.9.33
 ~~~~~~
 

testtools/run.py

@@ -35,6 +35,31 @@
     have_discover = True
 
 
+def list_test(test):
+    """Return the test ids that would be run if test() was run.
+
+    When things fail to import they can be represented as well, though
+    we use an ugly hack (see http://bugs.python.org/issue19746 for details)
+    to determine that. The difference matters because if a user is
+    filtering tests to run on the returned ids, a failed import can reduce
+    the visible tests but it can be impossible to tell that the selected
+    test would have been one of the imported ones.
+
+    :return: A tuple of test ids that would run and error strings
+        describing things that failed to import.
+    """
+    unittest_import_str = 'unittest.loader.ModuleImportFailure.' 
+    test_ids = []
+    errors = []
+    for test in iterate_tests(test):
+        # to this ugly.
+        if test.id().startswith(unittest_import_str):
+            errors.append(test.id()[len(unittest_import_str):])
+        else:
+            test_ids.append(test.id())
+    return test_ids, errors
+
+
 class TestToolsTestRunner(object):
     """ A thunk object to support unittest.TestProgram."""
 
@@ -52,8 +77,14 @@ def __init__(self, verbosity=None, failfast=None, buffer=None,
 
     def list(self, test):
         """List the tests that would be run if test() was run."""
-        for test in iterate_tests(test):
-            self.stdout.write('%s\n' % test.id())
+        test_ids, errors = list_test(test)
+        for test_id in test_ids:
+            self.stdout.write('%s\n' % test_id)
+        if errors:
+            self.stdout.write('Failed to import\n')
+            for test_id in errors:
+                self.stdout.write('%s\n' % test_id)
+            sys.exit(2)
 
     def run(self, test):
         "Run the given test case or test suite."

testtools/tests/test_run.py

@@ -22,9 +22,13 @@
     class SampleTestFixture(fixtures.Fixture):
         """Creates testtools.runexample temporarily."""
 
-        def __init__(self):
-            self.package = fixtures.PythonPackage(
-            'runexample', [('__init__.py', _b("""
+        def __init__(self, broken=False):
+            """Create a SampleTestFixture.
+
+            :param broken: If True, the sample file will not be importable.
+            """
+            if not broken:
+                init_contents = _b("""\
 from testtools import TestCase
 
 class TestFoo(TestCase):
@@ -35,7 +39,11 @@ def test_quux(self):
 def test_suite():
     from unittest import TestLoader
     return TestLoader().loadTestsFromName(__name__)
-"""))])
+""")
+            else:
+                init_contents = b"class not in\n"
+            self.package = fixtures.PythonPackage(
+            'runexample', [('__init__.py', init_contents)])
 
         def setUp(self):
             super(SampleTestFixture, self).setUp()
@@ -127,6 +135,19 @@ def test_run_list(self):
             raise AssertionError("-l tried to exit. %r" % exc_info[1])
         self.assertEqual("""testtools.runexample.TestFoo.test_bar
 testtools.runexample.TestFoo.test_quux
+""", out.getvalue())
+
+    def test_run_list_failed_import(self):
+        if not run.have_discover:
+            self.skipTest("Need discover")
+        broken = self.useFixture(SampleTestFixture(broken=True))
+        out = StringIO()
+        exc = self.assertRaises(
+            SystemExit,
+            run.main, ['prog', 'discover', '-l', broken.package.base, '*.py'], out)
+        self.assertEqual(2, exc.args[0])
+        self.assertEqual("""Failed to import
+runexample.__init__
 """, out.getvalue())
 
     def test_run_orders_tests(self):