Opened 9 years ago

Last modified 8 years 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: Jonathan Lange Branch: branches/test-docstrings-6301-2
branch-diff, diff-cov, branch-cov, buildbot
Author: tomprince, jml


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 9 years 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 9 years ago by sreepriya

Owner: set to sreepriya

comment:3 Changed 9 years ago by sreepriya

Owner: sreepriya deleted

comment:4 Changed 9 years ago by Richard Wall

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 9 years ago by Tom Prince

Keywords: policy added

comment:6 Changed 9 years 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 9 years ago by Tom Prince

Author: tomprince
Branch: branches/test-docstrings-6301

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

comment:8 Changed 9 years ago by Tom Prince

Author: tomprincejml, tomprince
Keywords: review added

This is a very rough first pass at adding some documentation

comment:9 Changed 8 years ago by Richard Wall

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.

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



comment:10 Changed 8 years ago by Tom Prince

Author: jml, tomprincetomprince, jml
Branch: branches/test-docstrings-6301branches/test-docstrings-6301-2

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

Note: See TracTickets for help on using tickets.