Use pytest.fail(..., pytrace=False) when treating user errors

This prevents an enormous and often useless stack trace from showing
to end users.

Fix #3867
Fix #2293

changelog/2293.feature.rst

@@ -0,0 +1,4 @@
+Improve usage errors messages by hiding internal details which can be distracting and noisy.
+
+This has the side effect that some error conditions that previously raised generic errors (such as
+``ValueError`` for unregistered marks) are now raising ``Failed`` exceptions.

changelog/2293.trivial.rst

@@ -0,0 +1 @@
+The internal ``MarkerError`` exception has been removed.

src/_pytest/fixtures.py

@@ -579,7 +579,7 @@ class FixtureRequest(FuncargnamesCompatAttr):
                     nodeid=funcitem.nodeid,
                     typename=type(funcitem).__name__,
                 )
-                fail(msg)
+                fail(msg, pytrace=False)
             if has_params:
                 frame = inspect.stack()[3]
                 frameinfo = inspect.getframeinfo(frame[0])
@@ -600,7 +600,7 @@ class FixtureRequest(FuncargnamesCompatAttr):
                         source_lineno,
                     )
                 )
-                fail(msg)
+                fail(msg, pytrace=False)
         else:
             # indices might not be set if old-style metafunc.addcall() was used
             param_index = funcitem.callspec.indices.get(argname, 0)
@@ -718,10 +718,11 @@ def scope2index(scope, descr, where=None):
     try:
         return scopes.index(scope)
     except ValueError:
