pytest: Test functions that return non-None should raise a warning/error

Consider this test:

# The function we're testing
def foo(a: int, b: int) -> int:
  return a * 3 + b

@pytest.mark.parametrize(['a', 'b', 'result'], [
  [1, 2, 5],
  [2, 3, 8],
  [5, 3, 18],
])
def test_foo(a, b, result):
  return foo(a, b) == result

Did you spot the error? The second parametrization has a typo, 2 * 3 + 3 is 9, not 8. But this test actually doesn’t test anything, because it returns the assertion rather than asserting it. This is a common enough mistake, and it wouldn’t normally be a problem except that it can silently cause false positives in test suites.

I propose that test functions that return anything except None fail with a message that cues users that they probably meant to assert rather than return. This feature could be disabled (or enabled, if there are backwards-compatibility issues) via a config flag if necessary.

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 1
Comments: 20 (18 by maintainers)

Links to this issue

Update pytest requirement from ~=7.1.3 to ~=7.2.0 (!31) · Merge requests · Claudius Kastner Kastner / tgenv · GitLab

Commits related to this issue

Warn when test functions return other than None (#9956) Closes #7337 — committed to pytest-dev/pytest by Cheukting 2 years ago
chore(deps): update dependency pytest to v7 (#3) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [pytest](https://docs.pytest.org/en/latest/) ([sourc... — committed to jaypikay/doxy by deleted user a year ago

Most upvoted comments

interestingly enough, unittest in cpython now warns in this situation: https://github.com/python/cpython/pull/27748

asottile on May 9, 2022

my note is about refactoring to common functions instead of running other test functions

RonnyPfannschmidt on Jun 10, 2020

I am guilty of such testsuites myself and I think it’s warranted to extract operational components to function

RonnyPfannschmidt on Jun 10, 2020

I’m a little hesitant about this one, I’ve seen a few testsuites where there’s like

def test_one():
    # do some stuff
    # assert some stuff
    return some_info()

def test_two():
    res = test_one()
    # do more things
    # assert more things

I’m not saying they’re good tests, but forbidding return will break them (the test works fine and returning something isn’t breaking its correctness)

asottile on Jun 10, 2020

I’m +1 on this - in fact Hypothesis already makes non-None returns from wrapped tests into errors.

it catches some trivial user errors, as noted in the OP
more importantly, it also catches problems like definining generators as tests (or async test functions), where calling the function doesn’t actually execute the test body. On this basis we could e.g. explicitly recommend pytest-asyncio or pytest-trio if an awaitable object is returned.

I’d probably do this as a warning to start with, have an option to disable the warning, and then upgrade it to an error (still disabled by the same option) in pytest 6.0

Zac-HD on Jun 11, 2020

Indeed, my mistake, thanks for clarifying.

I’m still not convinced that this is common enough to warrant an error/warning however, but I’m changing my vote to -0.5 now that the proposal is clearer to me.

If others provide more use cases or situations where this error is common then I’m not against adding a warning for tests which return non-None. 😁

nicoddemus on Jun 9, 2020