classification
Title: unittest: deprecate test methods returning non-None values
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: defreng, ezio.melotti, michael.foord, rbcollins, serhiy.storchaka, terry.reedy, xtreak, zach.ware
Priority: normal Keywords:

Created on 2020-07-17 09:46 by defreng, last changed 2020-07-18 06:40 by serhiy.storchaka.

Messages (8)
msg373807 - (view) Author: Alexander Hungenberg (defreng) Date: 2020-07-17 09:46
The following testcase will always be marked as passed:

from unittest import TestCase

class BuggyTestCase(TestCase):
    def test_generator(self):
        self.assertTrue(False)
        yield None


It happened to us that someone accidentally made the test method a generator function. That error was caught very late, because it always appears to have executed correctly.

Maybe an additional check can be introduced in the unittest module, to check if a test_ method was executed correctly?
msg373810 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2020-07-17 10:17
https://github.com/python/cpython/blob/8e836bb21ce73f0794fd769db5883c29680dfe47/Lib/unittest/case.py#L548 . _callTestMethod just calls the test method and doesn't check for the method to be a generator function to be iterated through. In Python 3.8 the call to test method was separated out as _callTestMethod. So something like below added in the original method should work. I guess there is an existing issue for this.


import unittest
from unittest import TestCase

class BuggyTestCase(TestCase):

    def _callTestMethod(self, method):
        import inspect

        if inspect.isgeneratorfunction(method):
            list(method())
        else:
            method()

    def test_generator(self):
        self.assertTrue(False)
        yield None

if __name__ == "__main__":
    unittest.main()


python3.8 test_foo.py
F
======================================================================
FAIL: test_generator (__main__.BuggyTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_foo.py", line 10, in _callTestMethod
    list(method())
  File "test_foo.py", line 15, in test_generator
    self.assertTrue(False)
AssertionError: False is not true

----------------------------------------------------------------------
Ran 1 test in 0.000s

FAILED (failures=1)
msg373812 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2020-07-17 10:43
This is a duplicate of issue15551 and as per discussion in the thread my approach is incoherent with design of unittest to execute generators. I propose closing this as duplicate with same resolution as not fixed unless there is a change required here.
msg373816 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-07-17 10:54
I would raser raise error if the test method returns something suspicious, like generator or coroutine. Or maybe if it returns anything except None.
msg373824 - (view) Author: Alexander Hungenberg (defreng) Date: 2020-07-17 11:20
I would also strongly vote for raising an error if the test method is a generator - not even silently turn it into a list.

It would be straight forward to implement various checks (like the ones you mentioned for generators, coroutines, ...) - and they will absolutely help to prevent unintended errors, especially by more junior developers.
msg373850 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2020-07-17 19:23
In the same vein as Serhiy's suggestion, I would like to see unittest raise a DeprecationWarning when a test method returns anything other than `None`, and eventually switch that to an error (probably TypeError or ValueError).
msg373874 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2020-07-18 06:29
The current behavior is not a bug as it follows documented behavior.  In the absence of any use for returning values from test functions, I agree that we should do the simple thing and raise ValueError (I checked and did not find anything like a UnittestError.) That would add minimal time to testing.
msg373876 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-07-18 06:40
It is also a good idea for linters to catch such kind of errors. It will help users of older Python versions.

We cannot raise error without deprecation period or add warnings in old versions because it potentially can break existing code, e.g.:

    def test_ham(self, arg='ham'):
        ...

    def test_spam(self):
        res = test_ham('spam')
        self.assertTrue(res)
History
Date User Action Args
2020-07-18 06:40:36serhiy.storchakasetmessages: + msg373876
2020-07-18 06:29:19terry.reedysetnosy: + terry.reedy
title: unittest: Generator test methods will always be marked as passed -> unittest: deprecate test methods returning non-None values
messages: + msg373874

versions: + Python 3.10, - Python 3.8
type: behavior -> enhancement
2020-07-17 19:23:35zach.waresetnosy: + zach.ware
messages: + msg373850
2020-07-17 11:20:58defrengsetmessages: + msg373824
2020-07-17 10:54:03serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg373816
2020-07-17 10:43:52xtreaksetmessages: + msg373812
2020-07-17 10:17:58xtreaksetnosy: + ezio.melotti, xtreak, michael.foord, rbcollins
messages: + msg373810
2020-07-17 09:46:57defrengcreate