Patchwork [RFC] run-tests: add support for xunit test reports

login
register
mail settings
Submitter Augie Fackler
Date June 30, 2014, 3:13 p.m.
Message ID <e783e767f540a61eeb87.1404141213@102.17.16.172.in-addr.arpa>
Download mbox | patch
Permalink /patch/5082/
State Superseded
Commit a06172e85fd4d605c49bd26df48549d4130747f2
Headers show

Comments

Augie Fackler - June 30, 2014, 3:13 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1403842355 14400
#      Fri Jun 27 00:12:35 2014 -0400
# Node ID e783e767f540a61eeb877a5b307f1ad76d2d86a6
# Parent  becb61de90a1a0384af535a393fb32e7da7a9059
run-tests: add support for xunit test reports

The Jenkins CI system understands xunit reports natively, so this will
be helpful for anyone that wants to use Jenkins for testing hg.
Gregory Szorc - June 30, 2014, 4:37 p.m.
On 6/30/14, 8:13 AM, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1403842355 14400
> #      Fri Jun 27 00:12:35 2014 -0400
> # Node ID e783e767f540a61eeb877a5b307f1ad76d2d86a6
> # Parent  becb61de90a1a0384af535a393fb32e7da7a9059
> run-tests: add support for xunit test reports
>
> The Jenkins CI system understands xunit reports natively, so this will
> be helpful for anyone that wants to use Jenkins for testing hg.

I'm overall positive about this patch.