-        raise ValueError(
-            "{} {}has an unsupported scope value '{}'".format(
+        fail(
+            "{} {}got an unexpected scope value '{}'".format(
                 descr, "from {} ".format(where) if where else "", scope
-            )
+            ),
+            pytrace=False,
         )
 
 
@@ -854,7 +855,9 @@ class FixtureDef(object):
         self.argname = argname
         self.scope = scope
         self.scopenum = scope2index(
-            scope or "function", descr="fixture {}".format(func.__name__), where=baseid
+            scope or "function",
+            descr="Fixture '{}'".format(func.__name__),
+            where=baseid,
         )
         self.params = params
         self.argnames = getfuncargnames(func, is_method=unittest)

src/_pytest/mark/__init__.py

@@ -24,11 +24,6 @@ __all__ = [
 ]
 
 
-class MarkerError(Exception):
-
-    """Error in use of a pytest marker/attribute."""
-
-
 def param(*values, **kw):
     """Specify a parameter in `pytest.mark.parametrize`_ calls or
     :ref:`parametrized fixtures <fixture-parametrize-marks>`.

src/_pytest/mark/structures.py

@@ -6,6 +6,7 @@ from operator import attrgetter
 
 import attr
 
+from _pytest.outcomes import fail
 from ..deprecated import MARK_PARAMETERSET_UNPACKING, MARK_INFO_ATTRIBUTE
 from ..compat import NOTSET, getfslineno, MappingMixin
 from six.moves import map
@@ -393,7 +394,7 @@ class MarkGenerator(object):
             x = marker.split("(", 1)[0]
             values.add(x)
         if name not in self._markers:
-            raise AttributeError("%r not a registered marker" % (name,))
+            fail("{!r} not a registered marker".format(name), pytrace=False)
 
 
 MARK_GEN = MarkGenerator()

src/_pytest/nodes.py

@@ -9,6 +9,7 @@ import attr
 import _pytest
 import _pytest._code
 from _pytest.compat import getfslineno
+from _pytest.outcomes import fail
 
 from _pytest.mark.structures import NodeKeywords, MarkInfo
 
@@ -346,6 +347,9 @@ class Node(object):
         pass
 
     def _repr_failure_py(self, excinfo, style=None):
+        if excinfo.errisinstance(fail.Exception):
+            if not excinfo.value.pytrace:
+                return six.text_type(excinfo.value)
         fm = self.session._fixturemanager
         if excinfo.errisinstance(fm.FixtureLookupError):
             return excinfo.value.formatrepr()

src/_pytest/python.py

@@ -13,7 +13,6 @@ from textwrap import dedent
 import py
 import six
 from _pytest.main import FSHookProxy
-from _pytest.mark import MarkerError
 from _pytest.config import hookimpl
 
 import _pytest
@@ -159,8 +158,8 @@ def pytest_generate_tests(metafunc):
     alt_spellings = ["parameterize", "parametrise", "parameterise"]
     for attr in alt_spellings:
         if hasattr(metafunc.function, attr):
-            msg = "{0} has '{1}', spelling should be 'parametrize'"
-            raise MarkerError(msg.format(metafunc.function.__name__, attr))
+            msg = "{0} has '{1}' mark, spelling should be 'parametrize'"
+            fail(msg.format(metafunc.function.__name__, attr), pytrace=False)
     for marker in metafunc.definition.iter_markers(name="parametrize"):
         metafunc.parametrize(*marker.args, **marker.kwargs)
 
@@ -760,12 +759,6 @@ class FunctionMixin(PyobjMixin):
                     for entry in excinfo.traceback[1:-1]:
                         entry.set_repr_style("short")
 
-    def _repr_failure_py(self, excinfo, style="long"):
-        if excinfo.errisinstance(fail.Exception):
-            if not excinfo.value.pytrace:
-                return six.text_type(excinfo.value)
-        return super(FunctionMixin, self)._repr_failure_py(excinfo, style=style)
-
     def repr_failure(self, excinfo, outerr=None):
         assert outerr is None, "XXX outerr usage is deprecated"
         style = self.config.option.tbstyle
@@ -987,7 +980,9 @@ class Metafunc(fixtures.FuncargnamesCompatAttr):
 
         ids = self._resolve_arg_ids(argnames, ids, parameters, item=self.definition)
 
-        scopenum = scope2index(scope, descr="call to {}".format(self.parametrize))
+        scopenum = scope2index(
+            scope, descr="parametrize() call in {}".format(self.function.__name__)
+        )
 
         # create the new calls: if we are parametrize() multiple times (by applying the decorator
         # more than once) then we accumulate those calls generating the cartesian product
@@ -1026,15 +1021,16 @@ class Metafunc(fixtures.FuncargnamesCompatAttr):
             idfn = ids
             ids = None
         if ids:
+            func_name = self.function.__name__
             if len(ids) != len(parameters):
-                raise ValueError(
-                    "%d tests specified with %d ids" % (len(parameters), len(ids))
-                )
+                msg = "In {}: {} parameter sets specified, with different number of ids: {}"
+                fail(msg.format(func_name, len(parameters), len(ids)), pytrace=False)
             for id_value in ids:
                 if id_value is not None and not isinstance(id_value, six.string_types):
-                    msg = "ids must be list of strings, found: %s (type: %s)"
-                    raise ValueError(
-                        msg % (saferepr(id_value), type(id_value).__name__)
+                    msg = "In {}: ids must be list of strings, found: {} (type: {!r})"
+                    fail(
+                        msg.format(func_name, saferepr(id_value), type(id_value)),
+                        pytrace=False,
                     )
         ids = idmaker(argnames, parameters, idfn, ids, self.config, item=item)
         return ids
@@ -1059,9 +1055,11 @@ class Metafunc(fixtures.FuncargnamesCompatAttr):
             valtypes = dict.fromkeys(argnames, "funcargs")
             for arg in indirect:
                 if arg not in argnames:
-                    raise ValueError(
-                        "indirect given to %r: fixture %r doesn't exist"
-                        % (self.function, arg)
+                    fail(
+                        "In {}: indirect fixture '{}' doesn't exist".format(
+                            self.function.__name__, arg
+                        ),
+                        pytrace=False,
                     )
                 valtypes[arg] = "params"
         return valtypes
@@ -1075,19 +1073,25 @@ class Metafunc(fixtures.FuncargnamesCompatAttr):
         :raise ValueError: if validation fails.
         """
         default_arg_names = set(get_default_arg_names(self.function))
+        func_name = self.function.__name__
         for arg in argnames:
             if arg not in self.fixturenames:
                 if arg in default_arg_names:
-                    raise ValueError(
-                        "%r already takes an argument %r with a default value"
-                        % (self.function, arg)
+                    fail(
+                        "In {}: function already takes an argument '{}' with a default value".format(
+                            func_name, arg
+                        ),
+                        pytrace=False,
                     )
                 else:
                     if isinstance(indirect, (tuple, list)):
                         name = "fixture" if arg in indirect else "argument"
                     else:
                         name = "fixture" if indirect else "argument"
-                    raise ValueError("%r uses no %s %r" % (self.function, name, arg))
+                    fail(
+                        "In {}: function uses no {} '{}'".format(func_name, name, arg),
+                        pytrace=False,
+                    )
 
     def addcall(self, funcargs=None, id=NOTSET, param=NOTSET):
         """ Add a new call to the underlying test function during the collection phase of a test run.

testing/python/fixture.py

@@ -1217,8 +1217,7 @@ class TestFixtureUsages(object):
         result = testdir.runpytest_inprocess()
         result.stdout.fnmatch_lines(
             (
-                "*ValueError: fixture badscope from test_invalid_scope.py has an unsupported"
-                " scope value 'functions'"
+                "*Fixture 'badscope' from test_invalid_scope.py got an unexpected scope value 'functions'"
             )
         )
 
@@ -3607,16 +3606,15 @@ class TestParameterizedSubRequest(object):
         )
         result = testdir.runpytest()
         result.stdout.fnmatch_lines(
-            """
-            E*Failed: The requested fixture has no parameter defined for test:
-            E*    test_call_from_fixture.py::test_foo
-            E*
-            E*Requested fixture 'fix_with_param' defined in:
-            E*test_call_from_fixture.py:4
-            E*Requested here:
-            E*test_call_from_fixture.py:9
-            *1 error*
-            """
+            [
+                "The requested fixture has no parameter defined for test:",
+                "    test_call_from_fixture.py::test_foo",
+                "Requested fixture 'fix_with_param' defined in:",
+                "test_call_from_fixture.py:4",
+                "Requested here:",
+                "test_call_from_fixture.py:9",
+                "*1 error in*",
+            ]
         )
 
     def test_call_from_test(self, testdir):
@@ -3634,16 +3632,15 @@ class TestParameterizedSubRequest(object):
         )
         result = testdir.runpytest()
         result.stdout.fnmatch_lines(
-            """
-            E*Failed: The requested fixture has no parameter defined for test:
-            E*    test_call_from_test.py::test_foo
-            E*
-            E*Requested fixture 'fix_with_param' defined in:
-            E*test_call_from_test.py:4
-            E*Requested here:
-            E*test_call_from_test.py:8
-            *1 failed*
-            """
+            [
+                "The requested fixture has no parameter defined for test:",
+                "    test_call_from_test.py::test_foo",
+                "Requested fixture 'fix_with_param' defined in:",
+                "test_call_from_test.py:4",
+                "Requested here:",
+                "test_call_from_test.py:8",
+                "*1 failed*",
+            ]
         )
 
     def test_external_fixture(self, testdir):
@@ -3665,16 +3662,16 @@ class TestParameterizedSubRequest(object):
         )
         result = testdir.runpytest()
         result.stdout.fnmatch_lines(
-            """
-            E*Failed: The requested fixture has no parameter defined for test:
-            E*    test_external_fixture.py::test_foo
-            E*
-            E*Requested fixture 'fix_with_param' defined in:
-            E*conftest.py:4
-            E*Requested here:
-            E*test_external_fixture.py:2
-            *1 failed*
-            """
+            [
+                "The requested fixture has no parameter defined for test:",
+                "    test_external_fixture.py::test_foo",
+                "",
+                "Requested fixture 'fix_with_param' defined in:",
+                "conftest.py:4",
+                "Requested here:",
+                "test_external_fixture.py:2",
+                "*1 failed*",
+            ]
         )
 
     def test_non_relative_path(self, testdir):
@@ -3709,16 +3706,16 @@ class TestParameterizedSubRequest(object):
         testdir.syspathinsert(fixdir)
         result = testdir.runpytest()
         result.stdout.fnmatch_lines(
-            """
-            E*Failed: The requested fixture has no parameter defined for test:
-            E*    test_foos.py::test_foo
-            E*
-            E*Requested fixture 'fix_with_param' defined in:
-            E*fix.py:4
-            E*Requested here:
-            E*test_foos.py:4
-            *1 failed*
-            """
+            [
+                "The requested fixture has no parameter defined for test:",
+                "    test_foos.py::test_foo",
+                "",
+                "Requested fixture 'fix_with_param' defined in:",
+                "*fix.py:4",
+                "Requested here:",
+                "test_foos.py:4",
+                "*1 failed*",
+            ]
         )
 
 

testing/python/metafunc.py

@@ -127,10 +127,11 @@ class TestMetafunc(object):
             pass
 
         metafunc = self.Metafunc(func)
-        try:
+        with pytest.raises(
+            pytest.fail.Exception,
+            match=r"parametrize\(\) call in func got an unexpected scope value 'doggy'",
+        ):
             metafunc.parametrize("x", [1], scope="doggy")
-        except ValueError as ve:
-            assert "has an unsupported scope value 'doggy'" in str(ve)
 
     def test_find_parametrized_scope(self):
         """unittest for _find_parametrized_scope (#3941)"""
@@ -206,16 +207,13 @@ class TestMetafunc(object):
 
         metafunc = self.Metafunc(func)
 
-        pytest.raises(
-            ValueError, lambda: metafunc.parametrize("x", [1, 2], ids=["basic"])
-        )
+        with pytest.raises(pytest.fail.Exception):
+            metafunc.parametrize("x", [1, 2], ids=["basic"])
 
-        pytest.raises(
-            ValueError,
-            lambda: metafunc.parametrize(
+        with pytest.raises(pytest.fail.Exception):
+            metafunc.parametrize(
                 ("x", "y"), [("abc", "def"), ("ghi", "jkl")], ids=["one"]
-            ),
-        )
+            )
 
     @pytest.mark.issue510
     def test_parametrize_empty_list(self):
@@ -573,7 +571,7 @@ class TestMetafunc(object):
             pass
 
         metafunc = self.Metafunc(func)
-        with pytest.raises(ValueError):
+        with pytest.raises(pytest.fail.Exception):
             metafunc.parametrize("x, y", [("a", "b")], indirect=["x", "z"])
 
     @pytest.mark.issue714
@@ -1189,7 +1187,9 @@ class TestMetafuncFunctional(object):
         )
         result = testdir.runpytest()
         result.stdout.fnmatch_lines(
-            ["*ids must be list of strings, found: 2 (type: int)*"]
+            [
+                "*In test_ids_numbers: ids must be list of strings, found: 2 (type: *'int'>)*"
+            ]
         )
 
     def test_parametrize_with_identical_ids_get_unique_names(self, testdir):
@@ -1326,13 +1326,13 @@ class TestMetafuncFunctional(object):
                 attr
             )
         )
