Skip to content
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

Documentation of TestCase.runTest is incorrect and confusing #66349

Closed
vadmium opened this issue Aug 6, 2014 · 17 comments
Closed

Documentation of TestCase.runTest is incorrect and confusing #66349

vadmium opened this issue Aug 6, 2014 · 17 comments
Labels
docs Documentation in the Doc dir type-feature A feature request or enhancement

Comments

@vadmium
Copy link
Member

vadmium commented Aug 6, 2014

BPO 22153
Nosy @pitrou, @rbtcollins, @ezio-melotti, @voidspace, @berkerpeksag, @vadmium
Files
  • myworkdoc.patch
  • runtest.patch
  • runTest2-default.patch
  • runTest2-3.4.patch
  • 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:

    assignee = None
    closed_at = <Date 2015-07-22.19:25:05.938>
    created_at = <Date 2014-08-06.05:00:02.156>
    labels = ['type-feature', 'docs']
    title = 'Documentation of TestCase.runTest is incorrect and confusing'
    updated_at = <Date 2015-07-22.19:25:05.936>
    user = 'https://github.com/vadmium'

    bugs.python.org fields:

    activity = <Date 2015-07-22.19:25:05.936>
    actor = 'rbcollins'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2015-07-22.19:25:05.938>
    closer = 'rbcollins'
    components = ['Documentation']
    creation = <Date 2014-08-06.05:00:02.156>
    creator = 'martin.panter'
    dependencies = []
    files = ['36401', '37372', '37376', '37377']
    hgrepos = []
    issue_num = 22153
    keywords = ['patch']
    message_count = 17.0
    messages = ['224907', '225466', '230137', '230147', '230180', '230181', '230182', '230188', '230190', '230191', '230231', '230312', '230352', '232226', '232260', '247143', '247145']
    nosy_count = 9.0
    nosy_names = ['pitrou', 'rbcollins', 'ezio.melotti', 'michael.foord', 'docs@python', 'python-dev', 'berker.peksag', 'martin.panter', 'evilzero']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue22153'
    versions = ['Python 3.4', 'Python 3.5', 'Python 3.6']

    @vadmium
    Copy link
    Member Author

    vadmium commented Aug 6, 2014

    The documentation for "unittest.TestCase" says "the standard implementation of the default 'methodName', runTest(), will run every method starting with 'test' as an individual test". However:

    >>> from unittest import *
    >>> class Test(TestCase):
    ...     def test_method(self): pass
    ... 
    >>> t = Test()
    >>> t.run()
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.4/unittest/case.py", line 552, in run
        testMethod = getattr(self, self._testMethodName)
    AttributeError: 'Test' object has no attribute 'runTest'

    After further experimentation, I see that if my test method is called "runTest", it can be automatically discovered, but only if there are no other test- prefixed methods.

    Perhaps you could drop the end of the second paragraph for TestCase, so that it just reads:

    Each instance of TestCase will run a single base method: the method named "methodName".

    I think the details about the test- prefix and counting results are covered elsewhere, and in most uses you wouldn't instantiate a TestCase yourself, so changing the method name is irrelevant.

    Also, perhaps under "TestLoader.loadTestsFromTestCase" it should say:

    If no methods with the usual name prefix are found, but the "runTest" method is implemented, there will be a single test case using that method.

    @vadmium vadmium added the docs Documentation in the Doc dir label Aug 6, 2014
    @evilzero
    Copy link
    Mannequin

    evilzero mannequin commented Aug 17, 2014

    Updated documentation following suggestion.

    @evilzero evilzero mannequin added the type-feature A feature request or enhancement label Aug 17, 2014
    @vadmium
    Copy link
    Member Author

    vadmium commented Oct 28, 2014

    The patch seems reasonable to me

    @pitrou
    Copy link
    Member

    pitrou commented Oct 28, 2014

    IMO hiding the existence of runTest would be best. It doesn't seem to make anything more flexible, and it complicates the documentation.

    @vadmium
    Copy link
    Member Author

    vadmium commented Oct 28, 2014

    Do you mean pretending there is no default “methodName” value, or pretending that the runTest() method is not invoked by discovery?

    I would have to check, but I think I have relied on the runTest() method being discovered in the past, when I did not think a more original test method name was useful. Though I agree that it makes the behaviour more complicated for little extra flexibility.

    @rbtcollins
    Copy link
    Member

    runTest is part of the current API. I think if we're going to hide it we should do so as part of a deprecation path. (I'm +1 on hiding it).

    @pitrou
    Copy link
    Member

    pitrou commented Oct 28, 2014

    Le 28/10/2014 23:01, Robert Collins a écrit :

    runTest is part of the current API. I think if we're going to hide
    it
    we should do so as part of a deprecation path. (I'm +1 on hiding it).

    We don't need a deprecation path to remove something from the
    documentation, IMO. It wouldn't break any existing code, it would just
    reduce the cognitive load for newcomers.

    (and, while people may have chosen to use runTest, it doesn't mean they
    need it in any way)

    @vadmium
    Copy link
    Member Author

    vadmium commented Oct 28, 2014

    Removing stuff from the documentation would make it hard to understand what existing code means that uses runTest(). Isn’t this a case where you would add a big red “Deprecated since version . . .” warning instead? Maybe a comprimise would be to “hide” its documentation away in a separate deprecated section or something.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 29, 2014

    Le 29/10/2014 00:22, Martin Panter a écrit :

    Martin Panter added the comment:

    Removing stuff from the documentation would make it hard to
    understand
    what existing code means that uses runTest(). Isn’t this a case where
    you would add a big red “Deprecated since version . . .” warning
    instead? Maybe a comprimise would be to “hide” its documentation away in
    a separate deprecated section or something.

    Well, I wasn't proposing a deprecation (although that can be done as
    well, if we want). I was proposing that this detail be hidden. I don't
    know if it would make existing software harder to understand. I don't
    think I've ever seen an actively-maintained project that used runTest().

    The simple way to hide it would be to stop documenting the TestCase
    constructor and its signature. You aren't supposed to instantiate
    TestCase yourself.

    @rbtcollins
    Copy link
    Member

    Constructing test case objects directly is part of the protocol, and its needed for framework and extension authors.

    I've seen folk use runTest, but rarely enough that I think we could deprecate it. My argument is that while its supported we should be clear about when it should be used, and how.

    @rbtcollins
    Copy link
    Member

    Oh, one thought - in testtools we split out 'docs for test writers' and 'docs for framework folk'. That would facilitate getting runTest out of test writers face while still documenting it appropriately.

    Related to that is my ongoing push to split the protocol so that these concerns are not intertwined the way they are today, but that needs to wait for the existing backlog to clear up :)

    @rbtcollins rbtcollins changed the title There is no standard TestCase.runTest implementation Documentation of TestCase.runTest is incorrect and confusing Oct 29, 2014
    @rbtcollins
    Copy link
    Member

    I'll apply the patch monday if there are no further comments before then.

    @ezio-melotti
    Copy link
    Member

    Oh, one thought - in testtools we split out 'docs for test writers' and
    'docs for framework folk'. That would facilitate getting runTest out of
    test writers face while still documenting it appropriately.

    This has already been discussed in the past, and IIRC there was consensus about giving unittest docs the same treatment. I don't remember if there was an issue for this.

    @vadmium
    Copy link
    Member Author

    vadmium commented Dec 5, 2014

    Updated patch with indentation fixed and new wording. I am just guessing the RST syntax based on the surrounding text, so please review :)

    @vadmium
    Copy link
    Member Author

    vadmium commented Dec 6, 2014

    Updated patch, which applies to current tip of the default branch, and includes the formatting fix. Also including a version that applies to the 3.4 branch. Alternatively, if you patch the 3.4 branch it looks like merging to default automatically gives the correct result.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 22, 2015

    New changeset eefc157b3096 by Robert Collins in branch '3.4':
    Issue bpo-22153: Improve unittest docs. Patch from Martin Panter and evilzero.
    https://hg.python.org/cpython/rev/eefc157b3096

    New changeset 10f5a7fa26d5 by Robert Collins in branch '3.5':
    Issue bpo-22153: Improve unittest docs. Patch from Martin Panter and evilzero.
    https://hg.python.org/cpython/rev/10f5a7fa26d5

    New changeset 45bd2dadbd0d by Robert Collins in branch 'default':
    Issue bpo-22153: Improve unittest docs. Patch from Martin Panter and evilzero.
    https://hg.python.org/cpython/rev/45bd2dadbd0d

    @rbtcollins
    Copy link
    Member

    Thanks for the update, it looks good to me. Applied to 3.4 and up. I'm not applying to 2.7 at this stage.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    docs Documentation in the Doc dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants