documentation patch questions

[Annie]

Hi all -

I have some questions before I submit a ticket with a patch to change
a few things in the docs/topics/testing.txt file. One change is a typo
fix, and another is a possible change I have a question about. Since
they're both relatively minor changes to the same file (no major
rewrites required), do I submit both on one ticket (after my question
about the second change is answered here) with my patch, or do they
need to be submitted separately?

This example under the "Running tests" section of the page is what
confused me when I read it, so I thought I should check before I
suggested a change in case I was reading the example incorrectly. *g*

Here's the relevant bit of the docs:

------------

And it gets even more granular than that! To run a *single* test
method inside a test case, add the name of the test method to the
label::

$ ./manage.py test animals.AnimalTestCase.testFluffyAnimals

------------

In both unittest and doctest examples given above that test run
example, there is no testFluffyAnimals method. The unittest defines
testSpeaking and the doctest uses speak. I think it would be clearer
if the example read like this:

$ ./manage.py test animals.AnimalTestCase.testSpeaking

Am I reading the original example correctly? Or is it just that my
brain's tired from looking at this stuff too long today? :)

Ok, I lied, there's actually a third issue--something I noticed as I
was double checking before I sent this email: shouldn't the method be
named test_speaking, if we're supposed to follow the guidelines on the
coding style page? "Use underscores, not camelCase, for variable,
function and method names (i.e. poll.get_unique_voters(), not
poll.getUniqueVoters)."

Thanks for the input -
Annie Mullin

[Russell Keith-Magee]

On Wed, Dec 7, 2011 at 4:55 PM, Annie <ann...@gmail.com> wrote:
> Hi all -
>
> I have some questions before I submit a ticket with a patch to change
> a few things in the docs/topics/testing.txt file. One change is a typo
> fix, and another is a possible change I have a question about. Since
> they're both relatively minor changes to the same file (no major
> rewrites required), do I submit both on one ticket (after my question
> about the second change is answered here) with my patch, or do they
> need to be submitted separately?

Hi Annie,

If someone were committing in the testing docs, and there are two
minor fixes, they're just as likely to roll them into a single commit,
with the comment "Minor updates to the testing documentation"; in
which case, a single ticket is probably ok.

The point at which a second ticket is called for is the point at which
it is likely that one of the fixes will *not* be applied. This could
be due to a number of reasons:

* One of the changes is large, and requires further review

* The updates relate to different parts of the documentation, and
aren't thematically bound (e.g., fixing usage of its/it's in multiple
locations)

* One of the changes is controversial or requires further discussion.

In this case, I'd be happy to say both these changes could be
classified as two related, uncontroversial improvements, so a single
ticket is probably OK.

> This example under the "Running tests" section of the page is what
> confused me when I read it, so I thought I should check before I
> suggested a change in case I was reading the example incorrectly. *g*
>
> Here's the relevant bit of the docs:
>
> ------------
>
> And it gets even more granular than that! To run a *single* test
> method inside a test case, add the name of the test method to the
> label::
>
>    $ ./manage.py test animals.AnimalTestCase.testFluffyAnimals
>
> ------------
>
>
> In both unittest and doctest examples given above that test run
> example, there is no testFluffyAnimals method. The unittest defines
> testSpeaking and the doctest uses speak. I think it would be clearer
> if the example read like this:
>
>    $ ./manage.py test animals.AnimalTestCase.testSpeaking
>
>
> Am I reading the original example correctly? Or is it just that my
> brain's tired from looking at this stuff too long today? :)

Looks like a good catch to me. We should certainly aim to be
internally consistent, and I can't see any reason not to be in this
case.

> Ok, I lied, there's actually a third issue--something I noticed as I
> was double checking before I sent this email: shouldn't the method be
> named test_speaking, if we're supposed to follow the guidelines on the
> coding style page? "Use underscores, not camelCase, for variable,
> function and method names (i.e. poll.get_unique_voters(), not
> poll.getUniqueVoters)."

Also a good catch.

If you're in the area and aiming for perfection, I'd be inclined to
take a closer look at the test names, too. testSpeaking isn't an
especially good test name -- something like test_animals_can_speak(),
that phrases the test as an assertion would be a better example.
Giving the test a docstring that will be used by the test runner
verbose/error output would also be a good idea.

I look forward to seeing your ticket and patch. Thanks for pitching in!

Yours,
Russ Magee %-)

[Annie]

Thanks for the feedback, Russ. :) I've created the ticket and
submitted the patch. It's ticket #17364.

On Dec 7, 4:44 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
<snip>

> If you're in the area and aiming for perfection, I'd be inclined
> to take a closer look at the test names, too. testSpeaking isn't
> an especially good test name -- something like
> test_animals_can_speak(), that phrases the test as an assertion
> would be a better example.

I've used your suggestion, thanks.

> Giving the test a docstring that will be used by the test runner
> verbose/error output would also be a good idea.

I'm still learning the unittest framework, so I'm not sure exactly how
to clearly phrase what it's doing, but I'll give it a shot. *g* Do you
mean something like:

'''
AnimalTestCase creates several animal objects and uses assertEqual
to verify that the results from speak() match the expected results.
'''

Something in that general vicinity? *g* (There isn't a full example
that all the sample tests are being written for to refer back to,
unfortunately. I know I would find that useful, coming in as someone
not familiar with writing tests, because then it's easier to see what
exactly is different about the *testing* methods being demonstrated,
rather than having to worry about keeping track of what the example
class or method being tested is doing, too. FWIW. *g*)

> I look forward to seeing your ticket and patch. Thanks for
> pitching in!

You bet. :)
Annie

[Russell Keith-Magee]

On Fri, Dec 9, 2011 at 9:58 AM, Annie <ann...@gmail.com> wrote:
> Thanks for the feedback, Russ. :) I've created the ticket and
> submitted the patch. It's ticket #17364.

Thanks! The patch looks good; the only thing preventing me from
marking it ready for checkin is the discussion below about something
else you may want to add to the patch.

>> Giving the test a docstring that will be used by the test runner
>> verbose/error output would also be a good idea.
>
> I'm still learning the unittest framework, so I'm not sure exactly how
> to clearly phrase what it's doing, but I'll give it a shot. *g* Do you
> mean something like:
>
> '''
> AnimalTestCase creates several animal objects and uses assertEqual
> to verify that the results from speak() match the expected results.
> '''

Close, but probably a little verbose. All you're looking for is a 1
line expression of what is being tested. Aside from the basic
explanatory benefit, the docstring is used when you run the test suite
in verbose mode. If you run Django's test suite with -v 2, you'll see
what I mean. Without a docstring, you will see statements like:

test_can_speak (myapp.AnimalTestCase)... ok

but if you have a docstring on your test method:

def test_animals_can_speak(self):
"Animals that can speak are correctly identified"

The verbose output is a little more humane:

Animals that can speak are correctly identified... ok

Long comments aren't required; you just need to provide something that
can be answered "OK".

Yours,
Russ Magee %-)

[Annie]

Russ:

> Long comments aren't required; you just need to provide something
> that can be answered "OK".

That makes perfect sense, especially now that I see how it's being
used. *g* (Hadn't gotten to that part of the python unittest docs yet.
*g*) I've updated the patch and attached it.

Thanks for your help!
Annie

[Russell Keith-Magee]

I've just marked the ticket ready for checkin. As soon as a commiter
has some free time, it will go into trunk.

Thanks for the contribution! May this not be your last :-)

Yours,
Russ Magee %-)