Re: Something wrong about AssertRaisesRegexp and assertFieldOutput

2011-04-11 Thread Julien Phalip
On Apr 12, 2:00 pm, Karen Tracey  wrote:
> Adding re.escape in that function does break some tests, from a quick look
> it seems mostly ones that are raising multiple errors but only one has been
> listed by the caller of that utility function. It isn't clear to me from the
> docstring of that utility function how it is supposed to work in this
> case...is the caller required to be passing in all errors or is the function
> supposed to find it acceptable if the raised errors include the one that is
> passed in? I don't know.

Thanks for looking into this, Karen. I agree that the API for
assertFieldOutput is quite confusing. Since assertRaisesRegexp does a
search (and not a match), I guess it means that providing one of the
errors is enough to pass the test. However, regardless of that, from
an end-user perspective it doesn't really make sense that it uses
regular expressions at all. I tend to think that assertFieldOutput
shouldn't use assertRaisesRegexp but either use the good old
assertRaises instead.

I think I may have found a fix for this and posted a patch to a new
ticket: http://code.djangoproject.com/ticket/15805

Thank you,

Julien

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Something wrong about AssertRaisesRegexp and assertFieldOutput

2011-04-11 Thread Karen Tracey
On Mon, Apr 11, 2011 at 10:17 PM, Julien Phalip  wrote:

> I'm not sure how to properly fix this or where the core issue is. Am I
> missing something or is it worth opening a ticket?
>

I think there's definitely a problem with that assertFieldOutput utility
function. That 2nd parameter to assertRaisesRegexp is supposed to be a
regular expression object or string containing a regular expression (
http://docs.python.org/library/unittest.html#unittest.TestCase.assertRaisesRegexp).
If we're going to be passing in arbitrary error messages, then re.escape()
should be used on them to ensure that any special characters in those
messages are not mistakenly interpreted as part of a regular expression. The
unexpected results you mention are explained when you consider how the
special characters (e.g. []) in the strings affect the resulting regexp.

Adding re.escape in that function does break some tests, from a quick look
it seems mostly ones that are raising multiple errors but only one has been
listed by the caller of that utility function. It isn't clear to me from the
docstring of that utility function how it is supposed to work in this
case...is the caller required to be passing in all errors or is the function
supposed to find it acceptable if the raised errors include the one that is
passed in? I don't know.

Karen

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.