refine and extend custom error reporting particularly for collection-related errors

--HG--
branch : trunk

CHANGELOG

@@ -28,9 +28,17 @@ New features
     def test_function(arg):
         ...
 
+- customizable error reporting: allow custom error reporting for 
+  custom (test and particularly collection) nodes by always calling 
+  ``node.repr_failure(excinfo)`` which you may override to return a 
+  string error representation of your choice which is going to be 
+  reported as a (red) string. 
+
 Bug fixes / Maintenance
 ++++++++++++++++++++++++++
 
+- improve error messages if importing a test module failed (ImportError,
+  import file mismatches, syntax errors) 
 - refine --pdb: ignore xfailed tests, unify its TB-reporting and 
   don't display failures again at the end.
 - fix assertion interpretation with the ** operator (thanks Benjamin Peterson)

doc/test/customize.txt

@@ -466,6 +466,15 @@ and test classes and methods. Test functions and methods
 are prefixed ``test`` by default.  Test classes must 
 start with a capitalized ``Test`` prefix. 
 
+Customizing error messages 
+-------------------------------------------------
+
+On test and collection nodes ``py.test`` will invoke 
+the ``node.repr_failure(excinfo)`` function which
+you may override and make it return an error 
+representation string of your choice.  It 
+will be reported as a (red) string. 
+
 .. _`package name`: 
 
 constructing the package name for test modules

py/_path/local.py

@@ -90,6 +90,9 @@ class LocalPath(FSBase):
     """ object oriented interface to os.path and other local filesystem 
         related information. 
     """
+    class ImportMismatchError(ImportError):
+        """ raised on pyimport() if there is a mismatch of __file__'s"""
+        
     sep = os.sep
     class Checkers(common.Checkers):
         def _stat(self):
@@ -531,10 +534,7 @@ class LocalPath(FSBase):
             if modfile.endswith("__init__.py"):
                 modfile = modfile[:-12]
             if not self.samefile(modfile):
-                raise EnvironmentError("mismatch:\n"
+                raise self.ImportMismatchError(modname, modfile, self)
-                "imported module %r\n"
-                "does not stem from %r\n" 
-                "maybe __init__.py files are missing?" % (mod, str(self)))
             return mod
         else:
             try:

py/_plugin/pytest_runner.py

@@ -127,6 +127,12 @@ class BaseReport(object):
         else:
             out.line(str(longrepr))
 
+class CollectErrorRepr(BaseReport):
+    def __init__(self, msg):
+        self.longrepr = msg 
+    def toterminal(self, out):
+        out.line(str(self.longrepr), red=True)
+   
 class ItemTestReport(BaseReport):
     failed = passed = skipped = False
 
@@ -188,16 +194,16 @@ class CollectReport(BaseReport):
             self.passed = True
             self.result = result 
         else:
-            style = "short"
-            if collector.config.getvalue("fulltrace"):
-                style = "long"
-            self.longrepr = self.collector._repr_failure_py(excinfo, 
-                style=style)
             if excinfo.errisinstance(py.test.skip.Exception):
                 self.skipped = True
                 self.reason = str(excinfo.value)
+                self.longrepr = self.collector._repr_failure_py(excinfo, "line")
             else:
                 self.failed = True
+                errorinfo = self.collector.repr_failure(excinfo)
+                if not hasattr(errorinfo, "toterminal"):
+                    errorinfo = CollectErrorRepr(errorinfo)
+                self.longrepr = errorinfo 
 
     def getnode(self):
         return self.collector 
@@ -448,3 +454,4 @@ def importorskip(modname, minversion=None):
                      modname, verattr, minversion))
     return mod
 
+

py/_plugin/pytest_terminal.py

@@ -274,7 +274,6 @@ class TerminalReporter:
         if not report.passed:
             if report.failed:
                 self.stats.setdefault("error", []).append(report)
-                msg = report.longrepr.reprcrash.message 
                 self.write_fspath_result(report.collector.fspath, "E")
             elif report.skipped:
                 self.stats.setdefault("skipped", []).append(report)
@@ -403,7 +402,7 @@ class TerminalReporter:
                 msg = self._getfailureheadline(rep)
                 if not hasattr(rep, 'when'):
                     # collect
-                    msg = "ERROR during collection " + msg
+                    msg = "ERROR collecting " + msg
                 elif rep.when == "setup":
                     msg = "ERROR at setup of " + msg 
                 elif rep.when == "teardown":

py/_test/collect.py

@@ -174,7 +174,10 @@ class Node(object):
         return traceback 
 
     def _repr_failure_py(self, excinfo, style=None):
-        excinfo.traceback = self._prunetraceback(excinfo.traceback)
+        if self.config.option.fulltrace:
+            style="long"
+        else:
+            excinfo.traceback = self._prunetraceback(excinfo.traceback)
         # XXX should excinfo.getrepr record all data and toterminal()
         # process it? 
         if style is None:
@@ -200,6 +203,8 @@ class Collector(Node):
     """
     Directory = configproperty('Directory')
     Module = configproperty('Module')
+    class CollectError(Exception):
+        """ an error during collection, contains a custom message. """
 
     def collect(self):
         """ returns a list of children (items and collectors) 
@@ -213,10 +218,12 @@ class Collector(Node):
             if colitem.name == name:
                 return colitem
 
-    def repr_failure(self, excinfo, outerr=None):
+    def repr_failure(self, excinfo):
         """ represent a failure. """
