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

Matthew Treinish mtreinish at kortar.org
Fri Nov 21 22:23:28 UTC 2014


On Fri, Nov 21, 2014 at 04:15:07PM -0500, Sean Dague wrote:
> On 11/21/2014 01:52 PM, Matthew Treinish wrote:
> > On Fri, Nov 21, 2014 at 07:15:49PM +0100, jordan pittier wrote:
> >> Hey,
> >> I am not a Nova developer but I still have an opinion.
> >>
> >>> Using boolean assertions
> >> I like what you propose. We should use and enforce the assert* that best matches the intention. It's about semantic and the more precise we are, the better.
> >>
> >>> Using same order of arguments in equality assertions
> >> Why not. But I don't know how we can write a Hacking rule for this. So you may fix all the occurrences for this now, but it might get back in the future.
> > 
> > Ok I'll bite, besides the enforceability issue which you pointed out, it just
> > doesn't make any sense, you're asserting 2 things are equal: (A == B) == (B == A)
> > and I honestly feel that it goes beyond nitpicking because of that. 
> > 
> > It's also a fallacy that there will always be an observed value and an
> > expected value. For example:
> > 
> >   self.assertEqual(method_a(), method_b())
> > 
> > Which one is observed and which one is expected? I think this proposal is just
> > reading into the parameter names a bit too much.
> 
> If you are using assertEqual with 2 variable values... you are doing
> your test wrong.
> 
> I was originally in your camp. But honestly, the error message provided
> to the user does say expected and observed, and teaching everyone that
> you have to ignore the error message is a much harder thing to do than
> flip the code to conform to it, creating less confusion.
> 

Uhm, no it doesn't, the default error message is "A != B". [1][2][3] (both with
unittest and testtools) If the error message was like that, then sure saying
one way was right over the other would be fine, (assuming you didn't specify a
different error message) but, that's not what it does.


[1] https://github.com/testing-cabal/testtools/blob/master/testtools/testcase.py#L340
[2] https://github.com/testing-cabal/testtools/blob/master/testtools/matchers/_basic.py#L85 
[3] https://hg.python.org/cpython/file/301d62ef5c0b/Lib/unittest/case.py#l508
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20141121/4e26c913/attachment.pgp>


More information about the OpenStack-dev mailing list