Merge pull request #1143 from twisted/9628-trial3-importerror

[9628] Fix trial3 not knowing the difference between ImportErrors and modules not existing

Author: hawkowl

Reviewers: markrwilliams

Fixes: ticket:9628

src/twisted/newsfragments/9628.bugfix

@@ -0,0 +1 @@
+Trial on Python 3 will now properly re-raise ImportErrors that occur during the import of a module, rather than saying the module doesn't exist.

src/twisted/trial/runner.py

@@ -724,8 +724,27 @@ def findByName(self, _name, recurse=False):
                 break
 
             except ImportError:
+                # Check to see where the ImportError happened. If it happened
+                # in this file, ignore it.
+                tb = sys.exc_info()[2]
+
+                # Walk down to the deepest frame, where it actually happened.
+                while tb.tb_next is not None:
+                    tb = tb.tb_next
+
+                # Get the filename that the ImportError originated in.
+                filenameWhereHappened = tb.tb_frame.f_code.co_filename
+
+                # If it originated in the reflect file, then it's because it
+                # doesn't exist. If it originates elsewhere, it's because an
+                # ImportError happened in a module that does exist.
+                if filenameWhereHappened != reflect.__file__:
+                    raise
+
                 if remaining == "":
-                    raise reflect.ModuleNotFound("The module {} does not exist.".format(name))
+                    raise reflect.ModuleNotFound(
+                        "The module {} does not exist.".format(name)
+                    )
 
         if obj is None:
             # If it's none here, we didn't get to import anything.

src/twisted/trial/test/packages.py

@@ -102,6 +102,8 @@ class PackageTest(unittest.SynchronousTestCase):
     files = [
         ('badpackage/__init__.py', 'frotz\n'),
         ('badpackage/test_module.py', ''),
+        ('unimportablepackage/__init__.py', ''),
+        ('unimportablepackage/test_module.py', 'import notarealmoduleok\n'),
         ('package2/__init__.py', ''),
         ('package2/test_module.py', 'import frotz\n'),
         ('package/__init__.py', ''),

src/twisted/trial/test/test_loader.py

@@ -21,6 +21,7 @@
 
 from twisted.python.modules import getModule
 from twisted.python.compat import _PY3
+from twisted.python.reflect import ModuleNotFound
 
 
 
@@ -35,65 +36,111 @@ def testNames(tests):
 
 
 
-class FinderTests(packages.PackageTest):
+class FinderPy2Tests(unittest.SynchronousTestCase):
     """
     Tests for L{runner.TestLoader.findByName}.
     """
+    if _PY3:
+        skip = "Not relevant on Python 3"
+
     def setUp(self):
-        packages.PackageTest.setUp(self)
         self.loader = runner.TestLoader()
 
-    def tearDown(self):
-        packages.PackageTest.tearDown(self)
 
     def test_findPackage(self):
         sample1 = self.loader.findByName('twisted')
         import twisted as sample2
         self.assertEqual(sample1, sample2)
 
+
     def test_findModule(self):
         sample1 = self.loader.findByName('twisted.trial.test.sample')
         from twisted.trial.test import sample as sample2
         self.assertEqual(sample1, sample2)
 
+
     def test_findFile(self):
         path = util.sibpath(__file__, 'sample.py')
         sample1 = self.loader.findByName(path)
         from twisted.trial.test import sample as sample2
         self.assertEqual(sample1, sample2)
 
+
     def test_findObject(self):
         sample1 = self.loader.findByName('twisted.trial.test.sample.FooTest')
         from twisted.trial.test import sample
         self.assertEqual(sample.FooTest, sample1)
 
-    if _PY3:
-        # In Python 3, `findByName` returns full TestCases, not the objects
-        # inside them. This because on Python 3, unbound methods don't exist,
-        # so you can't simply make a TestCase after finding it -- it's easier
-        # to just find it and put it in a TestCase immediately.
-        _Py3SkipMsg = ("Not relevant on Python 3")
-        test_findPackage.skip = _Py3SkipMsg
-        test_findModule.skip = _Py3SkipMsg
-        test_findFile.skip = _Py3SkipMsg
-        test_findObject.skip = _Py3SkipMsg
 
     def test_findNonModule(self):
-        self.assertRaises(AttributeError,
-                              self.loader.findByName,
-                              'twisted.trial.test.nonexistent')
+        self.assertRaises(
+            AttributeError, self.loader.findByName,
+            'twisted.trial.test.nonexistent'
+        )
+
 
     def test_findNonPackage(self):
-        self.assertRaises(ValueError,
-                              self.loader.findByName,
-                              'nonextant')
+        self.assertRaises(
+            ValueError, self.loader.findByName,
+            'nonextant'
+        )
+
 
     def test_findNonFile(self):
         path = util.sibpath(__file__, 'nonexistent.py')
         self.assertRaises(ValueError, self.loader.findByName, path)
 
 
 
+class FinderPy3Tests(packages.SysPathManglingTest):
+
+    if not _PY3:
+        skip = "Not relevant on Python 2"
+
+    def setUp(self):
+        super(FinderPy3Tests, self).setUp()
+        self.loader = runner.TestLoader()
+
+
+    def test_findNonModule(self):
+        """
+        findByName, if given something findable up until the last entry, will
+        raise AttributeError (as it cannot tell if 'nonexistent' here is
+        supposed to be a module or a class).
+        """
+        self.assertRaises(
+            AttributeError, self.loader.findByName,
+            'twisted.trial.test.nonexistent'
+        )
+
+
+    def test_findNonPackage(self):
+        self.assertRaises(
+            ModuleNotFound, self.loader.findByName, 'nonextant'
+        )
+
+
+    def test_findNonFile(self):
+        """
+        findByName, given a file path that doesn't exist, will raise a
+        ValueError saying that it is not a Python file.
+        """
+        path = util.sibpath(__file__, 'nonexistent.py')
+        self.assertRaises(ValueError, self.loader.findByName, path)
+
+
+    def test_findFileWithImportError(self):
+        """
+        findByName will re-raise ImportErrors inside modules that it has found
+        and imported.
+        """
+        self.assertRaises(
+            ImportError, self.loader.findByName,
+            "unimportablepackage.test_module"
+        )
+
+
+
 class FileTests(packages.SysPathManglingTest):
     """
     Tests for L{runner.filenameToModule}.