From c17027e5767bda8ac8237be4deea03a7fd898415 Mon Sep 17 00:00:00 2001 From: Vasily Kuznetsov Date: Sat, 25 Jun 2016 18:45:47 +0200 Subject: [PATCH 1/3] Warn about asserts on tuples (fixes #1562) --- _pytest/assertion/rewrite.py | 20 +++++++++++++++----- testing/test_assertion.py | 8 ++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/_pytest/assertion/rewrite.py b/_pytest/assertion/rewrite.py index fd4f66cd0d5..921d17a1018 100644 --- a/_pytest/assertion/rewrite.py +++ b/_pytest/assertion/rewrite.py @@ -125,7 +125,7 @@ def find_module(self, name, path=None): co = _read_pyc(fn_pypath, pyc, state.trace) if co is None: state.trace("rewriting %r" % (fn,)) - source_stat, co = _rewrite_test(state, fn_pypath) + source_stat, co = _rewrite_test(self.config, fn_pypath) if co is None: # Probably a SyntaxError in the test. return None @@ -252,8 +252,9 @@ def _write_pyc(state, co, source_stat, pyc): cookie_re = re.compile(r"^[ \t\f]*#.*coding[:=][ \t]*[-\w.]+") BOM_UTF8 = '\xef\xbb\xbf' -def _rewrite_test(state, fn): +def _rewrite_test(config, fn): """Try to read and rewrite *fn* and return the code object.""" + state = config._assertstate try: stat = fn.stat() source = fn.read("rb") @@ -298,7 +299,7 @@ def _rewrite_test(state, fn): # Let this pop up again in the real import. state.trace("failed to parse: %r" % (fn,)) return None, None - rewrite_asserts(tree) + rewrite_asserts(tree, fn, config) try: co = compile(tree, fn.strpath, "exec") except SyntaxError: @@ -354,9 +355,9 @@ def _read_pyc(source, pyc, trace=lambda x: None): return co -def rewrite_asserts(mod): +def rewrite_asserts(mod, module_path=None, config=None): """Rewrite the assert statements in mod.""" - AssertionRewriter().run(mod) + AssertionRewriter(module_path, config).run(mod) def _saferepr(obj): @@ -543,6 +544,11 @@ class AssertionRewriter(ast.NodeVisitor): """ + def __init__(self, module_path, config): + super(AssertionRewriter, self).__init__() + self.module_path = module_path + self.config = config + def run(self, mod): """Find all assert statements in *mod* and rewrite them.""" if not mod.body: @@ -683,6 +689,10 @@ def visit_Assert(self, assert_): the expression is false. """ + if isinstance(assert_.test, ast.Tuple) and self.config is not None: + fslocation = (self.module_path, assert_.lineno) + self.config.warn('R1', 'assertion is always true, perhaps ' + 'remove parentheses?', fslocation=fslocation) self.statements = [] self.variables = [] self.variable_counter = itertools.count() diff --git a/testing/test_assertion.py b/testing/test_assertion.py index dfa1b942034..4694bb26134 100644 --- a/testing/test_assertion.py +++ b/testing/test_assertion.py @@ -640,3 +640,11 @@ def test_diff(): * + asdf * ? + """) + +def test_assert_tuple_warning(testdir): + testdir.makepyfile(""" + def test_tuple(): + assert(False, 'you shall not pass') + """) + result = testdir.runpytest('-rw') + result.stdout.fnmatch_lines('WR1*:2 assertion is always true*') From 0db4ae15a95f76b9f71848140318a4a6613e39d9 Mon Sep 17 00:00:00 2001 From: Vasily Kuznetsov Date: Sat, 25 Jun 2016 19:34:55 +0200 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index faaa6537498..a5338fc9cb3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -79,6 +79,10 @@ finalizer and has access to the fixture's result cache. Thanks `@d6e`_, `@sallner`_ +* Issue a warning for asserts whose test is a tuple literal. Such asserts will + never fail because tuples are always truthy and are usually a mistake + (see `#1562`_). Thanks `@kvas-it`_, for the PR. + * New cli flag ``--override-ini`` or ``-o`` that overrides values from the ini file. Example '-o xfail_strict=True'. A complete ini-options can be viewed by py.test --help. Thanks `@blueyed`_ and `@fengxx`_ for the PR @@ -207,6 +211,7 @@ .. _#1619: https://github.com/pytest-dev/pytest/issues/1619 .. _#372: https://github.com/pytest-dev/pytest/issues/372 .. _#1544: https://github.com/pytest-dev/pytest/issues/1544 +.. _#1562: https://github.com/pytest-dev/pytest/issues/1562 .. _#1616: https://github.com/pytest-dev/pytest/pull/1616 .. _#1628: https://github.com/pytest-dev/pytest/pull/1628 .. _#1629: https://github.com/pytest-dev/pytest/issues/1629 From 6d4cee21598f35100b46c5d892a8b3af99ec8087 Mon Sep 17 00:00:00 2001 From: Vasily Kuznetsov Date: Sun, 26 Jun 2016 02:21:51 +0200 Subject: [PATCH 3/3] Add a test for indirect use of tuple in the assert that should not cause a warning --- testing/test_assertion.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/testing/test_assertion.py b/testing/test_assertion.py index 4694bb26134..60b1fb02be4 100644 --- a/testing/test_assertion.py +++ b/testing/test_assertion.py @@ -648,3 +648,13 @@ def test_tuple(): """) result = testdir.runpytest('-rw') result.stdout.fnmatch_lines('WR1*:2 assertion is always true*') + +def test_assert_indirect_tuple_no_warning(testdir): + testdir.makepyfile(""" + def test_tuple(): + tpl = ('foo', 'bar') + assert tpl + """) + result = testdir.runpytest('-rw') + output = '\n'.join(result.stdout.lines) + assert 'WR1' not in output