It's worth noting that Python code for producing xunit results has been 
written before. Nose - a widely used package that supplements Python's 
built-in testing mechanisms - has a plugin for this 
(http://nose.readthedocs.org/en/latest/plugins/xunit.html).

My run-tests.py refactoring stopped short of allowing the integration of 
things like nose into the mix. I'm a bit disappointed you had to write 
this. But it's not that much code, so meh.

> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -190,6 +191,8 @@
>                " (implies --keep-tmpdir)")
>       parser.add_option("-v", "--verbose", action="store_true",
>           help="output verbose messages")
> +    parser.add_option("--xunit", type="string",
> +                      help="record xunit results at specified path")
>       parser.add_option("--view", type="string",
>           help="external diff viewer")
>       parser.add_option("--with-hg", type="string",
> @@ -304,6 +307,24 @@
>
>       return log(*msg)

Both this patch and Anurag's JSON output patch added options for 
producing extra metadata. I think it would be really nice if machine 
readable output were enabled by default. We already have enough command 
arguments as it is.

That being said, I'm not sure where the logical place for writing this 
type of output would be.

> +allsuccess = []
> +allfailures = {}
> +def xunitfail(test, lines):
> +    """Log a failure to the xunit output file if one is in use."""
> +    allfailures[test.name] = xmlsafe(''.join(lines))
> +
> +allerrors = {}
> +def xuniterr(test, err):
> +    allerrors[test] = xmlsafe(err)

I'm not a huge fan of stuffing these in global variables. Can you move 
these to instance variables of our custom TestResult class?

> @@ -1088,8 +1109,14 @@
>
>               self.stream.write('!')
>
> -    def addError(self, *args, **kwargs):
> -        super(TestResult, self).addError(*args, **kwargs)
> +    def addSuccess(self, test):
> +        super(TestResult, self).addSuccess(test)
> +        allsuccess.append(test)
> +
> +    def addError(self, test, err):
> +        super(TestResult, self).addError(test, err)
> +        tb = traceback.format_exception(*err)
> +        xuniterr(test, tb)

Yes, capturing things on the TestResult instance is the proper approach 
here.

>           return accepted
> @@ -1321,6 +1351,40 @@
>           for test, msg in result.errors:
>               self.stream.writeln('Errored %s: %s' % (test.name, msg))
>
> +        if self._runner.options.xunit:
> +            xuf = open(self._runner.options.xunit, 'wb')
> +            try:
> +                timesd = dict(result.times)
> +                xuf.write(
> +                    '<?xml version="1.0" encoding="%(encoding)s"?>'
> +                    '<testsuite name="run-tests" tests="%(total)d" '
> +                    'errors="%(errors)d" failures="%(failures)d" '
> +                    'skip="%(skipped)d">\n' % {
> +                        'total': result.testsRun,
> +                        'encoding': 'utf-8',
> +                        'failures': failed,
> +                        'skipped': skipped + ignored,
> +                        'errors': 0,
> +                        })

Manual XML writing? Can we not use a xml library here?
Augie Fackler - June 30, 2014, 4:41 p.m.
Thanks for the design insights - I'll see about moving things to the custom result class.

On Jun 30, 2014, at 12:37 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote:

> On 6/30/14, 8:13 AM, Augie Fackler wrote:
>> # HG changeset patch
>> # User Augie Fackler <raf@durin42.com>
>> # Date 1403842355 14400
>> #      Fri Jun 27 00:12:35 2014 -0400
>> # Node ID e783e767f540a61eeb877a5b307f1ad76d2d86a6
>> # Parent  becb61de90a1a0384af535a393fb32e7da7a9059
>> run-tests: add support for xunit test reports
>> 
>> The Jenkins CI system understands xunit reports natively, so this will
>> be helpful for anyone that wants to use Jenkins for testing hg.
> 
> I'm overall positive about this patch.
> 
> It's worth noting that Python code for producing xunit results has been written before. Nose - a widely used package that supplements Python's built-in testing mechanisms - has a plugin for this (http://nose.readthedocs.org/en/latest/plugins/xunit.html).

It may interest you to know that I wrote that nose plugin. :)

> My run-tests.py refactoring stopped short of allowing the integration of things like nose into the mix. I'm a bit disappointed you had to write this. But it's not that much code, so meh.

We could (almost certainly) wire up support for .t files as a nose plugin. I'm just lazy, and run-tests is nice because it's in-tree.

> 
>> diff --git a/tests/run-tests.py b/tests/run-tests.py
>> --- a/tests/run-tests.py
>> +++ b/tests/run-tests.py
>> @@ -190,6 +191,8 @@
>>               " (implies --keep-tmpdir)")
>>      parser.add_option("-v", "--verbose", action="store_true",
>>          help="output verbose messages")
>> +    parser.add_option("--xunit", type="string",
>> +                      help="record xunit results at specified path")
>>      parser.add_option("--view", type="string",
>>          help="external diff viewer")
>>      parser.add_option("--with-hg", type="string",
>> @@ -304,6 +307,24 @@
>> 
>>      return log(*msg)
> 
> Both this patch and Anurag's JSON output patch added options for producing extra metadata. I think it would be really nice if machine readable output were enabled by default. We already have enough command arguments as it is.
> 
> That being said, I'm not sure where the logical place for writing this type of output would be.

Yeah, I'm not sure there's any better options.

> 
>> +allsuccess = []
>> +allfailures = {}
>> +def xunitfail(test, lines):
>> +    """Log a failure to the xunit output file if one is in use."""
>> +    allfailures[test.name] = xmlsafe(''.join(lines))
>> +
>> +allerrors = {}
>> +def xuniterr(test, err):
>> +    allerrors[test] = xmlsafe(err)
> 
> I'm not a huge fan of stuffing these in global variables. Can you move these to instance variables of our custom TestResult class?

Sure. That's why I cc'ed you on this - I figured you'd have better design insights than I was getting on my plane home.

> 
>> @@ -1088,8 +1109,14 @@
>> 
>>              self.stream.write('!')
>> 
>> -    def addError(self, *args, **kwargs):
>> -        super(TestResult, self).addError(*args, **kwargs)
>> +    def addSuccess(self, test):
>> +        super(TestResult, self).addSuccess(test)
>> +        allsuccess.append(test)
>> +
>> +    def addError(self, test, err):
>> +        super(TestResult, self).addError(test, err)
>> +        tb = traceback.format_exception(*err)
>> +        xuniterr(test, tb)
> 
> Yes, capturing things on the TestResult instance is the proper approach here.
> 
>>          return accepted
>> @@ -1321,6 +1351,40 @@
>>          for test, msg in result.errors:
>>              self.stream.writeln('Errored %s: %s' % (test.name, msg))
>> 
>> +        if self._runner.options.xunit:
>> +            xuf = open(self._runner.options.xunit, 'wb')
>> +            try:
>> +                timesd = dict(result.times)
>> +                xuf.write(
>> +                    '<?xml version="1.0" encoding="%(encoding)s"?>'
>> +                    '<testsuite name="run-tests" tests="%(total)d" '
>> +                    'errors="%(errors)d" failures="%(failures)d" '
>> +                    'skip="%(skipped)d">\n' % {
>> +                        'total': result.testsRun,
>> +                        'encoding': 'utf-8',
>> +                        'failures': failed,
>> +                        'skipped': skipped + ignored,
>> +                        'errors': 0,
>> +                        })
> 
> Manual XML writing? Can we not use a xml library here?

We could, but last time I wrote this the xml libs turned out to be a pain in the rear. I'll take another look at minidom though.

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -52,6 +52,7 @@ 
 import sys
 import tempfile
 import time
+import traceback
 import random
 import re
 import threading
@@ -190,6 +191,8 @@ 
              " (implies --keep-tmpdir)")
     parser.add_option("-v", "--verbose", action="store_true",
         help="output verbose messages")
