Opened 11 years ago

Last modified 18 months ago

#84 enhancement new

We need tests for example code

Reported by: spiv Owned by: rwall
Priority: high Milestone:
Component: core Keywords: documentation
Cc: exarkun, spiv, jml, tom@…, richard@… Branch: branches/testable-examples-84
(diff, github, buildbot, log)
Author: rwall Launchpad Bug:

Description


Change History (25)

comment:1 Changed 11 years ago by spiv

We occasionally break example code with new releases,
because we don't have any tests that verify that the example
code works.  I'm not sure how to fix this, but it would be
nice to figure out a way to do it.

comment:2 Changed 11 years ago by jml

Perhaps just running pychecker over the examples would be a
good start?

comment:3 Changed 11 years ago by spiv

Hmm, yeah, that could help -- but see also #86.  Twisted
currently causes lots of spurious warnings from pychecker,
so it'd be hard to easily tell whether an example is broken,
or is just importing code that triggers false positives.

comment:4 Changed 9 years ago by exarkun

doctests would be the best way to deal with this problem, I think.

comment:5 Changed 8 years ago by hypatia

  • Cc hypatia removed
  • Component set to core
  • Owner changed from hypatia to edsuom

comment:6 Changed 5 years ago by glyph

  • Keywords review added
  • Owner changed from edsuom to khorn
  • Priority changed from low to high

Here's hoping that you can do something with this shortly after the Sphinx conversion...

comment:7 Changed 5 years ago by glyph

This code might serve as a good example, or at least, an example: http://divmod.org/trac/browser/trunk/Nevow/nevow/test/test_howtolistings.py

comment:8 Changed 5 years ago by glyph

  • Keywords review removed

comment:9 Changed 4 years ago by binjured

  • Cc tom@… added

Just to clarify, we are now looking to replace broken/stale code with tested-first examples. I don't think a ticket that addresses tests for all sample code is very useful (or likely to be maintained properly) in the first place, so it'd be nice if we could close this.

comment:10 follow-up: Changed 4 years ago by exarkun

We should resolve this ticket with a plan about how to write testable examples. Then, indeed, there can be many more tickets for replacing (which may not actually mean deleting and rewriting in all cases, existing examples that can be reformed should be) existing examples with tested equivalents.

comment:11 Changed 4 years ago by khorn

Even better than a plan would be a plan plus an actual example of a test harness for a particular example or set of examples.

comment:13 Changed 4 years ago by <automation>

  • Owner khorn deleted

comment:14 Changed 2 years ago by rwall

  • Cc richard@… added

In attachment:ticket:5596:names-examples-tests-5596.patch Download

  • Added tests for consistent USAGE documentation and command line error reporting
  • Added tests for standard python shebang line and that the script is executable.

I copied and modified the Nevow example testing mixin mentioned above.

Maybe the same tests can be re-used on other examples.

comment:15 Changed 19 months ago by rwall

  • Author set to rwall
  • Branch set to branches/testable-examples-84

(In [37383]) Branching to 'testable-examples-84'

comment:16 Changed 19 months ago by rwall

(In [37384]) some new twisted.names example tests for discussion. refs #84

comment:17 in reply to: ↑ 10 Changed 19 months ago by rwall

  • Keywords review added

Replying to exarkun:

We should resolve this ticket with a plan about how to write testable examples. Then, indeed, there can be many more tickets for replacing (which may not actually mean deleting and rewriting in all cases, existing examples that can be reformed should be) existing examples with tested equivalents.

I've started a new Wiki page with the outlines of a plan.

There is also a branch where I added some further tests for the twisted.names examples.

Before I go any further I'd appreciate a review of the plan.

Tell me which parts of the plan make sense, and I will create corresponding tickets where the discussion can continue.

Other parts of the plan I will refine or ditch, based on feedback.

comment:18 Changed 19 months ago by tom.prince

That is a very well thought out document. Thanks.

  1. "example code should be a complete working example suitable for copying and pasting and running by the reader"
  • Should it also be cross platform? Or should there be platform specic examples? eg can an example refer to /etc/hosts

I think it will depend on the examples. systemd examples will clearly be platform specific. Perhaps ideally, examples of cross-platform functionality will be cross-platform. But, clearly, that shouldn't be a reason for removing an example. And, I think it doesn't hurt to have some platform-specific examples of cross-platform functionality, in addition to cross-platform examples.

  1. "example code should be short"
  • How short? In terms of lines? Or in terms of the number of top level fuctions and classes? Perhaps an example of a too long example.

I'm not sure if there is a good single measure. Those are both reasonable hints. Perhaps some measure of complexity, or extraneous complexity would be good.

  1. "example code should be commented very extensively"
  • Does this include docstrings?

Yes

  • Does this include epytext style API docs in docstrings?

Maybe? I think this might depend on a case-by-case basis.

