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
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?
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(']]>', ']]>]]><![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 ====================