Opened 19 years ago
Last modified 9 years ago
#84 enhancement new
We need tests for example code
Reported by: | spiv | Owned by: | Richard Wall |
---|---|---|---|
Priority: | high | Milestone: | |
Component: | core | Keywords: | documentation |
Cc: | Jean-Paul Calderone, spiv, Jonathan Lange, Tom Davis, Richard Wall | Branch: |
branches/testable-examples-84
branch-diff, diff-cov, branch-cov, buildbot |
Author: | rwall |
Description
Change History (25)
comment:2 Changed 19 years ago by
Perhaps just running pychecker over the examples would be a good start?
comment:3 Changed 19 years ago by
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 17 years ago by
doctests would be the best way to deal with this problem, I think.
comment:5 Changed 16 years ago by
Cc: | hypatia removed |
---|---|
Component: | → core |
Owner: | changed from hypatia to edsuom |
comment:6 Changed 12 years ago by
Keywords: | review added |
---|---|
Owner: | changed from edsuom to khorn |
Priority: | low → high |
Here's hoping that you can do something with this shortly after the Sphinx conversion...
comment:7 Changed 12 years ago by
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 12 years ago by
Keywords: | review removed |
---|
comment:9 Changed 11 years ago by
Cc: | Tom Davis 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: 17 Changed 11 years ago by
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 11 years ago by
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 11 years ago by
Owner: | khorn deleted |
---|
comment:14 Changed 10 years ago by
Cc: | Richard Wall 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 9 years ago by
Author: | → rwall |
---|---|
Branch: | → branches/testable-examples-84 |
(In [37383]) Branching to 'testable-examples-84'
comment:16 Changed 9 years ago by
comment:17 Changed 9 years ago by
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.
- log:branches/testable-examples-84
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 9 years ago by
That is a very well thought out document. Thanks.
- "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.
- "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.
- "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.
- "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 9 years ago by
Some comments on the code in the branch (which should eventually be attached to a separate ticket:
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.
- I like the unit tests. But, we should figure out some way to test that
fakeGetHostByName
behaves enough likegetHostByName
to serve as a fake. CallinggetHostByName
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 9 years ago by
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 extendtwistedchcker
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: 22 Changed 9 years ago by
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 Changed 9 years ago by
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 9 years ago by
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 9 years ago by
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 9 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Richard Wall |
Taking this out of review, as there isn't really anything actionable in here to review.