-        assert outerr is None, "XXX deprecated"
+        if excinfo.errisinstance(self.CollectError):
-        return self._repr_failure_py(excinfo)
+            exc = excinfo.value
+            return str(exc.args[0])
+        return self._repr_failure_py(excinfo, style="short")
 
     def _memocollect(self):
         """ internal helper method to cache results of calling collect(). """

py/_test/pycollect.py

@@ -3,6 +3,7 @@ Python related collection nodes.
 """ 
 import py
 import inspect
+import sys
 from py._test.collect import configproperty, warnoldcollect
 from py._test import funcargs
 from py._code.code import TerminalRepr
@@ -140,7 +141,22 @@ class Module(py.test.collect.File, PyCollectorMixin):
 
     def _importtestmodule(self):
         # we assume we are only called once per module 
-        mod = self.fspath.pyimport()
+        try:
+            mod = self.fspath.pyimport(ensuresyspath=True)
+        except SyntaxError:
+            excinfo = py.code.ExceptionInfo()
+            raise self.CollectError(excinfo.getrepr(style="short"))
+        except self.fspath.ImportMismatchError:
+            e = sys.exc_info()[1]
+            raise self.CollectError(
+                "import file mismatch:\n"
+                "imported module %r has this __file__ attribute:\n" 
+                "  %s\n"
+                "which is not the same as the test file we want to collect:\n"
+                "  %s\n"
+                "HINT: use a unique basename for your test file modules"
+                 % e.args
+            )
         #print "imported test module", mod
         self.config.pluginmanager.consider_module(mod)
         return mod

testing/path/test_local.py

@@ -360,10 +360,13 @@ class TestImport:
         pseudopath = tmpdir.ensure(name+"123.py")
         mod.__file__ = str(pseudopath)
         monkeypatch.setitem(sys.modules, name, mod)
-        excinfo = py.test.raises(EnvironmentError, "p.pyimport()")
+        excinfo = py.test.raises(pseudopath.ImportMismatchError, 
-        s = str(excinfo.value)
+            "p.pyimport()")
-        assert "mismatch" in s 
+        modname, modfile, orig = excinfo.value.args
-        assert name+"123" in s 
+        assert modname == name
+        assert modfile == pseudopath 
+        assert orig == p
+        assert issubclass(pseudopath.ImportMismatchError, ImportError)
 
 def test_pypkgdir(tmpdir):
     pkg = tmpdir.ensure('pkg1', dir=1)

testing/plugin/test_pytest_capture.py

@@ -84,6 +84,7 @@ def test_capturing_unicode(testdir, method):
     else:
         obj = "u'\u00f6y'"
     testdir.makepyfile("""
+        # coding=utf8
         # taken from issue 227 from nosetests
         def test_unicode():
             import sys

testing/test_collect.py

@@ -152,10 +152,35 @@ class TestPrunetraceback:
         result = testdir.runpytest(p)
         assert "__import__" not in result.stdout.str(), "too long traceback"
         result.stdout.fnmatch_lines([
-            "*ERROR during collection*",
+            "*ERROR collecting*",
             "*mport*not_exists*"
         ])
 
+    def test_custom_repr_failure(self, testdir):
+        p = testdir.makepyfile("""
+            import not_exists
+        """)
+        testdir.makeconftest("""
+            import py
+            def pytest_collect_file(path, parent):
+                return MyFile(path, parent)
+            class MyError(Exception):
+                pass
+            class MyFile(py.test.collect.File):
+                def collect(self):
+                    raise MyError()
+                def repr_failure(self, excinfo):
+                    if excinfo.errisinstance(MyError):
+                        return "hello world"
+                    return py.test.collect.File.repr_failure(self, excinfo)
+        """)
+
+        result = testdir.runpytest(p)
+        result.stdout.fnmatch_lines([
+            "*ERROR collecting*",
+            "*hello world*",
+        ])
+
 class TestCustomConftests:
     def test_ignore_collect_path(self, testdir):
         testdir.makeconftest("""

testing/test_pycollect.py

@@ -22,15 +22,20 @@ class TestModule:
         del py.std.sys.modules['test_whatever']
         b.ensure("test_whatever.py")
         result = testdir.runpytest()
-        s = result.stdout.str()
+        result.stdout.fnmatch_lines([
-        assert 'mismatch' in s
+            "*import*mismatch*",
-        assert 'test_whatever' in s
+            "*imported*test_whatever*",
+            "*%s*" % a.join("test_whatever.py"),
+            "*not the same*",
+            "*%s*" % b.join("test_whatever.py"),
+            "*HINT*",
+        ])
 
     def test_syntax_error_in_module(self, testdir):
         modcol = testdir.getmodulecol("this is a syntax error") 
-        py.test.raises(SyntaxError, modcol.collect)
+        py.test.raises(modcol.CollectError, modcol.collect)
-        py.test.raises(SyntaxError, modcol.collect)
+        py.test.raises(modcol.CollectError, modcol.collect)
-        py.test.raises(SyntaxError, modcol.run)
+        py.test.raises(modcol.CollectError, modcol.run)
 
     def test_module_considers_pluginmanager_at_import(self, testdir):
         modcol = testdir.getmodulecol("pytest_plugins='xasdlkj',")

testing/test_session.py

@@ -74,7 +74,7 @@ class SessionTests:
         reprec = testdir.inline_runsource("this is really not python")
         l = reprec.getfailedcollections()
         assert len(l) == 1
-        out = l[0].longrepr.reprcrash.message
+        out = str(l[0].longrepr)
         assert out.find(str('not python')) != -1
 
     def test_exit_first_problem(self, testdir):