python / cpython

The Python programming language
https://www.python.org
Other
62.36k stars 29.95k forks source link

Unittest - Return empty string instead of None object on shortDescription() #73872

Closed ad6ec1fe-04ad-4314-a37d-583d2453dfb2 closed 7 years ago

ad6ec1fe-04ad-4314-a37d-583d2453dfb2 commented 7 years ago
BPO 29686
Nosy @rhettinger, @bitdancer, @serhiy-storchaka, @viniciusd
PRs
  • python/cpython#383
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = created_at = labels = ['type-bug', 'tests'] title = 'Unittest - Return empty string instead of None object on shortDescription()' updated_at = user = 'https://github.com/viniciusd' ``` bugs.python.org fields: ```python activity = actor = 'viniciusd' assignee = 'none' closed = True closed_date = closer = 'rhettinger' components = ['Tests'] creation = creator = 'viniciusd' dependencies = [] files = [] hgrepos = [] issue_num = 29686 keywords = [] message_count = 6.0 messages = ['288763', '288764', '288766', '288768', '288770', '289169'] nosy_count = 4.0 nosy_names = ['rhettinger', 'r.david.murray', 'serhiy.storchaka', 'viniciusd'] pr_nums = ['383'] priority = 'normal' resolution = 'rejected' stage = 'resolved' status = 'closed' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue29686' versions = ['Python 3.5'] ```

    ad6ec1fe-04ad-4314-a37d-583d2453dfb2 commented 7 years ago

    I have been browsing around the unittest standard library, and I realized that TestCase's shortDescription() method at lib/pythonX.X/unittest/case.py returns None when the there is no docstring on the test that is running. As shortDescription() should obviously return a string, I would recommend returning an empty string instead of None when no docstring is found. This came to mind when I was using testscenario package, which only displays the scenarioname when shortDescription() returns something but None. When we are starting from scratch a test suite, docstrings are left for another stage, when we have running (probably failed, if we are TDDing) unittests. Last yet not least, I am sure it's a good practice to avoid returning None, which forces None-checks, returning empty strings, lists, objects of the return type expected from that function.

    serhiy-storchaka commented 7 years ago

    This can break a code that checks whether shortDescription() is None.

    If you an empty string rather than None, just use the expression shortDescription() or ''.

    rhettinger commented 7 years ago

    It's too late to change this API. As Serhiy says, it risks breaking code that is currently running and correct.

    Note the the __doc__ on functions is set to None when there is no docstring. For better or worse, returning None is common in the Python world.

    ad6ec1fe-04ad-4314-a37d-583d2453dfb2 commented 7 years ago

    If it is tagged for future releases, whoever choose to update their Python version should expect code breaking and API changes, shouldn't them? I don't see code breaking as an issue against this request.

    On 1 March 2017 at 14:33, Raymond Hettinger \report@bugs.python.org\ wrote:

    Raymond Hettinger added the comment:

    It's too late to change this API. As Serhiy says, it risks breaking code that is currently running and correct.

    Note the the __doc__ on functions is set to None when there is no docstring. For better or worse, returning None is common in the Python world.

    ---------- nosy: +rhettinger resolution: -> rejected stage: -> resolved status: open -> closed


    Python tracker \report@bugs.python.org\ \http://bugs.python.org/issue29686\


    -- Vinícius Dantas de Lima Melo Graduando em Ciências e Tecnologia Universidade Federal do Rio Grande do Norte (UFRN) Escola de Ciências e Tecnologia (ECT) Natal, Rio Grande do Norte vinicius.gppcom@gmail.com viniciusdantas@bct.ect.ufrn.br vinicius.dantasdelimamelo@mail.utoronto.ca | Celular/Mobile Phone: +1 647 447 5737 Skype: viniciusdantas01

    bitdancer commented 7 years ago

    We don't make breaking changes unless there is strong motivation, which is lacking here.

    ad6ec1fe-04ad-4314-a37d-583d2453dfb2 commented 7 years ago

    In the point of view of a tester, if it's an error, they will know right away it is a test case problem, not an assert problem. That makes debugging easier. It is also important to note that, if it's an AssertionError, we may add a message. While, if it is an error, no message would be displayed but the original Exception's.

    As Selenium's example, as I said, was just a use case example.

    Finally, having the failure reason explicit is better than keeping it implicit.