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

Nicolas Trangez nicolas.trangez at scality.com
Wed Nov 26 14:28:43 UTC 2014


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).

Nicolas




More information about the OpenStack-dev mailing list