Can too many comments be distracting in a simple example?

Probably.

  1. "example code should exhibit 'best practice'"
  • Should we identify

yes

remove examples which refer to old deprecated APIs?

no. As glyph has commented elsewhere, we don't want to remove documentation or tests for deprecated things until that functionality is actually removed. People may still encounter those APIs, and having examples to learn how they are to be used is useful for that purpose.

What is best practice? I guess most Twisted core devs will agree on most things, but are there any controversial practices eg inlineCallbacks?

I think currently this is largely an oral tradition (couting IRC as oral :)). What there is is encoded in the coding-standard and other policy documents. It would be good to capture more of it.

Can examples ever use, or suggest using external libraries? Eg treq, Nevow, ampoule, tx??? etc. If those external libraries are considered best practice? I *think* its quite common for people to be directed to better external alternatives eg treq vs Agent or wokel?? vs twisted.words??

<tomprince> dreid: Fetching webpages is the point of the client side of twisted web. Saying, "if you want to get webpages, go use treq", while currently the best answer and potentially a reasonable stance to take, isn't the stance twisted wants to take in the long run, in my opinion.
<glyph> tomprince: hi, yes. treq is license-compatible though, if you want to move things between them you can.
<tomprince       On the other hand, I'm not sure if the requst like api that treq has, is the api that twisted wants to expose.
<tomprince>       I think a similar argument could be made for parts of klein.
<tomprince>       glyph: Sure. I don't want to step on someones toes, though.
<glyph>   tomprince: right, so treq and klein have a more rapid release cycle and fewer compatibility guarantees so they can experiment and evolve
<glyph>   tomprince: when it looks right it can happily move into twisted :)
<dreid>   tomprince: I don't disagree that a higher level API for an HTTP client should be in Twisted.  I just have no immediate plans to propose treq as that higher level API.
<glyph>   tomprince: You've got basically the right idea but there's no need to rush.
<glyph>   I mean, no need to lag either; if you think you have an appropriate high-level request API taking ideas and possibly code from treq, by all means submit it for review.
<glyph>   But I don't think there's any need to worry if things continue as they are.

comment:19 Changed 19 months ago by tom.prince

Some comments on the code in the branch (which should eventually be attached to a separate ticket:

  1. positionalArgCount needs an @ivar.
    • But, I'm not sure this can be made generic. Some commands may take an arbitrary number of options and some may do further checks on the options.
  2. I like the unit tests. But, we should figure out some way to test that fakeGetHostByName behaves enough like getHostByName to serve as a fake. Calling getHostByName is the point of the example, so by patching it out, we avoid testing what is essentially the most important part of the example.

Buildbot has something.

comment:20 Changed 19 months ago by tom.prince

Some other comments:

  • doc/historic/ should be append only, so we don't want to run linter's on it.
  • find isn't portable. It would be better to extend twistedchcker to run on arbitrary directories, or something.
  • We also want to check code from the howtos (some of which might be incomplete snippets). Arguably, we probably want to turn a bunch of the examples into howtos at some point.

comment:21 follow-up: Changed 19 months ago by exarkun

We also want to check code from the howtos (some of which might be incomplete snippets). Arguably, we probably want to turn a bunch of the examples into howtos at some point.

I'd like to emphasize this. I think we actually want to turn *all* the examples into howtos. An example with narration is vastly more educational than an example in a void.

comment:22 in reply to: ↑ 21 Changed 19 months ago by khorn

Replying to exarkun:

We also want to check code from the howtos (some of which might be incomplete snippets). Arguably, we probably want to turn a bunch of the examples into howtos at some point.

I'd like to emphasize this. I think we actually want to turn *all* the examples into howtos. An example with narration is vastly more educational than an example in a void.

One way of handling this would be to move all example code out of the howto documents into separate python files, for ease of testing. Then pull relevant parts of the code (functions, classes, line number ranges, etc) into the howto documents for rendering into HTML. This might make things a bit more difficult for HOWTO authors, however, as they would have to be looking at two different files when writing up their docs.

Lore can only do this by line number, though IIRC there is a ticket for other types of includes. (aha - #1073)

Sphinx can do this, but obviously we haven't switched over to Sphinx at this point in time.

In any case, I'm really only talking about long-term plans here. Nothing that should delay or derail this ticket.

comment:23 Changed 19 months ago by exarkun

Lots of existing howtos already do this. I don't think there's much (if any) deficiency in the current tool support.

comment:24 Changed 19 months ago by rwall

Thanks for all the feedback. I've started creating some separate tickets and added links to the plan:

I'll do more later.

comment:25 Changed 18 months ago by tom.prince

  • Keywords review removed
  • Owner set to rwall

Taking this out of review, as there isn't really anything actionable in here to review.

Note: See TracTickets for help on using tickets.