[py-dev] Advanced assert equal

[holger krekel]

Hi Floris,

sidenote: the original implementation was done by Armin Rigo 
and i am not much more accustomed to the assert-interpretation
code than you are.  The other guy having done bits there is Benjamin -
still haven't got hold of him on this issue and your mail. 
But I discussed with Benjamin introducing a more general API that
would have the AST-transformation call back into a generic
pytest-hook if an assertion fails.  So one could customize
"hasattr(x, y)" or "x in y" or "x == y == z" etc. and all
such representation code would move to the py/_plugin/pytest_assertion.py 
plugin. That being said going for just comparisons right now
is fine, let's not have more general thoughts hold us up.

On Mon, Aug 16, 2010 at 14:51 +0100, Floris Bruynooghe wrote:
> On Mon, Aug 16, 2010 at 01:29:13PM +0200, Ronny Pfannschmidt wrote:
> > On Mon, 2010-08-16 at 01:25 +0100, Floris Bruynooghe wrote: 
> > > The attached patch makes compare equal a special case and checks if
> > > the two arguments to it are both a list, text or dict and tries to
> > > generate a nicer explanation text for them.  The patch is more like a
> > > proof of concept then a final implementation, I may have done some
> > > very strange or silly things as I'm not familiar with the code.  It
> > > would be great to get feedback, both on the general concept and the
> > > actual implementation (particularly note the way I had to hack
> > > _format_explanation() in assertion.py).
> > 
> > I think it will be helpful to have some kind of hook to add more
> > explain-functions
> 
> Adding hooks should be possible, looking at all the .visit_*()
> functions it would seem only one hook is required, unless separate
> hooks for each rich compare operator are deemed useful.

no, a single one for all comparisons sound fine.

> The trickiest bit I think is how to produce multiline explanations.
> _format_explanation() concatenates all newlines.  Having only special
> cases for \n{ and \n} which is used by .visit_Call() to force nested
> and indented newlines.  In the patch I added \n== for this but a more
> general one is probably required, something like \n> or \n~ could work
> I guess.  This could be completely hidden from the hook however, by
> returning a list for each line to be printed, the caller of the hook
> would then join these up correctly so that _format_explanation() will
> add newlines and indentation correctly.  

Makes sense to have the caller deal with concatentation.  

> A possible api for the hook
> could be:
> 
> def pytest_assert_compare(op, left, right, left_repr, right_repr):
>     """Customise compare
> 
>     Return None for no custom compare, otherwise return a list of
>     strings.  The strings will be joined by newlines but any newlines
>     *in* a string will be escaped.
>     """
>     pass
> 
> I guess the reprs are not really necessary if there's an easy way to
> make them.  It's just that I used them in my original patch.

Hum, couldn't custom representations just mean that there is no 
"history" of where the object comes from? 

The hook looks otherwise good to me.  

> Another option is to encapsulate the arguments into an object that
> also knows how the builtin types and operators are compared, something
> like:
> 
> class CompareItem(object):
>     def __init__(self, op, left, right, ...):
>         self.op = op
>         self.left = left
>         self.right = right
> 	...
> 
>     def default(self):
>         if type(self.left) != type(self.right):
>             return None
>         if self.op == '==':
>             if isinstance(self.left, (list, tuple)):
>                 return self.sequence_equal()
>             elif isinstance(self.left, basestring):
>                 return self.string_equal()
>             ...
>         elif self.op == '!=':
>             ...
> 
>     def sequence_equal(self):
>         pass
> 
>     def string_equal(self):
>         pass
> 
>     ...
 
> This would allow pytest_assert_compare() to use those methods as part
> of the implementation.

I think the other style is more sensible because delegation to
"builtin representation styles" a) can happen via plugins and
the plugin mechanism b) we could pass a "testrepr" or "builtinrepr"
argument to the hook that helps to invoke helpful default machinery.
(I guess you are aware that any pytest-hook implementation can always
choose to accept less than the available arguments).

> There's also the question of who should truncate large amounts of data
> (e.g. screenfulls of diffs): the hook itself, the caller of the hook
> or _format_explanation()?  Probably one of the first two to get rid of
> the memory usage as soon as possible.

If a hook returns something we should (probably) not further do anything
with it in _format_explanation().  And making truncation the repsonsibility 
of the hook makes sense i think.  

> > In particular, cause there are many more build-in types to manage,
> > and i have at least 2 projects where custom compare-explain is helpfull
> > 
> > another wishlist item i see is the rest of rich compare
> > i.e. <, >, <=, >=, !=
> 
> Sure, all builtin types and operators should ideally be supported by
> default as best as possible.  I just started with some I wanted most.

I am not sure about the general use cases, from my side:

    x == y
    x != y
    x in y
   
are the interesting ones (with some of the objects being long
lists, long strings etc.). so some hook for a "binary" relation
makes sense, pytest_assert_binrepr(op) where op could be "==",
"in" or "is" etc.

Floris, i'd very much like to start using/having improved assertion
representations.  Happy to review patches or forks for inclusion.

best,
holger