Menu

#134 No test case for Text.shortrepr with long string.

closed-fixed
nobody
None
5
2010-03-26
2010-03-24
No

There is no test case for when Text.shortrepr is called on a Text node with more than 20 characters. If there was, I believe you would see an error as my test code for inline nested markup does exercise this possibility and generates an error in the if clause. I believe the same holds true for the __repr__ function when called on an object with over 70 characters.

I would write the test case myself but I'm not sure how to write test code for something so low-level, though a unit test would definitely be appropriate. None the less, I shall try to work on creating a test case myself, but any help would be greatly appreciated. Once this test case is constructed, if it fails, as I predict it will, I think it would be appropriate to migrate this bug to fixing the error rather than creating a new issue since I've already said I believe the code is in error based on exercising it in my esoteric tests.

Discussion

  • Jeffrey C. Jacobs

    Okay, that's wasn't so hard -- I forgot I'd written node test cases before; so there you have it: this patch to the test_nodes.py test cases demonstrates the flaw in the current text node __repr__ and shortrepr functions. I need help to fix the issue though as I don't understand why the code is failing.

     
  • Jeffrey C. Jacobs

    Fixed a couple of typos in the test case; also deduced the issue: when calling reprunicode.__repr__ with a parameter that is not of type reprunicode you will receive the error described. I have not tried Python 3.x but I assume since you don't need to strip the u from the output string like you do in the 2.x version that the code works in 3.x and since I don't think this logic: an instance function acting like a free / static function or the unicode string being promoted to a reprunicode string -- has been implemented in any of the 2.x versions. Therefore, I don't yet see a clear solution; stay tuned.

     
  • Jeffrey C. Jacobs

    A fix for the issue listed; including test case

     
  • Jeffrey C. Jacobs

    Okay, there y'all have it! I've written a patch to solve this issue that I think should work for both Python 3.x and Python 2.x >= 2.3. Enjoy, I hope!

     
  • Jeffrey C. Jacobs

    FYI, on my installation:

    2.5.1 (r251:54863, Feb 6 2009, 19:02:12)
    [GCC 4.0.1 (Apple Inc. build 5465)]

    (The version installed with Mac OS X 10.5 (Leopard))

    I see the following exception when I try to run the "Mary had a little lamb..." test:

    ERROR: test_longrepr (test_nodes.TextTests)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
    File "/Users/darklord/Documents/Remote/docutils/test/test_nodes.py", line 59, in test_longrepr
    self.assertEquals(repr(self.longtext), r"<#text: Mary had a "
    File "/Users/darklord/Documents/Remote/docutils/docutils/nodes.py", line 341, in __repr__
    data = reprunicode.__repr__(self[:64] + ' ...')
    TypeError: unbound method __repr__() must be called with reprunicode instance as first argument (got unicode instance instead)

    =================

    As you can see from the error, I believe the problem is that in this version of python, the call of the form reprunicode.__repr__(self) passes because self is derived from reprunicode so there is no casting involved in the base class call. But when a string is added to the result of an indexing operation (which is allowed because nodes.Text is derived from unicode), this is coerced automatically to unicode type, NOT to reprunicode type. Now, in some versions of python this will be okay because the call to reprunicode.__repr__ will automatically coerce the resultant unicode string into a reprunicode type, but not, apparently in version 2.5.1 as packaged with Mac OS X Leopard. Normally, I'd say upgrade but the thing is, when you're talking bundled software that is built into the OS framework, this isn't so easy; you and I can do it, but not an average user. That, coupled with the fact that all we're doing is forcing a cast from the unicode string generated into a reprunicode object and just calling the __repr__ method directly (we could also call it via the repr keyword since the object is now of type reprunicode, not nodes.Text) and since this isn't all that much glue code -- if the object is already a unicode string, all that's happening there is a reference count increment, and if it's before 3.x, it's still likely a reference count increment since reprunicode is a subclass of and thus container for unicode object anyway. So, even if you can't reproduce this issue with your version of Python, the solution should not adversely effect any version of python to any great extent and has the added bonus of fixing a problem with the Mac OS X Leopard default install.

     
  • Günter Milde

    Günter Milde - 2010-03-26

    Fixed; thanks for the bug report.

    You can download a current snapshot from:
    http://docutils.sf.net/docutils-snapshot.tgz

     
  • Günter Milde

    Günter Milde - 2010-03-26
    • status: open --> closed-fixed
     

Log in to post a comment.