Hi Floris, On Sun, Sep 05, 2010 at 22:42 +0100, Floris Bruynooghe wrote: > Hello Holger > > On Thu, Sep 02, 2010 at 11:27:22AM +0200, holger krekel wrote: > > 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. > > I wouldn't want to make this more difficult or incompatible though, so > I'd be interested in how you where thinking of doing this. I guess > this would be exposing the various .visit_*() methods in > _assertionnew.py in some way? If so I think there's also value in the > more simple pytest_assert_compare(op, left, right) I'm proposing here > (and others like "in", "is" etc) as they don't require having any AST > knowledge to implement, at a cost of slightly less flexibility.
Sure. The eventual callback that is to receive AST-nodes can remain internal and there is nothing from keeping us to offer both levels of hooks (the default impl for the low-level hook can just call the higher level ones). > > On Mon, Aug 16, 2010 at 14:51 +0100, Floris Bruynooghe wrote: > > > 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. > > I've made this hook pytest_assert_compare(op, left, right) now, as > I've found py.io.saferepr() which seems to do a good job. ok. > Not sure what you're referring too with the "history" comment. If you > mean that as soon as a specialised hook is found the previous > explanations (e.g. from Assert AST node) get lost I'd like to > disagree. It seems reasonable for the new hooks to just provide a > more detailed/specialised explanation of a particular part of the > failure rather then replace the entire explanation. Otherwise you > might also lose detail. Hum, ok. Let's see how this plays out with real failures. > > (I guess you are aware that any pytest-hook implementation can always > > choose to accept less than the available arguments). > > I wasn't actually, neat. > > > > 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. > > ok > > > 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. > > To me it seems more logical to add a separate hook for each .visit_*() > method rather then put multiple together. But maybe that seems > artificial from a user point of view? I guess I prefer "pytest_assert_binop(op, val1, val2)" but keeping strictly to the visit relation also has merits. I guess my preference also comes from trying to avoid introducing too many hooks. > > Floris, i'd very much like to start using/having improved assertion > > representations. Happy to review patches or forks for inclusion. > > I've attached a new patch in which I attempt to use the hook system > and added a pytest_assert_comare(op, left, right) hook. I must admit > I don't fully understand the plugin/hook system so hope I did it > right [0]. Again I've not concentrated on the actual specific > comparisons, rather would like to get a reasonable idea of how good > the general approach is. Your hook implementation looks fine and [0] is correct. Please feel free to ask any questions regarding hook machinery here on the list. Are you aware of (the rather minimal) documentation here? http://codespeak.net/py/dist/test/customize.html#important-py-test-hooks > If you like this version I can create a fork on bitbucket and start > working on more/better hook implementations. Great, please do so. Two review notes regarding your patch already: * please try to avoid global imports in plugins, rather only import e.g. difflib/pprint where you actually use it. This is to try to keep interactive startup-time minimal, particularly wrt builtin plugins. * i suggest to put tests to testing/plugin/test_pytest_assertion.py (rather than testing/code/...) especially since the tested functionality is contained in the pytest_assertion.py plugin. For direct unit tests you may also directly do "from py._plugin.pytest_assertion import XYZ". best, holger > Regards > Floris > > > [0] I assume that py.test.config.hook.pytest_assert_compare is the > function to call and that it returns a list with the results of > each such function found, with the first element being the most > "specific" result. But I just figured that out using trial and > error rather then understand the plugin system. > > -- > Debian GNU/Linux -- The Power of Freedom > www.debian.org | www.gnu.org | www.kernel.org > diff --git a/py/_code/_assertionnew.py b/py/_code/_assertionnew.py > --- a/py/_code/_assertionnew.py > +++ b/py/_code/_assertionnew.py > @@ -162,10 +162,7 @@ class DebugInterpreter(ast.NodeVisitor): > def visit_Compare(self, comp): > left = comp.left > left_explanation, left_result = self.visit(left) > - got_result = False > for op, next_op in zip(comp.ops, comp.comparators): > - if got_result and not result: > - break > next_explanation, next_result = self.visit(next_op) > op_symbol = operator_map[op.__class__] > explanation = "%s %s %s" % (left_explanation, op_symbol, > @@ -177,9 +174,16 @@ class DebugInterpreter(ast.NodeVisitor): > __exprinfo_right=next_result) > except Exception: > raise Failure(explanation) > - else: > - got_result = True > + if not result: > + break > left_explanation, left_result = next_explanation, next_result > + hook_result = py.test.config.hook.pytest_assert_compare( > + op=op_symbol, left=left_result, right=next_result) > + if hook_result: > + for new_expl in hook_result: > + if result: > + explanation = '\n~'.join(new_expl) > + break > return explanation, result > > def visit_BoolOp(self, boolop): > diff --git a/py/_code/assertion.py b/py/_code/assertion.py > --- a/py/_code/assertion.py > +++ b/py/_code/assertion.py > @@ -5,12 +5,20 @@ BuiltinAssertionError = py.builtin.built > > > def _format_explanation(explanation): > - # uck! See CallFunc for where \n{ and \n} escape sequences are used > + """This formats an explanation > + > + Normally all embedded newlines are escaped, however there are > + three exceptions: \n{, \n} and \n~. The first two are intended > + cover nested explanations, see function and attribute explanations > + for examples (.visit_Call(), visit_Attribute()). The last one is > + for when one explanation needs to span multiple lines, e.g. when > + displaying diffs. > + """ > raw_lines = (explanation or '').split('\n') > - # escape newlines not followed by { and } > + # escape newlines not followed by {, } and ~ > lines = [raw_lines[0]] > for l in raw_lines[1:]: > - if l.startswith('{') or l.startswith('}'): > + if l.startswith('{') or l.startswith('}') or l.startswith('~'): > lines.append(l) > else: > lines[-1] += '\\n' + l > @@ -28,11 +36,14 @@ def _format_explanation(explanation): > stackcnt[-1] += 1 > stackcnt.append(0) > result.append(' +' + ' '*(len(stack)-1) + s + line[1:]) > - else: > + elif line.startswith('}'): > assert line.startswith('}') > stack.pop() > stackcnt.pop() > result[stack[-1]] += line[1:] > + else: > + assert line.startswith('~') > + result.append(' '*len(stack) + line[1:]) > assert len(stack) == 1 > return '\n'.join(result) > > diff --git a/py/_plugin/hookspec.py b/py/_plugin/hookspec.py > --- a/py/_plugin/hookspec.py > +++ b/py/_plugin/hookspec.py > @@ -124,6 +124,19 @@ def pytest_sessionfinish(session, exitst > """ whole test run finishes. """ > > # ------------------------------------------------------------------------- > +# hooks for customising the assert methods > +# ------------------------------------------------------------------------- > + > +def pytest_assert_compare(op, left, right): > + """Customise compare assertion > + > + Return None or an empty list for no custom compare, otherwise > + return a list of strings. The strings will be joined by newlines > + but any newlines *in* as string will be escaped. Note that all > + but the first line will be indented sligthly. > + """ > + > +# ------------------------------------------------------------------------- > # hooks for influencing reporting (invoked from pytest_terminal) > # ------------------------------------------------------------------------- > > diff --git a/py/_plugin/pytest_assertion.py b/py/_plugin/pytest_assertion.py > --- a/py/_plugin/pytest_assertion.py > +++ b/py/_plugin/pytest_assertion.py > @@ -1,3 +1,6 @@ > +import difflib > +import pprint > + > import py > import sys > > @@ -26,3 +29,49 @@ def warn_about_missing_assertion(): > else: > py.std.warnings.warn("Assertions are turned off!" > " (are you using python -O?)") > + > + > +def pytest_assert_compare(op, left, right): > + """Make a specialised explanation for comapare equal""" > + if op != '==' or type(left) != type(right): > + return None > + explanation = [] > + left_repr = py.io.saferepr(left, maxsize=30) > + right_repr = py.io.saferepr(right, maxsize=30) > + explanation += ['%s == %s' % (left_repr, right_repr)] > + issquence = lambda x: isinstance(x, (list, tuple)) > + istext = lambda x: isinstance(x, basestring) > + isdict = lambda x: isinstance(x, dict) > + if istext(left): > + explanation += [line.strip('\n') for line in > + difflib.ndiff(left.splitlines(), right.splitlines())] > + elif issquence(left): > + explanation += _compare_eq_sequence(left, right) > + elif isdict(left): > + explanation += _pprint_diff(left, right) > + else: > + return None # No specialised knowledge > + return explanation > + > + > +def _compare_eq_sequence(left, right): > + explanation = [] > + for i in xrange(min(len(left), len(right))): > + if left[i] != right[i]: > + explanation += ['First differing item %s: %s != %s' % > + (i, left[i], right[i])] > + break > + if len(left) > len(right): > + explanation += ['Left contains more items, ' > + 'first extra item: %s' % left[len(right)]] > + elif len(left) < len(right): > + explanation += ['Right contains more items, ' > + 'first extra item: %s' % right[len(right)]] > + return explanation + _pprint_diff(left, right) > + > + > +def _pprint_diff(left, right): > + """Make explanation using pprint and difflib""" > + return [line.strip('\n') for line in > + difflib.ndiff(pprint.pformat(left).splitlines(), > + pprint.pformat(right).splitlines())] > diff --git a/testing/code/test_assertionnew.py > b/testing/code/test_assertionnew.py > new file mode 100644 > --- /dev/null > +++ b/testing/code/test_assertionnew.py > @@ -0,0 +1,74 @@ > +import sys > + > +import py > +from py._code._assertionnew import interpret > + > + > +def getframe(): > + """Return the frame of the caller as a py.code.Frame object""" > + return py.code.Frame(sys._getframe(1)) > + > + > +def setup_module(mod): > + py.code.patch_builtins(assertion=True, compile=False) > + > + > +def teardown_module(mod): > + py.code.unpatch_builtins(assertion=True, compile=False) > + > + > +def test_assert_simple(): > + # Simply test that this way of testing works > + a = 0 > + b = 1 > + r = interpret('assert a == b', getframe()) > + assert r == 'assert 0 == 1' > + > + > +def test_assert_list(): > + r = interpret('assert [0, 1] == [0, 2]', getframe()) > + msg = ('assert [0, 1] == [0, 2]\n' > + ' First differing item 1: 1 != 2\n' > + ' - [0, 1]\n' > + ' ? ^\n' > + ' + [0, 2]\n' > + ' ? ^') > + print r > + assert r == msg > + > + > +def test_assert_string(): > + r = interpret('assert "foo and bar" == "foo or bar"', getframe()) > + msg = ("assert 'foo and bar' == 'foo or bar'\n" > + " - foo and bar\n" > + " ? ^^^\n" > + " + foo or bar\n" > + " ? ^^") > + print r > + assert r == msg > + > + > +def test_assert_multiline_string(): > + a = 'foo\nand bar\nbaz' > + b = 'foo\nor bar\nbaz' > + r = interpret('assert a == b', getframe()) > + msg = ("assert 'foo\\nand bar\\nbaz' == 'foo\\nor bar\\nbaz'\n" > + ' foo\n' > + ' - and bar\n' > + ' + or bar\n' > + ' baz') > + print r > + assert r == msg > + > + > +def test_assert_dict(): > + a = {'a': 0, 'b': 1} > + b = {'a': 0, 'c': 2} > + r = interpret('assert a == b', getframe()) > + msg = ("assert {'a': 0, 'b': 1} == {'a': 0, 'c': 2}\n" > + " - {'a': 0, 'b': 1}\n" > + " ? ^ ^\n" > + " + {'a': 0, 'c': 2}\n" > + " ? ^ ^") > + print r > + assert r == msg > _______________________________________________ > py-dev mailing list > py-dev@codespeak.net > http://codespeak.net/mailman/listinfo/py-dev -- _______________________________________________ py-dev mailing list py-dev@codespeak.net http://codespeak.net/mailman/listinfo/py-dev
