[Python-Dev] r87389 - in python/branches/py3k: Doc/library/unittest.rst Lib/unittest/case.py Misc/NEWS (original) (raw)
Michael Foord fuzzyman at voidspace.org.uk
Tue Dec 21 13:17:46 CET 2010
- Previous message: [Python-Dev] r87389 - in python/branches/py3k: Doc/library/unittest.rst Lib/unittest/case.py Misc/NEWS
- Next message: [Python-Dev] r87389 - in python/branches/py3k: Doc/library/unittest.rst Lib/unittest/case.py Misc/NEWS
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On 21/12/2010 01:57, Nick Coghlan wrote:
On Tue, Dec 21, 2010 at 1:31 AM, Antoine Pitrou<solipsis at pitrou.net> wrote:
Diffing is completely an implementation detail of how the failure messages are generated. The important thing is that failure messages make sense with respect to actual result and expected result. Which, again, they don't. Let's see:
self.assertEqual(actual, expected) AssertionError: 'a\nb\nc\ne\n' != 'a\nb\nc\nd\n' a b c - e + d The diff shows "expected - actual", but it would be logical (in your own logic) to display "actual - expected". The whole issue disappears if you drop this idea of naming the arguments "actual" and "expected". To make this a bit clearer...
class Ex(ut.TestCase): ... def demo(self): ... self.assertEqual("actual", "expected") ... Ex("demo").demo() Traceback (most recent call last): AssertionError: 'actual' != 'expected' - actual + expected For the actual/expected terminology the diff is the wrong way around (as of 3.2b1, anyway).
The recent commit that sparked the controversy was supposed to ensure that all the asserts were documented consistently and worked as per the documentation. The error above is from assertMultiLineEqual.
assertListEqual has the same issue:
t.assertListEqual([1], [2]) Traceback (most recent call last): ... AssertionError: Lists differ: [1] != [2]
First differing element 0: 1 2
- [1]
- [2]
Interestingly assertSetEqual already uses the first/second symmetric wording:
t.assertSetEqual({1}, {2}) ... AssertionError: Items in the first set but not the second: 1 Items in the second set but not the first: 2
My own +1 goes to keeping the actual/expected terminology (and ordering) and adjusting the diffs accordingly (with a header noting that the diff is old=expected, new=actual).
Well we don't have consensus. Whatever we do we need to be consistent, and in the absence of an agreement about a change we should at least make all the behaviour and documentation consistent.
From this discussion and the discussion on the issue tracker:
Myself, Nick Coghlan and Ezio Melotti prefer (actual, expected) Raymond like (actual, expected) but would be happy with symmetrical diffs Guido prefers the (actual, expected) ordering but prefers diffs to show the other way round R David Murray agreed with Guido Terry Reedy liked the change Glenn Linderman wants (actual, expected) and diffing to follow that Ron Adam ditto
Symmetrical diffs (element in first not in second, element in second not in first) solves the problem without imposing an order on the arguments. Actually unittest has used (first, second) to refer to the arguments to asserts pretty much since its inception. Losing the (actual, expected) terminology is a cost of this but unittest hasn't emphasised this terminology in the past (as I thought it had).
This won't work for diffing strings (assertMultiLineEqual) which use difflib and needs a direction for the diff. As above it is currently the wrong way round for (actual, expected).
The other alternative is to make them consistent and follow Nick's suggestion adding the header note to the diffs that old=expected, new=actual.
assertRaises() is an exception to the general actual/expected pattern, but that asymmetry is forced by the ability to pass arbitrary positional arguments to the function being tested (which later proved useful for the context manager form as well). The (actual, expected) pattern matches the way almost everyone I've ever seen write if statements and asserts:
if x == 5: rather than if 5 == x:
assert x == 5 rather than assert 5 == x
It also matches functions like isinstance and issubclass.
On the other hand it doesn't match the way we report TypeErrors where we report "expected , got ".
All the best,
Michael Foord
Cheers, Nick.
--
May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html
- Previous message: [Python-Dev] r87389 - in python/branches/py3k: Doc/library/unittest.rst Lib/unittest/case.py Misc/NEWS
- Next message: [Python-Dev] r87389 - in python/branches/py3k: Doc/library/unittest.rst Lib/unittest/case.py Misc/NEWS
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]