Yelp / Testify

A more pythonic testing framework.
Other
308 stars 67 forks source link

stringdiffer does not handle equality between unicode and ascii strings correctly #127

Closed att14 closed 11 years ago

att14 commented 11 years ago

I have a failure that shows this problem, but the diff is massive and not really of any worth. The problem arises when something like this happens:

Diff:
l: {<>'key': <'value'>}
r: {<u>'key': <'wrong'>}

The correct behavior is seen if the values of the dictionaries above are the same.

bukzor commented 11 years ago

What's the expected behavior here? It's pointing out that the str and unicode repr's are different, but that's not why the test failed.

baris commented 11 years ago

What should be the correct behavior in here? This is what we have currently and I think it 's the expected behavior:

In [3]: T.assert_equal({'key': 'value'}, {u'key': 'value'})

In [4]: T.assert_equal({'key': 'value'}, {u'key': 'wrong'})
---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
<ipython-input-4-2e4bebfdc03e> in <module>()
----> 1 T.assert_equal({'key': 'value'}, {u'key': 'wrong'})

/usr/local/lib/python2.7/dist-packages/testify/assertions.pyc in assert_equal(lval, rval, message)
    240         assert lval == rval, \
    241             "assertion failed: l == r\nl: %r\nr: %r\n\n%s" % \
--> 242                 (lval, rval, _diff_message(lval, rval))
    243 
    244 assert_equals = assert_equal

AssertionError: assertion failed: l == r
l: {'key': 'value'}
r: {u'key': 'wrong'}

Diff:
l: {<>'key': '<value>'}
r: {<u>'key': '<wrong>'}

In [5]: 
bukzor commented 11 years ago

Those keys are equal under all encodings, and in fact have the same hash. They're equal values for all intents and purposes.

If you want to assert about the types of the keys, you're using the wrong assert function.

baris commented 11 years ago

Closing this issue. Looks like Testify is doing the right thing in this case. @att14, please re-open and describe the expected behavior if you do not agree.