Skip to content

Improve internal error messages #4077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/2293.feature.rst
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions changelog/2293.trivial.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The internal ``MarkerError`` exception has been removed.
15 changes: 9 additions & 6 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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)
Expand Down Expand Up @@ -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,
)


Expand Down Expand Up @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions src/_pytest/mark/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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>`.
Expand Down
5 changes: 3 additions & 2 deletions src/_pytest/mark/structures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -315,7 +316,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. """

Expand Down Expand Up @@ -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()
Expand Down
4 changes: 4 additions & 0 deletions src/_pytest/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down
50 changes: 27 additions & 23 deletions src/_pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand Down
81 changes: 39 additions & 42 deletions testing/python/fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
)
)

Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand Down Expand Up @@ -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*",
]
)


Expand Down
Loading