This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author gvanrossum
Recipients benjamin.peterson, giampaolo.rodola, gpolo, gregory.p.smith, gvanrossum, michael.foord, michael.foord, pitrou, pupeno, purcell, rhettinger, skip.montanaro
Date 2009-03-31.15:01:08
SpamBayes Score 6.752596e-07
Marked as misclassified No
Message-id <00151750d9b8f174a104666b7729@google.com>
In-reply-to
Content
Hi Greg, I've mostly reviewed for style...

http://codereview.appspot.com/32080/diff/13/1006
File Doc/library/unittest.rst (right):

http://codereview.appspot.com/32080/diff/13/1006#newcode611
Line 611: assertTrue(expr[, msg])
Make assertTrue the first/preferred/recommended spelling.

http://codereview.appspot.com/32080/diff/13/1006#newcode623
Line 623: :const:`None`.  Note that using :meth:`failUnlessEqual`
improves upon
failUnlessEqal -> assertEqual

http://codereview.appspot.com/32080/diff/13/1006#newcode624
Line 624: doing the comparison as the first parameter to
:meth:`failUnless`: the
failUnless -> assertTrue

(This kind of change may have to be done throughout.)

http://codereview.appspot.com/32080/diff/13/1010
File Lib/test/test_unittest.py (right):

http://codereview.appspot.com/32080/diff/13/1010#newcode75
Line 75: self.fail("%s and %s do not hash equal" % (obj_1, obj_2))
maybe better %r and %r ?

http://codereview.appspot.com/32080/diff/13/1010#newcode84
Line 84: self.fail("%s and %s hash equal, but shouldn't" %
ditto?

http://codereview.appspot.com/32080/diff/13/1010#newcode2313
Line 2313: self.assertRaises(self.failureException, self.assertIn,
'elephant', animals)
lin too long

http://codereview.appspot.com/32080/diff/13/1010#newcode2317
Line 2317: self.assertRaises(self.failureException, self.assertNotIn,
'cow', animals)
linr too long

http://codereview.appspot.com/32080/diff/13/1007
File Lib/unittest.py (right):

http://codereview.appspot.com/32080/diff/13/1007#newcode257
Line 257: class AssertRaisesContext(object):
While you're at it, can you add a docstring?

http://codereview.appspot.com/32080/diff/13/1007#newcode332
Line 332: # Map types to custom assertEquals functions that will compare
assertEquals -> assertEqual

http://codereview.appspot.com/32080/diff/13/1007#newcode512
Line 512: # should use their type specific assertSpamEquals method to
compare
assertSpamEqual

http://codereview.appspot.com/32080/diff/13/1007#newcode521
Line 521: def _baseAssertEquals(self, first, second, msg=None):
Mind dropping the trailing 's'?

http://codereview.appspot.com/32080/diff/13/1007#newcode526
Line 526: def failUnlessEqual(self, first, second, msg=None):
We had talked about making the 'def' define the recommended name, e.g.
assertNotEqual, and using aliases to keep the other names.  Do you want
to do that at the same time as this change or in a separate one?

http://codereview.appspot.com/32080/diff/13/1007#newcode581
Line 581: def assertSequenceEquals(self, seq1, seq2, msg=None,
seq_type=None):
Drop the trailing 's' in the name?

http://codereview.appspot.com/32080
History
Date User Action Args
2009-03-31 15:01:10gvanrossumsetrecipients: + gvanrossum, skip.montanaro, rhettinger, purcell, pitrou, giampaolo.rodola, pupeno, benjamin.peterson, gpolo, michael.foord
2009-03-31 15:01:09gvanrossumlinkissue2578 messages
2009-03-31 15:01:08gvanrossumcreate