+    parser.add_option("--xunit", type="string",
+                      help="record xunit results at specified path")
     parser.add_option("--view", type="string",
         help="external diff viewer")
     parser.add_option("--with-hg", type="string",
@@ -304,6 +307,24 @@ 
 
     return log(*msg)
 
+
+# Bytes that break XML even in a CDATA block: control characters 0-31
+# sans \t, \n and \r
+CDATA_EVIL = re.compile(r"[\000-\010\013\014\016-\037]")
+
+def xmlsafe(data):
+    return CDATA_EVIL.sub('?', data).replace(']]>', ']]>]]&gt;<![CDATA[')
+
+allsuccess = []
+allfailures = {}
+def xunitfail(test, lines):
+    """Log a failure to the xunit output file if one is in use."""
+    allfailures[test.name] = xmlsafe(''.join(lines))
+
+allerrors = {}
+def xuniterr(test, err):
+    allerrors[test] = xmlsafe(err)
+
 def log(*msg):
     """Log something to stdout.
 
@@ -1088,8 +1109,14 @@ 
 
             self.stream.write('!')
 
-    def addError(self, *args, **kwargs):
-        super(TestResult, self).addError(*args, **kwargs)
+    def addSuccess(self, test):
+        super(TestResult, self).addSuccess(test)
+        allsuccess.append(test)
+
+    def addError(self, test, err):
+        super(TestResult, self).addError(test, err)
+        tb = traceback.format_exception(*err)
+        xuniterr(test, tb)
 
         if self._options.first:
             self.stop()
@@ -1130,6 +1157,8 @@ 
         """Record a mismatch in test output for a particular test."""
 
         accepted = False
+        failed = False
+        lines = []
 
         iolock.acquire()
         if self._options.nodiff:
@@ -1157,7 +1186,8 @@ 
                     else:
                         rename(test.errpath, '%s.out' % test.path)
                     accepted = True
-
+        if not accepted and not failed:
+            xunitfail(test, lines)
         iolock.release()
 
         return accepted
@@ -1321,6 +1351,40 @@ 
         for test, msg in result.errors:
             self.stream.writeln('Errored %s: %s' % (test.name, msg))
 
+        if self._runner.options.xunit:
+            xuf = open(self._runner.options.xunit, 'wb')
+            try:
+                timesd = dict(result.times)
+                xuf.write(
+                    '<?xml version="1.0" encoding="%(encoding)s"?>'
+                    '<testsuite name="run-tests" tests="%(total)d" '
+                    'errors="%(errors)d" failures="%(failures)d" '
+                    'skip="%(skipped)d">\n' % {
+                        'total': result.testsRun,
+                        'encoding': 'utf-8',
+                        'failures': failed,
+                        'skipped': skipped + ignored,
+                        'errors': 0,
+                        })
+                for tc in allsuccess:
+                    xuf.write(' <testcase name="%(test)s" '
+                              'time="%(time).3f"></testcase>\n' % {
+                                  'test': tc.name,
+                                  'time': timesd[tc.name],
+                              })
+                for tc, err in sorted(allfailures.iteritems()):
+                    xuf.write(
+                        ' <testcase name="%(test)s" time="%(time).3f">\n'
+                        '  <failure><![CDATA[%(error)s]]>\n'
+                        '  </failure>\n'
+                        ' </testcase>\n' % {'test': tc,
+                                            'error': err,
+                                            'time': timesd[tc]
+                                        })
+                xuf.write('</testsuite>\n')
+            finally:
+                xuf.close()
+
         self._runner._checkhglib('Tested')
 
         # When '--retest' is enabled, only failure tests run. At this point
diff --git a/tests/test-run-tests.t b/tests/test-run-tests.t
--- a/tests/test-run-tests.t
+++ b/tests/test-run-tests.t
@@ -42,6 +42,36 @@ 
   # Ran 2 tests, 0 skipped, 0 warned, 1 failed.
   python hash seed: * (glob)
   [1]
+test --xunit support
+  $ $TESTDIR/run-tests.py --with-hg=`which hg` --xunit=xunit.xml
+  
+  --- $TESTTMP/test-failure.t
+  +++ $TESTTMP/test-failure.t.err
+  @@ -1,2 +1,2 @@
+     $ echo babar
+  -  rataxes
+  +  babar
+  
+  ERROR: test-failure.t output changed
+  !.
+  Failed test-failure.t: output changed
+  # Ran 2 tests, 0 skipped, 0 warned, 1 failed.
+  python hash seed: * (glob)
+  [1]
+  $ cat xunit.xml
+  <?xml version="1.0" encoding="utf-8"?><testsuite name="run-tests" tests="2" errors="0" failures="1" skip="0">
+   <testcase name="test-success.t" time="*"></testcase> (glob)
+   <testcase name="test-failure.t" time="*"> (glob)
+    <failure><![CDATA[--- $TESTTMP/test-failure.t
+  +++ $TESTTMP/test-failure.t.err
+  @@ -1,2 +1,2 @@
+     $ echo babar
+  -  rataxes
+  +  babar
+  ]]>
+    </failure>
+   </testcase>
+  </testsuite>
 
 test for --retest
 ====================