Code review comment for lp:~henninge/launchpad/devel-766955-fix-forwardCheckAuthenticated

Am 28.04.2011 15:59, schrieb Gavin Panella:
> Review: Approve

Thank you very much for your review. ;-)

>
> self.assertEqual(
> (1, 1),
> (adapter.checkPermissionIsRegistered.call_count,
> next_adapter.checkAuthenticated.call_count))
>
> because then the error messages will make sense.

Funny enough, that is exactly the code (down to the indentation) that I had
before I added assertVectorEqual. Also, it will produce the exact same output
as assertVectorEqual. I think both this solution and assertVectorEqual are
superior to two consecutive asserts because both expected/observed pairs will
be asserted whereas with the two asserts, it is not clear if the second assert
would fail, too, once the first one fails. This leads to iterative bug fixing
("Ok, we got the first one passing but now the second one if failing.")
Vector-style assertion gives a more complete picture of the situation.

The reason I introduced assertVectorEqual is that it is clear that the error
output will be a vector of observed values compared to a vector of expected
values. It's just a matter of rotating the vector from horizontal to vertical
orientation and comparing that to the code to find out which test failed.

In short: I see a far greater benefit in using assertVectorEqual that is not
outweighed by the extra work of rotating a vector in your head.

A more complex implementation could alleviate the concern about the failure
message but that is beyond the scope of this branch.

Cheers,
Henning