Opened 15 years ago

Closed 14 years ago

#2631 enhancement closed fixed (fixed)

Update coding standard to indicate preference for TestCase methods which being with "assert" and which do not have an underscore in their name

Reported by: Jean-Paul Calderone Owned by:
Priority: normal Milestone:
Component: core Keywords: documentation
Cc: Ralph Meijer Branch: branches/test-standard-2631
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

TestCase offers failUnless as well as assertTrue and a number of other similar pairings. The coding standard should indicate that assertTrue is the preferred spelling and the fail spellings should be avoided where possible. It should also express the preference of assertTrue over assert_.

Change History (16)

comment:1 Changed 15 years ago by Jean-Paul Calderone

Oh. It should express the preference of the singular spelling over the plural spelling as well.

comment:2 Changed 15 years ago by Jonathan Lange

I think this is a step in the right direction, but:

  • I prefer failUnless over assertTrue.
  • Twisted is already choc-full of coding standard violations. Adding this rule will increase the number of violations. It would be good to:
    • Fix the existing violations of the existing coding standards.
    • Have an automated tool for checking standards compliance.

comment:3 Changed 15 years ago by Jean-Paul Calderone

I used to prefer failUnless over assertTrue, but radix pointed out that assertTrue is preferable for technical reasons (it has "assert" in it, so it's easily searchable, it's consistent with the other assert functions).

I'm all for fixing things. I'm not sure if you're suggesting a specific action in this case, though, and we shouldn't wait for the rest of the code base to catch up before applying this standard to new contributions.

I'm also all for automated tools, but I'm not going to implement it, and we shouldn't wait for someone else to before enacting this for new code.

comment:4 Changed 15 years ago by Antoine Pitrou

My two cents... I've always found failUnless makes code less easily understandable, because it's a double negative, while assertTrue is straightforward.

(while "unless" is common is the Perl world, it is not in the Python world and for good reasons IMHO)

comment:5 Changed 15 years ago by Glyph

I think that making the coding standard more public and accessible is an important part of this ticket. We've been developing an informal and ad-hoc definition of "good" code, and this emerging organizational knowledge needs to be presented to people at a useful time in the process of their contribution of code, which is before a review informs them of it after the fact (or, for that matter, when the reviewer informs them, a place to put a hyperlink to would be nice).

I've definitely done a few reviews which could have been much more productive if the contributor were aware of the coding standard beforehand, and I feel like the fault lies entirely with us at this point. Although, probably nobody will read it when it is prominently linked either, at least we'll have the moral high ground :).

comment:6 Changed 15 years ago by Ralph Meijer

Cc: Ralph Meijer added

comment:7 Changed 14 years ago by Jean-Paul Calderone

author: exarkun
Branch: branches/dns-str-eq-2631

(In [22378]) Branching to 'dns-str-eq-2631'

comment:8 Changed 14 years ago by Jean-Paul Calderone

author: exarkun
Branch: branches/dns-str-eq-2631

Just kidding. :/

comment:9 Changed 14 years ago by Jean-Paul Calderone

author: exarkun
Branch: branches/dns-str-eq-2631-2

(In [23923]) Branching to 'dns-str-eq-2631-2'

comment:10 Changed 14 years ago by Jean-Paul Calderone

(In [23923]) Branching to 'dns-str-eq-2631-2'

comment:11 Changed 14 years ago by Jean-Paul Calderone

author: exarkun
Branch: branches/dns-str-eq-2631-2

Crap, again, crap.

comment:12 Changed 14 years ago by Jean-Paul Calderone

author: exarkun
Branch: branches/test-standard-2631

(In [23985]) Branching to 'test-standard-2631'

comment:13 Changed 14 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jean-Paul Calderone deleted

Some changes made. These documents all need a lot of work. :/

comment:14 Changed 14 years ago by therve

Keywords: review removed
Owner: set to Jean-Paul Calderone

Nice improvements. Please merge.

comment:15 Changed 14 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [24019]) Merge test-standard-2631

Author: exarkun Reviewer: therve Fixes: #2631

Update the testing documentation to mention recent additions to trial (addCleanup, the -R option is implicit now), note that tests go into packages related to them, change the sample test methods to use the test_ naming convention, and point out that assertFoo is prefered over failUnlessFoo.

comment:16 Changed 11 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.