twisted.trial.test.test_asyncassertions doesn't follow the coding standard

[itamar]

twisted.trial.test.test_asyncassertions should be improved so that e.g. all tests have docstrings, and the existing docstrings are more explicit about what they are testing, etc..

[marktraceur]

A patch to elaborate existing docstrings and add new ones.

[marktraceur]

I figured I'd jump in and contribute a little patch! My Saturday nights are rather uneventful anyway :)

I'm also recording the process of fixing this bug as part of an attempt to drive contributions to free software! No need to act differently, but I figured I'd say something in the interest of full disclosure :)

[marktraceur]

A patch to elaborate existing docstrings and add new ones.

[marktraceur]

A patch to elaborate existing docstrings and add new ones.

[exarkun]

Thanks for the patch. Just a couple issue tracker comments. Make sure to unassign the ticket when you submit it for review (empty owner is at the top of the owners drop-down). Also, keywords are separated by spaces, not commas.

[marktraceur]

Sounds good! The interface is a bit arcane to new folks, but I'll see if I can't clarify some of this in the wiki pages for new contributors. Thanks a lot for the help!

[marktraceur]

Oh, hm, I should note that the latest attachment (docstring-elaboration.2.patch) is the patch I'd like to be reviewed.

[therve]

Thanks a lot for the patch. I'm going to be a bit pedantic, but as the patch is mostly about test docstring I feel like it's relevant here: test docstring shouldn't use "Test/check that" and "will/should". For example:

Test that assertFailure, when called on a function that raises a 
L{ZeroDivisionError}, will fail the deferred function if it instead 
raises an L{OverflowError}. 

could be written:

C{assertFailure} returns a C{Failure} if the passed L{Deferred} doesn't fail with expected exception.

Note that I also changed the docstring to make it a bit more generic: the test is not about the precise type of exception raised, but about the behavior of assertFailure in general. Also, there is no such thing as a deferred function, but rather a function that returns a Deferred.

Thanks!

[kkpattern]

A patch to elaborate existing docstrings and add new ones.

[kkpattern]

A patch to elaborate existing docstrings and add new ones, based on therve's comments.

[thijs]

Thanks for your contribution.

1. _checkInfo still needs a docstring with @param and @type for each parameter
2. Most of the test names are incorrect, eg. test_assertFailure_wrongException should be test_assertFailureWrongException
3. C{Failure} should be L{Failure}

[kkpattern]

A patch to elaborate existing docstrings and add new ones, also elaborate the test names.

[kkpattern]

Updated patch, incorporating review comments.

[therve]

(In [38585]) Branching to 'test-asyncassertions-cleanup-6049'

[therve]

Thanks a lot for the patch. I've committed the patch in a branch, and there are a bunch of twistedchecker failures remaining:  http://buildbot.twistedmatrix.com/builders/twistedchecker/builds/730/steps/run-twistedchecker/logs/new%20twistedchecker%20errors. Please provide against the branch to fix them.