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

Zane Bitter zbitter at redhat.com
Wed Nov 26 17:39:09 UTC 2014


On 26/11/14 09:33, Louis Taylor wrote:
> On Wed, Nov 26, 2014 at 08:54:35AM -0500, Jay Pipes wrote:
>> 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.
>>
>> This is especially true with large dict comparisons. If you get a message
>> like:
>>
>>   reference: <large_dict>
>>   actual: <large_dict>
>>
>> And the arguments are reversed, then you end up wasting time looking in the
>> test code instead of the real code for the thing that is different.
>>
>> Anyway, like I said, it's not something that we can write a simple hacking
>> check for, and therefore, it's not something that should have much time
>> spent on. But I do recommend that reviewers bring it up, especially if the
>> patch author has been inconsistent in their usage of (expected, actual) in
>> multiple assertEqual() calls in their patch.
>
> I think Nicolas's question was what made testtools choose this ordering. As far
> as I know, the python docs for unittest uses the opposite ordering. I think
> most people can see that the error messages involving 'reference' and 'actual'
> are useful, but maybe not the fact that in order to achieve them using
> testtools, you need to go against the norm for other testing frameworks.

The python docs for unittest mostly use 'first' and 'second' as the 
parameter names, and unittest doesn't distinguish between expected and 
actual in the default error messages.

That said, some of the newer assertions like assertDictEqual do use 
expected and actual, _in that order_, the same as testtools does.

The bottom line is that there are exactly two ways to do it, the entire 
world has now chosen one way and while I might otherwise have chosen 
differently for the same reasons as Nicolas, it would be absurd not to 
do it the same way.

That said, the entire discussion is moot because it can't be checked 
automatically.

cheers,
Zane.



More information about the OpenStack-dev mailing list