Opened 19 months ago

Last modified 5 months ago

#6301 enhancement new

There should be some clear documentation on how to document tests.

Reported by: tom.prince Owned by: tom.prince
Priority: normal Milestone:
Component: core Keywords: documentation policy
Cc: jml Branch: branches/test-docstrings-6301-2
(diff, github, buildbot, log)
Author: tomprince, jml Launchpad Bug:

Description

The current documentation seems to consist be:

All unit test methods should have docstrings specifying at a high
level the intent of the test. That is, a description that users of the
method would understand.

jml posted the following to Google+, which seems to capture the unstated guidelines that are used in practice:

A lot of projects I contribute to insist that tests have comments or docstrings in them. If ever you're working on a project that has a similar requirement, and you're struggling to write docstrings for your tests, here's a handy five-step guide:

  1. Write the first docstring that comes to mind. It will almost certainly be:
      """Test that input is parsed correctly."""
    
  1. Get rid of "Test that" or "Check that". We know it's a test.
      """Input should be parsed correctly."""
    
  1. Seriously?! Why'd you have to go and add "should"? It's a test, it's all about "should".
      """Input is parsed correctly."""
    
  1. "Correctly", "properly", and "as we expect" are all redundant. Axe them too.
      """Input is parsed."""
    


  1. Look at what's left. Is it saying anything at all? If so, great. If not, consider adding something specific about the test behaviour and perhaps even why it's desirable behaviour to have.
      """
      Input is parsed into an immutable dict according to the config
      schema, so we get config info without worrying about input
      validation all the time.
      """
    

Change History (10)

comment:1 Changed 19 months ago by tom.prince

I wrote some words on #5952:

Generally a test docstring wants to be something like When <some thing is done>, <something else occurs>. Often <something is done> is a method being called. And <something else occurs> is generally what is captured by the assertion(s).

comment:2 Changed 19 months ago by sreepriya

  • Owner set to sreepriya

comment:3 Changed 19 months ago by sreepriya

  • Owner sreepriya deleted

comment:4 Changed 17 months ago by rwall

  • Keywords documentation added; doc removed

I think you should also mention when / whether and why L{}, C{} @param etc should be included in test docstrings given that the tests are not included in the API docs.

See also: #6318, #6012

comment:5 Changed 16 months ago by tom.prince

  • Keywords policy added

comment:6 Changed 15 months ago by tom.prince

An explanation from glyph, on #5780:

[...] exarkun was referring to our preferred convention for test docstrings. Sadly I don't think this is documented officially anywhere, so I'll re-explain it again here. Test docstrings should make a positive statement about the attributes of the desired behavior of the code under test. So saying that something "does not raise an exception" is not helpful; most code doesn't raise an exception and it doesn't do a whole bunch of other things too :). Instead, docstrings should say things like "when a UTF-8 message is sent with sendMessage, it will be written to the transport." Also, if you're going to talk about exceptions, you should talk about specific types; "Exception" is a bit too vague. (It would be silly to talk about NameError though.

comment:7 Changed 15 months ago by tomprince

  • Author set to tomprince
  • Branch set to branches/test-docstrings-6301

(In [38868]) Branching to test-docstrings-6301.

comment:8 Changed 15 months ago by tom.prince

  • Author changed from tomprince to jml, tomprince
  • Keywords review added

This is a very rough first pass at adding some documentation

comment:9 Changed 14 months ago by rwall

  • Keywords review removed
  • Owner set to tom.prince

Code Review

  • I ran lore to verify that the changes render properly to HTML. They do.

Some points:

  1. doc/core/development/policy/test-standard.xhtml
    1. The new "Documenting Tests" section should start by stating that all unit tests *must* have docstrings.
      1. Maybe just copy the new paragraph from doc/core/development/policy/coding-standard.xhtml
    2. Also state that the docstrings should follow the same rules described in "coding-standard.html#auto9".
    3. Maybe add an explicit <a name="docstrings"> to coding-standard.html for a neat / stable link.
    4. Add an attribution to JML and a footnote link to the original work.
    5. Add a code block with a test method from trunk which demonstrates an ideal test.
    6. Say something about class docstrings for TestCases.
    7. I still don't understand why we bother to use epydoc markup in test docstrings.
      1. It makes the docstrings difficult to read.
      2. There are no rendered API docs for tests anyway.
      3. And since Pydoctor doesn't render the docstrings, the epydoc markup is never validated and therefore often broken.
      4. I suggest we change the policy to not require epydoc markup in test docstrings.
    8. "Further thoughts" is a poor title and the section seems out of context.
      1. Change the title.
      2. Add some context
      3. Maybe move that section to "What to Test, What Not to Test" which is currently pretty sparse.
    9. The "one sentence per line" rule makes the diff difficult to review in Github. Very long sentences are obscured by the overflow css and there's no horizontal scrollbar.
      1. https://github.com/twisted/twisted/compare/trunk...test-docstrings-6301#L1R159

Please re-submit for review after addressing or responding to the
points above.

Thanks.

-RichardW.

comment:10 Changed 5 months ago by tomprince

  • Author changed from jml, tomprince to tomprince, jml
  • Branch changed from branches/test-docstrings-6301 to branches/test-docstrings-6301-2

(In [42314]) Branching to test-docstrings-6301-2.

Note: See TracTickets for help on using tickets.