Skip to content

Deprecate raises(..., 'code(as_a_string)') / warns(..., 'code(as_a_string)'). #4443

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 1 commit into from
Nov 29, 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
1 change: 1 addition & 0 deletions changelog/4435.deprecation.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecate ``raises(..., 'code(as_a_string)')`` and ``warns(..., 'code(as_a_string)')``. See https://docs.pytest.org/en/latest/deprecations.html#raises-warns-exec
5 changes: 2 additions & 3 deletions doc/en/assert.rst
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,9 @@ If you want to write test code that works on Python 2.4 as well,
you may also use two other ways to test for an expected exception::

pytest.raises(ExpectedException, func, *args, **kwargs)
pytest.raises(ExpectedException, "func(*args, **kwargs)")

both of which execute the specified function with args and kwargs and
asserts that the given ``ExpectedException`` is raised. The reporter will
which will execute the specified function with args and kwargs and
assert that the given ``ExpectedException`` is raised. The reporter will
provide you with helpful output in case of failures such as *no
exception* or *wrong exception*.

Expand Down
35 changes: 35 additions & 0 deletions doc/en/deprecations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,41 @@ Below is a complete list of all pytest features which are considered deprecated.
:class:`_pytest.warning_types.PytestWarning` or subclasses, which can be filtered using
:ref:`standard warning filters <warnings>`.

.. _raises-warns-exec:

``raises`` / ``warns`` with a string as the second argument
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. deprecated:: 4.1

Use the context manager form of these instead. When necessary, invoke ``exec``
directly.

Example:

.. code-block:: python

pytest.raises(ZeroDivisionError, "1 / 0")
pytest.raises(SyntaxError, "a $ b")

pytest.warns(DeprecationWarning, "my_function()")
pytest.warns(SyntaxWarning, "assert(1, 2)")

Becomes:

.. code-block:: python

with pytest.raises(ZeroDivisionError):
1 / 0
with pytest.raises(SyntaxError):
exec("a $ b") # exec is required for invalid syntax

with pytest.warns(DeprecationWarning):
my_function()
with pytest.warns(SyntaxWarning):
exec("assert(1, 2)") # exec is used to avoid a top-level warning


Internal classes accessed through ``Node``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
6 changes: 3 additions & 3 deletions doc/en/example/assertion/failure_demo.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,11 @@ def globf(x):

class TestRaises(object):
def test_raises(self):
s = "qwe" # NOQA
raises(TypeError, "int(s)")
s = "qwe"
raises(TypeError, int, s)

def test_raises_doesnt(self):
raises(IOError, "int('3')")
raises(IOError, int, "3")

