On Wed, Jun 30, 2010 at 12:13 AM, Michael Foord <report@bugs.python.org> wrote:
>
> Michael Foord <michael@voidspace.org.uk> added the comment:
>
> Hey Nick,
>
> The tests are pretty much just copied from the previous version, so they aren't all appropriate. In fact I think that the first two tests (and even the typo tests) can just go as they really just test Python semantics and not the ContextDecorator itself.

Ah, yep, I can see how they would have been more important with the
old 2.4 compatible implementation. However, it's still useful to keep
them as a sanity check that the test context manager isn't doing
anything silly (i.e. if they pass and the decorator tests fail,
ContextDecorator is obviously at fault, whereas without these first
two tests the bug could be in the test context manager).

> The method tests are testing that the arguments (including keyword arguments) are properly passed through to the decorated function. I started to write a comment to that effect as I realised it wasn't obvious, but forgot to finish the comment... Separate instantiations just mean we don't need to worry about previous values.
>
> (I did them as methods as I was *starting* to write a test for general descriptor decorating - but the general case for that is very hard to solve so people just have to use their decorators in the right order instead. Nonetheless it is nice to have a test that decorates a method as well as the other function decorating tests.)

That makes sense, but a comment to that effect would be *very* helpful :)

Comments on the docs patch (I neglected to look at that initially):

For the @contextmanager updates, I would move the description of the
effect into an extra paragraph in the main body of the description,
then just use the versionadded tag to when the change was made.

For the ContextDecorator description itself, you may want to write the
last part of the first example (i.e. just the with statement) in a
doctest layout so you can show the expected output.