Remove "Tests that", "Ensures that", etc. from test docstings

[Tim Graham]

Our policy is for test docstrings to state the expected behavior that each test demonstrates. Preambles such as "Tests that" or "Ensures that" aren't necessary.

Good: "Horizontal and vertical filter widgets keep selected options on page reload."
Bad: "Tests that horizontal and vertical...".

For example:

$ grep -rI "Tests that" * | wc -l
158
$ grep -rI "Test that" * | wc -l
222
$ grep -rI "Ensure that" * | wc -l
173
$ grep -rI "Ensures that" * | wc -l
14

If you spot any other similar patterns, please correct them. The policy should also be added to docs/internals/contributing/writing-code/coding-style.txt.

[Tim Graham]

Another phrasing to remove is "Checks that" / "Check that".

A ​PR is started by za.

[za]

@Mahendra Yadav: currently I'm working on ​PR 7439.

[Reinout van Rees]

Marked the ticket as "patch needs improvement", see ​https://github.com/django/django/pull/7439#issuecomment-258603274

Mostly "just" re-running regexes with similar occurrences.

[Tim Graham <timograham@…>]

In 321e94fa:

Refs #27392 -- Removed "Tests that", "Ensures that", etc. from test docstrings.

[Tim Graham]

While reviewing the patch, I came across additional phrases that could be removed. There are probably more variants as well:

Tests if
Ensured that
Tests whether
Ensure
Make sure
Confirms that
Makes sure
Check to
Check we
Ensure the
Test

[Barun Parruck]

Hi! I was thinking of working on this. Two questions --

1. Do you want something like "Tests adding a FK constraint for an existing column" to go to "Adds foreign key constraint to an existing column", or "Foreign key constraint can be added to an existing column".

2. Secondly, could I create a simple pull request with only ~10 or so fixes to get the hang of the development workflow? (To make sure I'm doing the right thing!)

[Tim Graham]

For the "Tests adding" case, I'd just chop "Test". It's okay to make a work-in-progress PR.

[Eric Linden]

There are plenty of 'regression test for #xxx' and similar things. Should those be left as is, modified to keep the reference to the fact that it's a regression test, or just get rid of that type of statement?

[Tim Graham]

We use a ticket reference for obscure issues that can't be described in a docstring. Some places could be cleaned up but I wouldn't make it part of this ticket. It would be better to wait a bit before working on this, as described in an open ​PR.

[Theofanis Despoudis]

Is this Ticket worked on?. The PR listed here ​https://github.com/django/django/pull/7777 is closed. Is it ok to get correspondence for this because I would like to contribute?

[Tim Graham]

This is continued in ​PR #8648.

[Theofanis Despoudis]

Replying to Tim Graham:

This is continued in ​PR #8648.

Thank you, Tim.

Is it being worked on as it has been more than a month since the last updates and more conflicts may appear?

[Theofanis Despoudis]

Assigned to me as per
​https://github.com/django/django/pull/8648#issuecomment-315991751

[Theofanis Despoudis]

While working on the issue I also notices there are more edge cases:

• Tests the
• Test If
• Test the
• Ensures

plus more variations of Prefixes

I/m using a slightly more informative grep search

grep -rInw "Test" *

That prints whole words and line numbers

I will work on it this weekend

[za]

I'll try to work on this within this week.

[Tim Graham]

This ticket has too many poor attempts and is causing more distraction than it's worth.