[openstack-dev] [nova] Proposal new hacking rules

Jay Pipes jaypipes at gmail.com
Wed Nov 26 14:33:01 UTC 2014


On 11/26/2014 09:28 AM, Nicolas Trangez wrote:
> On Wed, 2014-11-26 at 08:54 -0500, Jay Pipes wrote:
>> On 11/26/2014 06:20 AM, Nicolas Trangez wrote:
>>> On Mon, 2014-11-24 at 13:19 -0500, Jay Pipes wrote:
>>>> I think pointing out that the default failure
>>>> message for testtools.TestCase.assertEqual() uses the terms
>>>> "reference"
>>>> (expected) and "actual" is a reason why reviewers *should* ask patch
>>>> submitters to use (expected, actual) ordering.
>>>
>>> Is there any reason for this specific ordering? Not sure about others,
>>> but I tend to write equality comparisons like this
>>>
>>>       if var == 1:
>>>
>>> instead of
>>>
>>>       if 1 == var:
>>>
>>> (although I've seen the latter in C code before).
>>>
>>> This gives rise to
>>>
>>>       assert var == 1
>>>
>>> or, moving into `unittest` domain
>>>
>>>       assertEqual(var, 1)
>>>
>>> reading it as 'Assert `var` equals 1', which makes me wonder why the
>>> `assertEqual` API is defined the other way around (unlike how I'd write
>>> any other equality check).
>>
>> It's not about an equality condition.
>>
>> It's about the message that is produced by
>> testtools.TestCase.assertEqual(), and the helpfulness of that message
>> when the order of the arguments is reversed.
>
> I'm aware of that. I was just wondering whether there's a particular
> reason the ordering (and as a result of that the error message) was
> chosen as it is.
>
> I'd rather design the API as `assertEqual(value, expected)`, and let the
> message indeed say 'Expected ..., but got ...' (and using the argument
> values accordingly).

I think you'd have the same problem, no? People would still need to get 
the order of the arguments "correct".

-jay



More information about the OpenStack-dev mailing list