= Review details =

 * Link to document: http://twistedmatrix.com/projects/core/documentation/howto/testing.html
 * Reviewer's name: Jonathan Lange
 * Review date: 15 March 2006

= Document expectations =

== Intended user ==

Before someone begins using this document, I'd really really like it if they knew:
 * what a unit test is
 * how to write unit tests using xUnit
 * how to run Trial
 * how Deferreds work and how to use them

== Outcomes ==

Once someone has read this document they should understand:
 * how to write tests for code that returns Deferreds
 * how to write tests for Twisted protocols (maybe)
 * how to use skip to write platform specific tests
 * how to use todo to write tests for non-existent/broken code


= Document review =

== Coverage of subject matter ==

This document seems to cover the following subject matter at least acceptably well:
 * Don't use reactor.start, reactor.stop, reactor.iterate

This document seems to be attempting to cover the following subject matter, but its coverage is flawed:
 * The timeout description doesn't describe what a timeout does to the test results
 * The heading "Twisted-specific quirks: ..." shouldn't enumerate the quirks, and probably shouldn't call them quirks
 * Discusses how to clean up the reactor but doesn't mention that leaving the reactor unclean will fail the test
 * Should provide examples of how to clean up the reactor
 * Explains that Trial extends unittest, but doesn't tease out what that means for writing tests

This document ought to be covering the following subject matter but is not:
 * the todo attribute
   * its existence
   * appropriate values to set it to
   * why one might want to use it
 * skipping tests
   * that it is possible
   * how to do it with the skip attribute
   * how to do it by raising SkipTest
 * That calling log.err will fail a test
   * [although the api surrounding this is still under discussion!]


This document is recommending the following things that it shouldn't be recommending:
 * Maybe mentioning reactor.iterate et al is a bad idea

This document could be supplemented by links to these existing pre-requisitie ("you should read this first") documents:
 * some sort of document on how to write unit tests
   * Python's unittest documentation?
   * diveintopython.org has an excellent chapter on test-driven development
 * The Trial manpage

This document could be supplemented by links to these existing follow-up ("now that you know X you can try Y") documents: 

This document could be supplemented by as-yet non-existant pre-requisite ("you should read this first") documents on:

This document could be supplemented by links to these as-yet non-existant follow-up ("now that you know X you can try Y") documents:
 * How to extend Trial


== Style ==
The following changes to the style of the document would make it easier to read:

 * Way more examples
 * Structure the entire document around how to test code, rather than documenting quirks
 * Create headings within "Leave the Reactor as you found it" section


== Overall summary ==

The document seems particularly slanted toward writing tests to be ''contributed'' to Twisted, rather than writing tests for code that ''uses'' Twisted.  I recommend that the idiosyncracies of code destined for the Twisted repository be documented in the coding standards.

This document is: better than having no documentation, but with some glaringly obvious improvements to make