-        reprec = testdir.inline_run("--collectonly")
-        failures = reprec.getfailures()
-        assert len(failures) == 1
-        expectederror = "MarkerError: test_foo has '{}', spelling should be 'parametrize'".format(
-            attr
+        result = testdir.runpytest("--collectonly")
+        result.stdout.fnmatch_lines(
+            [
+                "test_foo has '{}' mark, spelling should be 'parametrize'".format(attr),
+                "*1 error in*",
+            ]
         )
-        assert expectederror in failures[0].longrepr.reprcrash.message
 
 
 class TestMetafuncFunctionalAuto(object):

testing/test_mark.py

@@ -247,7 +247,7 @@ def test_marker_without_description(testdir):
     )
     ftdir = testdir.mkdir("ft1_dummy")
     testdir.tmpdir.join("conftest.py").move(ftdir.join("conftest.py"))
-    rec = testdir.runpytest_subprocess("--strict")
+    rec = testdir.runpytest("--strict")
     rec.assert_outcomes()
 
 
@@ -302,7 +302,7 @@ def test_strict_prohibits_unregistered_markers(testdir):
     )
     result = testdir.runpytest("--strict")
     assert result.ret != 0
-    result.stdout.fnmatch_lines(["*unregisteredmark*not*registered*"])
+    result.stdout.fnmatch_lines(["'unregisteredmark' not a registered marker"])
 
 
 @pytest.mark.parametrize(

testing/test_runner.py

@@ -570,7 +570,8 @@ def test_pytest_exit_msg(testdir):
     result.stderr.fnmatch_lines(["Exit: oh noes"])
 
 
-def test_pytest_fail_notrace(testdir):
+def test_pytest_fail_notrace_runtest(testdir):
+    """Test pytest.fail(..., pytrace=False) does not show tracebacks during test run."""
     testdir.makepyfile(
         """
         import pytest
@@ -585,6 +586,21 @@ def test_pytest_fail_notrace(testdir):
     assert "def teardown_function" not in result.stdout.str()
 
 
+def test_pytest_fail_notrace_collection(testdir):
+    """Test pytest.fail(..., pytrace=False) does not show tracebacks during collection."""
+    testdir.makepyfile(
+        """
+        import pytest
+        def some_internal_function():
+            pytest.fail("hello", pytrace=False)
+        some_internal_function()
+    """
+    )
+    result = testdir.runpytest()
+    result.stdout.fnmatch_lines(["hello"])
+    assert "def some_internal_function()" not in result.stdout.str()
+
+
 @pytest.mark.parametrize("str_prefix", ["u", ""])
 def test_pytest_fail_notrace_non_ascii(testdir, str_prefix):
     """Fix pytest.fail with pytrace=False with non-ascii characters (#1178).