Re: [PATCH] Handle special characters in Junit report

On Fri, 2009-12-04 at 10:37 +0100, Branko Čibej wrote:
> Bhuvaneswaran A wrote:
> > The failure message for few tests contain special characters, ex:
> > prop_tests.py. As a result, it creates an invalid xml file and not being
> > displayed in Hudson.
> >
> > This commit fixes this issue, also tracked in issue 3541. With this fix,
> > the test results are displayed in Hudson, especially the results
> > specific to 1.6.x solaris build.
> > http://subversion.tigris.org/issues/show_bug.cgi?id=3541
> >
> > Index: tools/dev/gen_junit_report.py
> > =======================================
> > --- tools/dev/gen_junit_report.py (revision 886204)
> > +++ tools/dev/gen_junit_report.py (working copy)
> > @@ -46,6 +46,16 @@
> > data = data.replace(char, encode[char])
> > return data
> > +def remove_special_characters(data):
> > + """remove special characters in test failure reasons"""
> > + if not data:
> > + return data
> > + chars_table = "".join([chr(n) for n in xrange(256)])
> > + # remove all special characters upto ascii value 31, except line
> > feed (10)
> > + # and carriage return (13)
> > + chars_to_remove = chars_table[0:9] + chars_table[11:12] +
> > chars_table[14:31]
> > + return data.translate(chars_table, chars_to_remove)
>
> You want to rebuild the table every single time? Be serious.

This is just a string of length 256, being created for every failure
message. Even if all tests (73) fail, this string is going to be created
73 times, and only!

> Also, wouldn't it be more proper to find out why the tests put control
> characters in the failure description than to just blindly throw them away?

To identify the reason why the failure message is one thing. This is
prop_tests.py. Refer to the attachment in the issue 3541 for details.

Even if there are control characters in failure message, the valid junit
file should be generated and shouldn't refuse to generate test result.

> > def start_junit():
> > """define the beginning of xml document"""
> > head = """<?xml version="1.0" encoding="UTF-8"?>"""
> > @@ -72,6 +82,7 @@
> > """mark the test case as FAILED"""
> > casename = xml_encode(casename)
> > sub_test_name = test_name.replace('.', '-')
> > + reason = remove_special_characters(reason)
> > case = """<testcase time="ELAPSED_CASE_%s" name="%s" classname="%s">
> > <failure type="Failed"><![CDATA[%s]]></failure>
> > </testcase>""" % (test_name, casename, sub_test_name, reason)
>
> Oh yeah. I'm going to write a test that fails and the reason will be
> "]]>", how would you like that? :)

I'll handle the following feedback:
  a) Define the table globally (once) and re-use it.
  b) Handle "]]>" in failure message.

-- 
Bhuvaneswaran A    
CollabNet Software P Ltd.  |  www.collab.net