From b42518acd5c8f7d8d034bd47addd06c36b062b48 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 4 Sep 2018 14:20:42 -0300 Subject: [PATCH] Change std_warn to receive a single warning instance, addressed review suggestions --- src/_pytest/config/__init__.py | 9 ++++----- src/_pytest/deprecated.py | 17 +++++++++++++---- src/_pytest/fixtures.py | 3 ++- src/_pytest/junitxml.py | 5 ++--- src/_pytest/mark/structures.py | 12 +++++++----- src/_pytest/nodes.py | 26 ++++++++++++++------------ src/_pytest/pytester.py | 8 +++----- src/_pytest/python.py | 33 +++++++++++++++------------------ src/_pytest/warning_types.py | 3 +++ src/_pytest/warnings.py | 6 +++--- testing/acceptance_test.py | 2 +- testing/deprecated_test.py | 3 ++- testing/python/metafunc.py | 8 ++++---- testing/test_mark.py | 6 +++++- testing/test_nodes.py | 11 +++++++++++ 15 files changed, 89 insertions(+), 63 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index e1f126af0..dfee41bdf 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -154,7 +154,7 @@ def get_plugin_manager(): def _prepareconfig(args=None, plugins=None): - warning_msg = None + warning = None if args is None: args = sys.argv[1:] elif isinstance(args, py.path.local): @@ -165,7 +165,7 @@ def _prepareconfig(args=None, plugins=None): args = shlex.split(args, posix=sys.platform != "win32") from _pytest import deprecated - warning_msg = deprecated.MAIN_STR_ARGS + warning = deprecated.MAIN_STR_ARGS config = get_config() pluginmanager = config.pluginmanager try: @@ -175,11 +175,10 @@ def _prepareconfig(args=None, plugins=None): pluginmanager.consider_pluginarg(plugin) else: pluginmanager.register(plugin) - if warning_msg: - from _pytest.warning_types import PytestWarning + if warning: from _pytest.warnings import _issue_config_warning - _issue_config_warning(PytestWarning(warning_msg), config=config) + _issue_config_warning(warning, config=config) return pluginmanager.hook.pytest_cmdline_parse( pluginmanager=pluginmanager, args=args ) diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 82bf1d98d..d7a07503d 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -9,9 +9,14 @@ from __future__ import absolute_import, division, print_function from _pytest.warning_types import RemovedInPytest4Warning -MAIN_STR_ARGS = "passing a string to pytest.main() is deprecated, " "pass a list of arguments instead." +MAIN_STR_ARGS = RemovedInPytest4Warning( + "passing a string to pytest.main() is deprecated, " + "pass a list of arguments instead." +) -YIELD_TESTS = "yield tests are deprecated, and scheduled to be removed in pytest 4.0" +YIELD_TESTS = RemovedInPytest4Warning( + "yield tests are deprecated, and scheduled to be removed in pytest 4.0" +) FUNCARG_PREFIX = ( '{name}: declaring fixtures using "pytest_funcarg__" prefix is deprecated ' @@ -48,7 +53,11 @@ MARK_PARAMETERSET_UNPACKING = RemovedInPytest4Warning( "For more details, see: https://docs.pytest.org/en/latest/parametrize.html" ) -RECORD_XML_PROPERTY = ( +NODE_WARN = RemovedInPytest4Warning( + "Node.warn has been deprecated, use Node.std_warn instead" +) + +RECORD_XML_PROPERTY = RemovedInPytest4Warning( 'Fixture renamed from "record_xml_property" to "record_property" as user ' "properties are now available to all reporters.\n" '"record_xml_property" is now deprecated.' @@ -58,7 +67,7 @@ COLLECTOR_MAKEITEM = RemovedInPytest4Warning( "pycollector makeitem was removed " "as it is an accidentially leaked internal api" ) -METAFUNC_ADD_CALL = ( +METAFUNC_ADD_CALL = RemovedInPytest4Warning( "Metafunc.addcall is deprecated and scheduled to be removed in pytest 4.0.\n" "Please use Metafunc.parametrize instead." ) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 476acab02..cbfda9a82 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1257,6 +1257,8 @@ class FixtureManager(object): items[:] = reorder_items(items) def parsefactories(self, node_or_obj, nodeid=NOTSET, unittest=False): + from _pytest import deprecated + if nodeid is not NOTSET: holderobj = node_or_obj else: @@ -1279,7 +1281,6 @@ class FixtureManager(object): if not callable(obj): continue marker = defaultfuncargprefixmarker - from _pytest import deprecated filename, lineno = getfslineno(obj) warnings.warn_explicit( diff --git a/src/_pytest/junitxml.py b/src/_pytest/junitxml.py index 776cd935c..e2579860b 100644 --- a/src/_pytest/junitxml.py +++ b/src/_pytest/junitxml.py @@ -262,7 +262,7 @@ def record_xml_property(record_property, request): """(Deprecated) use record_property.""" from _pytest import deprecated - request.node.std_warn(deprecated.RECORD_XML_PROPERTY, DeprecationWarning) + request.node.std_warn(deprecated.RECORD_XML_PROPERTY) return record_property @@ -276,8 +276,7 @@ def record_xml_attribute(request): from _pytest.warning_types import PytestWarning request.node.std_warn( - message="record_xml_attribute is an experimental feature", - category=PytestWarning, + PytestWarning("record_xml_attribute is an experimental feature") ) xml = getattr(request.config, "_xml", None) if xml is not None: diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py index 0e0ba96e5..52a780ead 100644 --- a/src/_pytest/mark/structures.py +++ b/src/_pytest/mark/structures.py @@ -65,7 +65,7 @@ class ParameterSet(namedtuple("ParameterSet", "values, marks, id")): return cls(values, marks, id_) @classmethod - def extract_from(cls, parameterset, legacy_force_tuple=False, item=None): + def extract_from(cls, parameterset, belonging_definition, legacy_force_tuple=False): """ :param parameterset: a legacy style parameterset that may or may not be a tuple, @@ -75,7 +75,7 @@ class ParameterSet(namedtuple("ParameterSet", "values, marks, id")): enforce tuple wrapping so single argument tuple values don't get decomposed and break tests - :param item: the item that we will be extracting the parameters from. + :param belonging_definition: the item that we will be extracting the parameters from. """ if isinstance(parameterset, cls): @@ -94,8 +94,8 @@ class ParameterSet(namedtuple("ParameterSet", "values, marks, id")): if legacy_force_tuple: argval = (argval,) - if newmarks and item is not None: - item.std_warn(MARK_PARAMETERSET_UNPACKING) + if newmarks and belonging_definition is not None: + belonging_definition.std_warn(MARK_PARAMETERSET_UNPACKING) return cls(argval, marks=newmarks, id=None) @@ -108,7 +108,9 @@ class ParameterSet(namedtuple("ParameterSet", "values, marks, id")): force_tuple = False parameters = [ ParameterSet.extract_from( - x, legacy_force_tuple=force_tuple, item=function_definition + x, + legacy_force_tuple=force_tuple, + belonging_definition=function_definition, ) for x in argvalues ] diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index e9ee74b1a..7a2b48ce2 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -144,12 +144,9 @@ class Node(object): Generate a warning with the given code and message for this item. """ - from _pytest.warning_types import RemovedInPytest4Warning + from _pytest.deprecated import NODE_WARN - self.std_warn( - "Node.warn has been deprecated, use Node.std_warn instead", - RemovedInPytest4Warning, - ) + self.std_warn(NODE_WARN) assert isinstance(code, str) fslocation = get_fslocation_from_item(self) @@ -159,22 +156,27 @@ class Node(object): ) ) - def std_warn(self, message, category=None): + def std_warn(self, warning): """Issue a warning for this item. Warnings will be displayed after the test session, unless explicitly suppressed - :param Union[str,Warning] message: text message of the warning or ``Warning`` instance. - :param Type[Warning] category: warning category. + :param Warning warning: the warning instance to issue. Must be a subclass of PytestWarning. + + :raise ValueError: if ``warning`` instance is not a subclass of PytestWarning. """ from _pytest.warning_types import PytestWarning - if category is None: - assert isinstance(message, PytestWarning) + if not isinstance(warning, PytestWarning): + raise ValueError( + "warning must be an instance of PytestWarning or subclass, got {!r}".format( + warning + ) + ) path, lineno = get_fslocation_from_item(self) warnings.warn_explicit( - message, - category, + six.text_type(warning), + type(warning), filename=str(path), lineno=lineno + 1 if lineno is not None else None, ) diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index 4140dfc50..62127651a 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -126,7 +126,7 @@ class LsofFdLeakChecker(object): error.append(error[0]) error.append("*** function %s:%s: %s " % item.location) error.append("See issue #2366") - item.std_warn("", "\n".join(error), pytest.PytestWarning) + item.std_warn(pytest.PytestWarning("\n".join(error))) # XXX copied from execnet's conftest.py - needs to be merged @@ -643,11 +643,9 @@ class Testdir(object): def copy_example(self, name=None): import warnings + from _pytest.warning_types import PYTESTER_COPY_EXAMPLE - warnings.warn( - pytest.PytestExperimentalApiWarning.simple("testdir.copy_example"), - stacklevel=2, - ) + warnings.warn(PYTESTER_COPY_EXAMPLE, stacklevel=2) example_dir = self.request.config.getini("pytester_example_dir") if example_dir is None: raise ValueError("pytester_example_dir is unset, can't copy examples") diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 0fb7fb732..fa14b7a33 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -660,9 +660,10 @@ class Class(PyCollector): return [] if hasinit(self.obj): self.std_warn( - "cannot collect test class %r because it has a " - "__init__ constructor" % self.obj.__name__, - PytestWarning, + PytestWarning( + "cannot collect test class %r because it has a " + "__init__ constructor" % self.obj.__name__ + ) ) return [] elif hasnew(self.obj): @@ -798,7 +799,7 @@ class Generator(FunctionMixin, PyCollector): ) seen[name] = True values.append(self.Function(name, self, args=args, callobj=call)) - self.std_warn(deprecated.YIELD_TESTS, RemovedInPytest4Warning) + self.std_warn(deprecated.YIELD_TESTS) return values def getcallargs(self, obj): @@ -1105,9 +1106,7 @@ class Metafunc(fixtures.FuncargnamesCompatAttr): invocation through the ``request.param`` attribute. """ if self.config: - self.definition.std_warn( - deprecated.METAFUNC_ADD_CALL, RemovedInPytest4Warning - ) + self.definition.std_warn(deprecated.METAFUNC_ADD_CALL) assert funcargs is None or isinstance(funcargs, dict) if funcargs is not None: @@ -1158,22 +1157,20 @@ def _find_parametrized_scope(argnames, arg2fixturedefs, indirect): return "function" -def _idval(val, argname, idx, idfn, config=None, item=None): +def _idval(val, argname, idx, idfn, item, config=None): if idfn: s = None try: s = idfn(val) except Exception as e: # See issue https://github.com/pytest-dev/pytest/issues/2169 - if item is not None: - # should really be None only when unit-testing this function! - msg = ( - "While trying to determine id of parameter {} at position " - "{} the following exception was raised:\n".format(argname, idx) - ) - msg += " {}: {}\n".format(type(e).__name__, e) - msg += "This warning will be an error error in pytest-4.0." - item.std_warn(msg, RemovedInPytest4Warning) + msg = ( + "While trying to determine id of parameter {} at position " + "{} the following exception was raised:\n".format(argname, idx) + ) + msg += " {}: {}\n".format(type(e).__name__, e) + msg += "This warning will be an error error in pytest-4.0." + item.std_warn(RemovedInPytest4Warning(msg)) if s: return ascii_escaped(s) @@ -1202,7 +1199,7 @@ def _idvalset(idx, parameterset, argnames, idfn, ids, config=None, item=None): return parameterset.id if ids is None or (idx >= len(ids) or ids[idx] is None): this_id = [ - _idval(val, argname, idx, idfn, config, item) + _idval(val, argname, idx, idfn, item=item, config=config) for val, argname in zip(parameterset.values, argnames) ] return "-".join(this_id) diff --git a/src/_pytest/warning_types.py b/src/_pytest/warning_types.py index 4422363b1..8861f6f2b 100644 --- a/src/_pytest/warning_types.py +++ b/src/_pytest/warning_types.py @@ -37,3 +37,6 @@ class PytestExperimentalApiWarning(PytestWarning, FutureWarning): apiname=apiname ) ) + + +PYTESTER_COPY_EXAMPLE = PytestExperimentalApiWarning.simple("testdir.copy_example") diff --git a/src/_pytest/warnings.py b/src/_pytest/warnings.py index 0b67bd8f1..2d73def0f 100644 --- a/src/_pytest/warnings.py +++ b/src/_pytest/warnings.py @@ -129,20 +129,20 @@ def warning_record_to_str(warning_message): return msg -@pytest.hookimpl(hookwrapper=True) +@pytest.hookimpl(hookwrapper=True, tryfirst=True) def pytest_runtest_protocol(item): with catch_warnings_for_item(config=item.config, ihook=item.ihook, item=item): yield -@pytest.hookimpl(hookwrapper=True) +@pytest.hookimpl(hookwrapper=True, tryfirst=True) def pytest_collection(session): config = session.config with catch_warnings_for_item(config=config, ihook=config.hook, item=None): yield -@pytest.hookimpl(hookwrapper=True) +@pytest.hookimpl(hookwrapper=True, tryfirst=True) def pytest_terminal_summary(terminalreporter): config = terminalreporter.config with catch_warnings_for_item(config=config, ihook=config.hook, item=None): diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 6b374083f..428ac464c 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -526,7 +526,7 @@ class TestInvocationVariants(object): assert pytest.main == py.test.cmdline.main def test_invoke_with_string(self, capsys): - retcode = pytest.main(["-h"]) + retcode = pytest.main("-h") assert not retcode out, err = capsys.readouterr() assert "--help" in out diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index ec53bf7eb..a74ce662d 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -289,6 +289,7 @@ def test_call_fixture_function_deprecated(): def test_pycollector_makeitem_is_deprecated(): from _pytest.python import PyCollector + from _pytest.warning_types import RemovedInPytest4Warning class PyCollectorMock(PyCollector): """evil hack""" @@ -301,6 +302,6 @@ def test_pycollector_makeitem_is_deprecated(): self.called = True collector = PyCollectorMock() - with pytest.deprecated_call(): + with pytest.warns(RemovedInPytest4Warning): collector.makeitem("foo", "bar") assert collector.called diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 5d9282435..e1dfb6d8b 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -217,7 +217,7 @@ class TestMetafunc(object): def test_idval_hypothesis(self, value): from _pytest.python import _idval - escaped = _idval(value, "a", 6, None) + escaped = _idval(value, "a", 6, None, item=None) assert isinstance(escaped, str) if PY3: escaped.encode("ascii") @@ -244,7 +244,7 @@ class TestMetafunc(object): ), ] for val, expected in values: - assert _idval(val, "a", 6, None) == expected + assert _idval(val, "a", 6, None, item=None) == expected def test_bytes_idval(self): """unittest for the expected behavior to obtain ids for parametrized @@ -262,7 +262,7 @@ class TestMetafunc(object): (u"αρά".encode("utf-8"), "\\xce\\xb1\\xcf\\x81\\xce\\xac"), ] for val, expected in values: - assert _idval(val, "a", 6, None) == expected + assert _idval(val, "a", 6, idfn=None, item=None, config=None) == expected def test_class_or_function_idval(self): """unittest for the expected behavior to obtain ids for parametrized @@ -278,7 +278,7 @@ class TestMetafunc(object): values = [(TestClass, "TestClass"), (test_function, "test_function")] for val, expected in values: - assert _idval(val, "a", 6, None) == expected + assert _idval(val, "a", 6, None, item=None) == expected @pytest.mark.issue250 def test_idmaker_autoname(self): diff --git a/testing/test_mark.py b/testing/test_mark.py index 12aded416..0a6567521 100644 --- a/testing/test_mark.py +++ b/testing/test_mark.py @@ -1041,7 +1041,11 @@ class TestKeywordSelection(object): ) @pytest.mark.filterwarnings("ignore") def test_parameterset_extractfrom(argval, expected): - extracted = ParameterSet.extract_from(argval) + class DummyItem: + def std_warn(self, warning): + pass + + extracted = ParameterSet.extract_from(argval, belonging_definition=DummyItem()) assert extracted == expected diff --git a/testing/test_nodes.py b/testing/test_nodes.py index eee3ac8e9..d62d7d78a 100644 --- a/testing/test_nodes.py +++ b/testing/test_nodes.py @@ -19,3 +19,14 @@ from _pytest import nodes def test_ischildnode(baseid, nodeid, expected): result = nodes.ischildnode(baseid, nodeid) assert result is expected + + +def test_std_warn_not_pytestwarning(testdir): + items = testdir.getitems( + """ + def test(): + pass + """ + ) + with pytest.raises(ValueError, match=".*instance of PytestWarning.*"): + items[0].std_warn(UserWarning("some warning"))