def test_raise(self):
raise ValueError("demo error")
Expand Down
3 changes: 2 additions & 1 deletion doc/en/example/parametrize.rst
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,8 @@ parametrizer`_ but in a lot less code::
assert a == b

def test_zerodivision(self, a, b):
pytest.raises(ZeroDivisionError, "a/b")
with pytest.raises(ZeroDivisionError):
a / b

Our test generator looks up a class-level definition which specifies which
argument sets to use for each test function. Let's run it:
Expand Down
9 changes: 9 additions & 0 deletions src/_pytest/deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,15 @@
"Node.warn(code, message) form has been deprecated, use Node.warn(warning_instance) instead."
)

RAISES_EXEC = PytestDeprecationWarning(
"raises(..., 'code(as_a_string)') is deprecated, use the context manager form or use `exec()` directly\n\n"
"See https://docs.pytest.org/en/latest/deprecations.html#raises-warns-exec"
)
WARNS_EXEC = PytestDeprecationWarning(
"warns(..., 'code(as_a_string)') is deprecated, use the context manager form or use `exec()` directly.\n\n"
"See https://docs.pytest.org/en/latest/deprecations.html#raises-warns-exec"
)

RECORD_XML_PROPERTY = RemovedInPytest4Warning(
'Fixture renamed from "record_xml_property" to "record_property" as user '
"properties are now available to all reporters.\n"
Expand Down
17 changes: 7 additions & 10 deletions src/_pytest/python_api.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
from __future__ import absolute_import

import math
import pprint
import sys
import warnings
from decimal import Decimal
from numbers import Number

Expand All @@ -14,6 +17,7 @@
from _pytest.compat import Mapping
from _pytest.compat import Sequence
from _pytest.compat import STRING_TYPES
from _pytest.deprecated import RAISES_EXEC
from _pytest.outcomes import fail

BASE_TYPE = (type, STRING_TYPES)
Expand Down Expand Up @@ -604,9 +608,9 @@ def raises(expected_exception, *args, **kwargs):
>>> with raises(ValueError, match=r'must be \d+$'):
... raise ValueError("value must be 42")

**Legacy forms**
**Legacy form**

The forms below are fully supported but are discouraged for new code because the
The form below is fully supported but discouraged for new code because the
context manager form is regarded as more readable and less error-prone.

It is possible to specify a callable by passing a to-be-called lambda::
Expand All @@ -623,14 +627,6 @@ def raises(expected_exception, *args, **kwargs):
>>> raises(ZeroDivisionError, f, x=0)
<ExceptionInfo ...>

It is also possible to pass a string to be evaluated at runtime::

>>> raises(ZeroDivisionError, "f(0)")
<ExceptionInfo ...>

The string will be evaluated using the same ``locals()`` and ``globals()``
at the moment of the ``raises`` call.

.. currentmodule:: _pytest._code

Consult the API of ``excinfo`` objects: :class:`ExceptionInfo`.
Expand Down Expand Up @@ -672,6 +668,7 @@ def raises(expected_exception, *args, **kwargs):
raise TypeError(msg)
return RaisesContext(expected_exception, message, match_expr)
elif isinstance(args[0], str):
warnings.warn(RAISES_EXEC, stacklevel=2)
code, = args
assert isinstance(code, str)
frame = sys._getframe(1)
Expand Down
2 changes: 2 additions & 0 deletions src/_pytest/recwarn.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import six

import _pytest._code
from _pytest.deprecated import WARNS_EXEC
from _pytest.fixtures import yield_fixture
from _pytest.outcomes import fail

Expand Down Expand Up @@ -89,6 +90,7 @@ def warns(expected_warning, *args, **kwargs):
match_expr = kwargs.pop("match")
return WarningsChecker(expected_warning, match_expr=match_expr)
elif isinstance(args[0], str):
warnings.warn(WARNS_EXEC, stacklevel=2)
code, = args
assert isinstance(code, str)
frame = sys._getframe(1)
Expand Down
2 changes: 1 addition & 1 deletion testing/code/test_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_code_with_class():
class A(object):
pass

pytest.raises(TypeError, "_pytest._code.Code(A)")
pytest.raises(TypeError, _pytest._code.Code, A)


def x():
Expand Down
6 changes: 4 additions & 2 deletions testing/code/test_excinfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ def test_traceback_cut(self):

def test_traceback_cut_excludepath(self, testdir):
p = testdir.makepyfile("def f(): raise ValueError")
excinfo = pytest.raises(ValueError, "p.pyimport().f()")
with pytest.raises(ValueError) as excinfo:
p.pyimport().f()
basedir = py.path.local(pytest.__file__).dirpath()
newtraceback = excinfo.traceback.cut(excludepath=basedir)
for x in newtraceback:
Expand Down Expand Up @@ -336,7 +337,8 @@ def f():
def test_excinfo_exconly():
excinfo = pytest.raises(ValueError, h)
assert excinfo.exconly().startswith("ValueError")
excinfo = pytest.raises(ValueError, "raise ValueError('hello\\nworld')")
with pytest.raises(ValueError) as excinfo:
raise ValueError("hello\nworld")
msg = excinfo.exconly(tryshort=True)
assert msg.startswith("ValueError")
assert msg.endswith("world")
Expand Down
25 changes: 9 additions & 16 deletions testing/code/test_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from __future__ import division
from __future__ import print_function

import ast
import inspect
import sys

Expand All @@ -14,7 +15,6 @@
import _pytest._code
import pytest
from _pytest._code import Source
from _pytest._code.source import ast


astonly = pytest.mark.nothing
Expand Down Expand Up @@ -306,8 +306,6 @@ def test_getstatementrange_with_syntaxerror_issue7(self):
pytest.raises(SyntaxError, lambda: source.getstatementrange(0))

def test_compile_to_ast(self):
import ast

source = Source("x = 4")
mod = source.compile(flag=ast.PyCF_ONLY_AST)
assert isinstance(mod, ast.Module)
Expand All @@ -317,10 +315,9 @@ def test_compile_and_getsource(self):
co = self.source.compile()
six.exec_(co, globals())
f(7)
excinfo = pytest.raises(AssertionError, "f(6)")
excinfo = pytest.raises(AssertionError, f, 6)
frame = excinfo.traceback[-1].frame
stmt = frame.code.fullsource.getstatement(frame.lineno)
# print "block", str(block)
assert str(stmt).strip().startswith("assert")

@pytest.mark.parametrize("name", ["", None, "my"])
Expand Down Expand Up @@ -361,17 +358,13 @@ def test_getline_finally():
def c():
pass

excinfo = pytest.raises(
TypeError,
"""
teardown = None
try:
c(1)
finally:
if teardown:
teardown()
""",
)
with pytest.raises(TypeError) as excinfo:
teardown = None
try:
c(1)
finally:
if teardown:
teardown()
source = excinfo.traceback[-1].statement
assert str(source).strip() == "c(1)"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@

@pytest.fixture
def arg2(request):
pytest.raises(Exception, "request.getfixturevalue('arg1')")
pytest.raises(Exception, request.getfixturevalue, "arg1")
5 changes: 3 additions & 2 deletions testing/python/collect.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ def test_gen():
assert len(colitems) == 1
gencol = colitems[0]
assert isinstance(gencol, pytest.Generator)
pytest.raises(ValueError, "gencol.collect()")
pytest.raises(ValueError, gencol.collect)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to highlight the importance of the call visually i'd propose a with statement in that case

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's ok, I'm going to leave these as is so it's a less noisy diff -- we can clean these up in a followup


def test_generative_methods_with_explicit_names(self, testdir):
modcol = testdir.getmodulecol(
Expand Down Expand Up @@ -1103,7 +1103,8 @@ def test_modulecol_roundtrip(testdir):

class TestTracebackCutting(object):
def test_skip_simple(self):
excinfo = pytest.raises(pytest.skip.Exception, 'pytest.skip("xxx")')
with pytest.raises(pytest.skip.Exception) as excinfo:
pytest.skip("xxx")
assert excinfo.traceback[-1].frame.code.name == "skip"
assert excinfo.traceback[-1].ishidden()

Expand Down
3 changes: 2 additions & 1 deletion testing/python/fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,8 @@ def test_func2(self, something):
assert "skipif" not in item1.keywords
req1.applymarker(pytest.mark.skipif)
assert "skipif" in item1.keywords
pytest.raises(ValueError, "req1.applymarker(42)")
with pytest.raises(ValueError):
req1.applymarker(42)

def test_accesskeywords(self, testdir):
testdir.makepyfile(
Expand Down
8 changes: 4 additions & 4 deletions testing/python/metafunc.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ def func(arg1):
pass

metafunc = self.Metafunc(func)
pytest.raises(ValueError, "metafunc.addcall(id=None)")
pytest.raises(ValueError, metafunc.addcall, id=None)

metafunc.addcall(id=1)
pytest.raises(ValueError, "metafunc.addcall(id=1)")
pytest.raises(ValueError, "metafunc.addcall(id='1')")
pytest.raises(ValueError, metafunc.addcall, id=1)
pytest.raises(ValueError, metafunc.addcall, id="1")
metafunc.addcall(id=2)
assert len(metafunc._calls) == 2
assert metafunc._calls[0].id == "1"
Expand Down Expand Up @@ -108,7 +108,7 @@ class obj(object):

metafunc.addcall(funcargs={"x": 2})
metafunc.addcall(funcargs={"x": 3})
pytest.raises(pytest.fail.Exception, "metafunc.addcall({'xyz': 0})")
pytest.raises(pytest.fail.Exception, metafunc.addcall, {"xyz": 0})
assert len(metafunc._calls) == 2
assert metafunc._calls[0].funcargs == {"x": 2}
assert metafunc._calls[1].funcargs == {"x": 3}
Expand Down
17 changes: 12 additions & 5 deletions testing/python/raises.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,32 @@

import pytest
from _pytest.outcomes import Failed
from _pytest.warning_types import PytestDeprecationWarning


class TestRaises(object):
def test_raises(self):
source = "int('qwe')"
excinfo = pytest.raises(ValueError, source)
with pytest.warns(PytestDeprecationWarning):
excinfo = pytest.raises(ValueError, source)
code = excinfo.traceback[-1].frame.code
s = str(code.fullsource)
assert s == source

def test_raises_exec(self):
pytest.raises(ValueError, "a,x = []")
with pytest.warns(PytestDeprecationWarning) as warninfo:
pytest.raises(ValueError, "a,x = []")
assert warninfo[0].filename == __file__

def test_raises_exec_correct_filename(self):
excinfo = pytest.raises(ValueError, 'int("s")')
assert __file__ in excinfo.traceback[-1].path
with pytest.warns(PytestDeprecationWarning):
excinfo = pytest.raises(ValueError, 'int("s")')
assert __file__ in excinfo.traceback[-1].path

def test_raises_syntax_error(self):
pytest.raises(SyntaxError, "qwe qwe qwe")
with pytest.warns(PytestDeprecationWarning) as warninfo:
pytest.raises(SyntaxError, "qwe qwe qwe")
assert warninfo[0].filename == __file__

def test_raises_function(self):
pytest.raises(ValueError, int, "hello")
Expand Down
10 changes: 5 additions & 5 deletions testing/test_capture.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def test_init_capturing(self):
try:
capman = CaptureManager("fd")
capman.start_global_capturing()
pytest.raises(AssertionError, "capman.start_global_capturing()")
pytest.raises(AssertionError, capman.start_global_capturing)
capman.stop_global_capturing()
finally:
capouter.stop_capturing()
Expand Down Expand Up @@ -798,10 +798,10 @@ def test_unicode_and_str_mixture(self):
f = capture.CaptureIO()
if sys.version_info >= (3, 0):
f.write("\u00f6")
pytest.raises(TypeError, "f.write(bytes('hello', 'UTF-8'))")
pytest.raises(TypeError, f.write, b"hello")
else:
f.write(text_type("\u00f6", "UTF-8"))
f.write("hello") # bytes
f.write(u"\u00f6")
f.write(b"hello")
s = f.getvalue()
f.close()
assert isinstance(s, text_type)
Expand Down Expand Up @@ -1149,7 +1149,7 @@ def test_stdin_nulled_by_default(self):
print("XXX which indicates an error in the underlying capturing")
print("XXX mechanisms")
with self.getcapture():
pytest.raises(IOError, "sys.stdin.read()")
pytest.raises(IOError, sys.stdin.read)


class TestStdCaptureFD(TestStdCapture):
Expand Down
Loading