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
_______________________________________________
py-dev mailing list
py-dev@codespeak.net
http://codespeak.net/mailman/listinfo/py-dev
