From 67f40e18a7420f2112bbf10a8eca4f317517cab9 Mon Sep 17 00:00:00 2001
From: Bruno Oliveira <nicoddemus@gmail.com>
Date: Wed, 10 Oct 2018 18:23:11 -0300
Subject: [PATCH 1/2] Use attr.s(repr=False) because we customize MarkInfo's
 repr

---
 src/_pytest/mark/structures.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py
index 2c4635bf40e..ef2757d62e5 100644
--- a/src/_pytest/mark/structures.py
+++ b/src/_pytest/mark/structures.py
@@ -315,7 +315,7 @@ def _marked(func, mark):
     return any(mark == info.combined for info in func_mark)
 
 
-@attr.s
+@attr.s(repr=False)
 class MarkInfo(object):
     """ Marking object created by :class:`MarkDecorator` instances. """
 

From 5436e4299075236023e8875c563a7c74cad9bb2a Mon Sep 17 00:00:00 2001
From: Bruno Oliveira <nicoddemus@gmail.com>
Date: Wed, 3 Oct 2018 20:07:59 -0300
Subject: [PATCH 2/2] 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     |  4 ++
 changelog/2293.trivial.rst     |  1 +
 src/_pytest/fixtures.py        | 15 ++++---
 src/_pytest/mark/__init__.py   |  5 ---
 src/_pytest/mark/structures.py |  3 +-
 src/_pytest/nodes.py           |  4 ++
 src/_pytest/python.py          | 50 +++++++++++----------
 testing/python/fixture.py      | 81 ++++++++++++++++------------------
 testing/python/metafunc.py     | 38 ++++++++--------
 testing/test_mark.py           |  4 +-
 testing/test_runner.py         | 18 +++++++-
 11 files changed, 124 insertions(+), 99 deletions(-)
 create mode 100644 changelog/2293.feature.rst
 create mode 100644 changelog/2293.trivial.rst

diff --git a/changelog/2293.feature.rst b/changelog/2293.feature.rst
new file mode 100644
index 00000000000..5e56ba321f6
--- /dev/null
+++ b/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.
diff --git a/changelog/2293.trivial.rst b/changelog/2293.trivial.rst
new file mode 100644
index 00000000000..a1124512761
--- /dev/null
+++ b/changelog/2293.trivial.rst
@@ -0,0 +1 @@
+The internal ``MarkerError`` exception has been removed.
diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py
index 0b8b6b04462..3dec56a351e 100644
--- a/src/_pytest/fixtures.py
+++ b/src/_pytest/fixtures.py
@@ -579,7 +579,7 @@ def _compute_fixture_value(self, fixturedef):
                     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 @@ def _compute_fixture_value(self, fixturedef):
                         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 @@ def __init__(
         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)
diff --git a/src/_pytest/mark/__init__.py b/src/_pytest/mark/__init__.py
index eb25cd4aa59..39005742866 100644
--- a/src/_pytest/mark/__init__.py
+++ b/src/_pytest/mark/__init__.py
@@ -24,11 +24,6 @@
 ]
 
 
-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>`.
diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py
index ef2757d62e5..32822c2bb10 100644
--- a/src/_pytest/mark/structures.py
+++ b/src/_pytest/mark/structures.py
@@ -6,6 +6,7 @@
 
 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 @@ def _check(self, name):
             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()
diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py
index 9cd7589415c..d80895ab58f 100644
--- a/src/_pytest/nodes.py
+++ b/src/_pytest/nodes.py
@@ -9,6 +9,7 @@
 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 @@ def _prunetraceback(self, excinfo):
         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()
diff --git a/src/_pytest/python.py b/src/_pytest/python.py
index 25890138026..e9d05666f23 100644
--- a/src/_pytest/python.py
+++ b/src/_pytest/python.py
@@ -13,7 +13,6 @@
 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 @@ def _prunetraceback(self, excinfo):
                     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 @@ def parametrize(self, argnames, argvalues, indirect=False, ids=None, scope=None)
 
         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 @@ def _resolve_arg_ids(self, argnames, ids, parameters, item):
             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 @@ def _resolve_arg_value_types(self, argnames, indirect):
             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 @@ def _validate_if_using_arg_names(self, argnames, indirect):
         :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.
diff --git a/testing/python/fixture.py b/testing/python/fixture.py
index a9e33b45530..f1027032615 100644
--- a/testing/python/fixture.py
+++ b/testing/python/fixture.py
@@ -1217,8 +1217,7 @@ def test_nothing(badscope):
         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 @@ def test_foo(request, get_named_fixture):
         )
         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 @@ def test_foo(request):
         )
         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 @@ def test_foo(request):
         )
         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 @@ def test_foo(request):
         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*",
+            ]
         )
 
 
diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py
index 2172c5e0c93..fea59ee98c8 100644
--- a/testing/python/metafunc.py
+++ b/testing/python/metafunc.py
@@ -127,10 +127,11 @@ def func(x):
             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 @@ def func(x, y):
 
         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 @@ def func(x, y):
             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 @@ def test_ids_numbers(x,expected):
         )
         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 @@ def test_foo(x):
                 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):
diff --git a/testing/test_mark.py b/testing/test_mark.py
index 3e5c86624a1..48a68029717 100644
--- a/testing/test_mark.py
+++ b/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_hello():
     )
     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(
diff --git a/testing/test_runner.py b/testing/test_runner.py
index 74118069228..b9538cfad19 100644
--- a/testing/test_runner.py
+++ b/testing/test_runner.py
@@ -570,7 +570,8 @@ def pytest_configure(config):
     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 teardown_function(function):
     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).
