Merge lp:~benji/lpbuildbot/add-worker-logging into lp:lpbuildbot

[Benji York]

revamp the subunit log observer to provide several logs, statistics, step description, and progress metrics

This branch revamps the subunit log observer to provide several logs
(along with the standard stdio log):

- a log per worker which contains the IDs of the tests that worker ran
  (for re-running that subset later)
- a log of just the subunit output (as best we can get it, there may be
  a few stray lines here and there)
- a "summary" log of just the failures and errors

There step's description is continuously updated to show the number of
tests run, passing, failing, and erroring as well as whether or not the
tests are done yet.

Finally, the number of tests run thus far is reported to the buildbot
progress metrics subsystem so test run ETAs are more accurate.

There is good test coverage, runnable with the command "python
bzrbuildbot/subunittest.py".

This branch has been run in a "production" EC2 instance and worked well.
Note that there were no errors in the production run, so there were a
few unexercised code paths, but I believe those are well covered by the
tests.

[Gary Poster (gary)]

Yay, great! Thank you.

- Please add a comment as to why you are not importing buildbot (and perhaps then also mention there how to run the tests)

- We talked on IRC about your idea of sorting worker logs by adding all "lower" worker logs whenever you add one. +1, though we can do that in a later branch.

- Similarly, I mentioned adding the failure log initially so it is always in a known place, and you liked that idea. We can also do that in a later branch.

- In bzrbuildbot/test.py , in Test.describe, if done is True, let's close out those logs in the Observer, as we discussed.

review: Approve

[Benji York (benji)]

All excellent points/suggestions. I will have very little time between
now and Monday night so I suggest landing it as-is and making cards for
the follow-up bits.

=== modified file 'bzrbuildbot/subunittest.py'
--- bzrbuildbot/subunittest.py	2012-04-13 17:02:46 +0000
+++ bzrbuildbot/subunittest.py	2012-04-27 20:56:18 +0000
@@ -1,98 +1,491 @@
 import re
+import subunit
+import testtools
 import unittest
 
-import buildbot.process.buildstep
-
-
-class SubunitObserver(buildbot.process.buildstep.LogLineObserver):
-    should_ignore = re.compile(
-        r'^(successful|skip): .+:(setUp|tearDown)').match
-
-    matchers = (
-        ('error_count', re.compile(r'^error: ').match),
-        ('failure_count', re.compile(r'^failure: ').match),
-        ('test_count', re.compile(r'^successful: ').match),
-    )
-
-    test_count = 0
-    failure_count = 0
-    error_count = 0
+
+class SubunitObserver(testtools.TestResult):
+    """Watch a subunit stream and report interesting things about it.
+
+    Test statistics (number of tests run, number of failures, etc. are
+    reported as well as a summary of errors and failures (see addError and
+    addFailure) and per-worker listings of the tests run so test-ordering and
+    inter-test effects can be reproduced later.
+    """
+
+    def __init__(self, step=None):
+        testtools.TestResult.__init__(self)
+        if step is not None:
+            self.step = step
+        self.logs = {}
+        self.started = False
+        self.real_test_count = 0
+        self.failure_count = 0
+        self.error_count = 0
+        # We will pass all incoming lines (which are a subunit stream) to the
+        # TestProtocolServer which will in turn call the corresponding
+        # TestResult methods on self (we are a TestResult subclass).
+        self.protocol = subunit.TestProtocolServer(
+            self, subunit.DiscardStream())
+
+    def log(self, name, line):
+        """Log a line to the named log."""
+        if name not in self.logs:
+            self.logs[name] = self.step.addLog(name)
+        self.logs[name].addStdout(line + '\n')
+
+    ### Begin Buildbot LogLineObserver methods.
 
     def outLineReceived(self, line):
-        # XXX 2012-04-13 frankban bug=949950:
-        #     This method matches subunit output. The implementation of
-        #     stats collection is blocked until `testr run --parallel`
-        #     will output the subunit stream.
-        if self.should_ignore(line):
-            return
-        for attr, matcher in self.matchers:
-            if matcher(line):
-                # Increment the corresponding statistic.
-                setattr(self, attr, getattr(self, attr) + 1)
+        """Process a line of the incoming subunit stream."""
+        # Impedance mismatch: subunit wants lines, observers get lines-no\n
+        self.protocol.lineReceived(line + '\n')
+        if self.started:
+            self.log('subunit', line)
+
+    ### End Buildbot LogLineObserver methods
+
+    def current_worker_id(self):
+        """Return the current worker ID tag value."""
+        for tag in self.current_tags:
+            if tag.startswith('worker-'):
+                worker_id = tag
                 break
-
-
-# Tests to verify that the subunit protocol is understood.
-
-class TestSubunitCounting(unittest.TestCase):
-
-    def test_error(self):
-        line = 'error: foo.bar.baz [ multipart'
-        observer = SubunitObserver()
-        self.assertEqual(observer.error_count, 0)
-        observer.outLineReceived(line)
-        self.assertEqual(observer.error_count, 1)
-
-    def test_failure(self):
-        line = 'failure: foo.bar.baz [ multipart'
-        observer = SubunitObserver()
-        self.assertEqual(observer.failure_count, 0)
-        observer.outLineReceived(line)
-        self.assertEqual(observer.failure_count, 1)
-
-    def test_success(self):
-        line = 'successful: foo.bar.baz'
-        observer = SubunitObserver()
-        self.assertEqual(observer.test_count, 0)
-        observer.outLineReceived(line)
-        self.assertEqual(observer.test_count, 1)
-
-    def test_ignore_setUp(self):
-        # Layer setup is reported as a test with a weird name.  They are
-        # detected and not counted in the statistics.
-        line = 'successful: lp.testing.layers.ZopelessAppServerLayer:setUp'
-        observer = SubunitObserver()
-        self.assertEqual(observer.test_count, 0)
-        observer.outLineReceived(line)
-        self.assertEqual(observer.test_count, 0)
-
-    def test_ignore_tearDown(self):
-        # Layer tearDown is reported as a test with a weird name.  They are
-        # detected and not counted in the statistics.
-        line = 'successful: lp.testing.layers.ZopelessAppServerLayer:tearDown'
-        observer = SubunitObserver()
-        self.assertEqual(observer.test_count, 0)
-        observer.outLineReceived(line)
-        self.assertEqual(observer.test_count, 0)
-
-    def test_ignore_tearDown_failure(self):
-        # Layer tearDown failure is reported as a skipped test with a weird
-        # name.  They are detected and not counted in the statistics.
-        line = 'skip: lp.testing.layers.ZopelessLayer:tearDown ['
-        observer = SubunitObserver()
-        self.assertEqual(observer.test_count, 0)
-        observer.outLineReceived(line)
-        self.assertEqual(observer.test_count, 0)
-
-    def test_ignore_false_positives(self):
-        # Just because a line has subunit-like text in it doesn't mean it's
-        # matched.
-        line = 'this is not successful: Betamax'
-        observer = SubunitObserver()
-        self.assertEqual(observer.test_count, 0)
-        observer.outLineReceived(line)
-        self.assertEqual(observer.test_count, 0)
-
+        else:
+            worker_id = 'unknown worker (bug in our subunit output?)'
+
+        return worker_id
+
+    sep1 = u'=' * 70 + '\n'
+    sep2 = u'-' * 70 + '\n'
+
+    def format_error(self, label, test_id, error_text):
+        """Format an error/failure for inclusion in the log."""
+        return u''.join([
+            self.sep1,
+            '%s: %s\n' % (label, test_id),
+            'worker ID: %s\n' % self.current_worker_id(),
+            self.sep2,
+            error_text,
+            ])
+
+    ### Begin TestResult methods.
+    def time(self, test):
+        self.started = True
+        testtools.TestResult.time(self, test)
+
+    def startTest(self, test):
+        self.started = True
+        testtools.TestResult.startTest(self, test)
+
+    def stopTest(self, test):
+        if ('zope:layer' not in self.current_tags and
+                'zope:info_suboptimal' not in self.current_tags):
+            self.log(self.current_worker_id(), test.id())
+            self.real_test_count += 1
+            self.step.setProgress('tests', self.real_test_count)
+            self.step.step_status.setStatistic(
+                'tests-total', self.real_test_count)
+        testtools.TestResult.stopTest(self, test)
+        self.step.step_status.setText(self.step.describe(False))
+
+    def addError(self, test, err):
+        testtools.TestResult.addError(self, test, err)
+        self.log(
+            'summary', self.format_error('ERROR', test.id(), str(err[1])))
+        self.error_count += 1
+        self.step.step_status.setStatistic('tests-errors', self.error_count)
+
+    def addFailure(self, test, err):
+        testtools.TestResult.addFailure(self, test, err)
+        self.log(
+            'summary', self.format_error('FAILURE', test.id(), str(err[1])))
+        self.failure_count += 1
+        self.step.step_status.setStatistic(
+            'tests-failures', self.failure_count)
+
+    ### End TestResult methods.
+
+
+def describe(step_status, base_description=None, done=False):
+    if base_description is None:
+        description = []
+    else:
+        description = base_description[:]
+    if step_status.hasStatistic('tests-total'):
+        total = step_status.getStatistic("tests-total", 0)
+        failures = step_status.getStatistic("tests-failures", 0)
+        errors = step_status.getStatistic("tests-errors", 0)
+        passed = total - (failures + errors)
+
+        if total:
+            description.append('%d tests' % total)
+        if passed:
+            description.append('%d passed' % passed)
+        if failures:
+            description.append('%d failures' % failures)
+        if errors:
+            description.append('%d errors' % errors)
+        if not done:
+            description.append('(still running)')
+    else:
+        description.append('no test results')
+    return description
+
+
+# Tests of the above start here.
+
+
+class LogLogger(object):
+    def __init__(self, log):
+        self.log = log
+
+    def addStdout(self, line):
+        self.log.append(line)
+
+
+class FauxStepStatus(object):
+    def __init__(self, stats=None, text=None):
+        if stats is None:
+            stats = {}
+        self.stats = stats
+        if text is None:
+            text = []
+        self.text = text
+
+    def setStatistic(self, name, value):
+        self.stats[name] = value
+
+    def hasStatistic(self, name):
+        return name in self.stats
+
+    def getStatistic(self, name, default=None):
+        return self.stats.get(name, default)
+
+    def setText(self, text):
+        self.text.append(text)
+
+
+class FauxStep(object):
+    def __init__(self):
+        self.progress = []
+        self.logs = {}
+        self.stats = {}
+        self.text = []
+        self.step_status = FauxStepStatus(self.stats, self.text)
+
+    def setProgress(self, name, value):
+        self.progress.append((name, value))
+
+    def addLog(self, name):
+        assert name not in self.logs, 'log already exists'
+        self.logs[name] = []
+        return LogLogger(self.logs[name])
+
+    def describe(self, done=False):
+        return describe(self.step_status, [], done)
+
+
+class TestSubunitObserverStatistics(unittest.TestCase):
+    """Verify that test statistics are reported.
+
+    SubunitObserver has a step object which has a setProgress method that
+    accepts a statistic name and an updated value for that statistic.
+
+    There are two ways a SubunitObserver can get access to the step, either
+    having it passed into its constructor (as we do in most of the tests) or
+    by having the buildbot set it on the instance after construction (as
+    Buildbot does it).
+    """
+
+    def test_step_provided(self):
+        # When creating a SubunitObserver a step can be passed in.
+        step = FauxStep()
+        observer = SubunitObserver(step)
+        # The step got stashed in the right place.
+        self.assertIs(step, observer.step)
+
+    def test_real_tests_counted(self):
+        # When a "real" test ends, the "tests" statistic is updated.  Tests
+        # are considered "real" if they don't represent one of the zope:layer
+        # or zope:info_suboptimal-tagged tests.
+        observer = SubunitObserver(FauxStep())
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('success: foo')
+        # The progress indicator is told about the test run.
+        self.assertEqual([('tests', 1)], observer.step.progress)
+        # As well as the step statistics.
+        self.assertIn('tests-total', observer.step.stats)
+        self.assertEqual(1, observer.step.stats['tests-total'])
+
+    def test_nonreal_tests_not_counted(self):
+        # When a "non-real" test ends, the "tests" statistic is not updated.
+        # Tests are considered "non-real" if they represent one of the
+        # zope:layer or zope:info_suboptimal-tagged tests.
+        observer = SubunitObserver(FauxStep())
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('tags: zope:layer')
+        observer.outLineReceived('success: foo')
+        observer.outLineReceived('test: bar')
+        observer.outLineReceived('tags: zope:info_suboptimal')
+        observer.outLineReceived('success: bar')
+        # The test is not reported to the progress counter or statistics.
+        self.assertEqual([], observer.step.progress)
+        self.assertEqual({}, observer.step.stats)
+
+    def test_failures_counted(self):
+        # When a new test failure is presented, the statistics are updated.
+        observer = SubunitObserver(FauxStep())
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('failure: foo [')
+        observer.outLineReceived(']')
+        # The failing test is counted.
+        self.assertIn('tests-failures', observer.step.stats)
+        self.assertEqual(1, observer.step.stats['tests-failures'])
+
+    def test_errors_counted(self):
+        # When a new test error is presented, the "tests failed" statistic
+        # is updated.
+        observer = SubunitObserver(FauxStep())
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('error: foo [')
+        observer.outLineReceived(']')
+        # The failing test is counted.
+        self.assertIn('tests-errors', observer.step.stats)
+        self.assertEqual(1, observer.step.stats['tests-errors'])
+
+    def test_statistics_summarized(self):
+        # When tests finish the step statistic's summary is updated.
+        observer = SubunitObserver(FauxStep())
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('failure: foo [')
+        observer.outLineReceived(']')
+        observer.outLineReceived('test: bar')
+        observer.outLineReceived('success: bar')
+        self.assertSequenceEqual(
+            [['1 tests', '1 failures', '(still running)'],
+             ['2 tests', '1 passed', '1 failures', '(still running)']],
+            observer.step.text)
+        # We don't see a final update without the "(still running)" bit
+        # because a final "describe" is called by Buildbot iteself.
+
+
+class TestSubunitObserverWorkerLogging(unittest.TestCase):
+    """Tests run in each worker are logged to a per-worker log.
+
+    Tests are run in many workers and then aggregated.  Sometimes test
+    interactions cause test failures.  In that case we want to be able to
+    reproduce a test run of just the tests executed by a particular worker.
+    To facilitate that, we log each worker's test IDs to their own log.
+    """
+
+    def test_worker_logs_contain_test_ids(self):
+        # If a test is tagged as having been run by a particular worker,
+        # the test ID is added to that worker's log.
+        step = FauxStep()
+        observer = SubunitObserver(step)
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('tags: worker-99')
+        observer.outLineReceived('success: foo')
+        # The worker had a log created just for it.
+        self.assertIn('worker-99', step.logs)
+        self.assertEqual(['foo\n'], step.logs['worker-99'])
+
+    def test_ignore_fake_test_layer_tests(self):
+        # Our subunit stream contains details about layer setUp masquerading
+        # as real tests.  We do not want to report those fake tests in the
+        # per-worker list of tests run.
+        step = FauxStep()
+        observer = SubunitObserver(step)
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('tags: worker-99 zope:layer')
+        observer.outLineReceived('success: foo')
+        # The worker did not have a log created, because layer "tests" are
+        # ignored.
+        self.assertNotIn('worker-99', step.logs)
+
+    def test_ignore_suboptimal_tests(self):
+        # Our subunit stream contains details about starting subprocesses
+        # masquerading as real tests.  We do not want to report those fake
+        # tests in the per-worker list of tests run.
+        step = FauxStep()
+        observer = SubunitObserver(step)
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('tags: worker-99 zope:info_suboptimal')
+        observer.outLineReceived('success: foo')
+        # The worker did not have a log created, because layer "tests" are
+        # ignored.
+        self.assertNotIn('worker-99', step.logs)
+
+
+class TestSubunitObserverStdoutLogging(unittest.TestCase):
+    """All subunit output is logged in the "subunit" log."""
+
+    def test_subunit_logged(self):
+        step = FauxStep()
+        observer = SubunitObserver(step)
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('tags: worker-99 zope:info_suboptimal')
+        observer.outLineReceived('success: foo')
+        # A stdout log was created...
+        self.assertIn('subunit', step.logs)
+        # ...and it contains the lines we fed in.
+        self.assertSequenceEqual(
+            ['test: foo\n',
+             'tags: worker-99 zope:info_suboptimal\n',
+             'success: foo\n'],
+            step.logs['subunit'])
+
+    def test_nonsubunit_not_logged(self):
+        # All output is ignored until subunit output starts.
+        step = FauxStep()
+        observer = SubunitObserver(step)
+        observer.outLineReceived('foo')
+        observer.outLineReceived('bar')
+        observer.outLineReceived('time: 2000-01-01T00:00:00')
+        observer.outLineReceived('test: foo')
+        # The subunit output was written to the "subunit" log.
+        self.assertSequenceEqual(
+            ['time: 2000-01-01T00:00:00\n', 'test: foo\n'],
+            step.logs['subunit'])
+
+
+class TestSubunitObserverSummaryLogging(unittest.TestCase):
+    """All test errors and failures are logged in the "summary" log."""
+
+    def test_successful_tests_produce_no_summary(self):
+        # The summary log only contains bad things, successes aren't bad.
+        step = FauxStep()
+        observer = SubunitObserver(step)
+        observer = SubunitObserver(FauxStep())
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('successes: foo')
+        # A summary log has not been created yet because there are no failures
+        # to report.
+        self.assertNotIn('summary', step.logs)
+
+    def test_failing_tests_are_reported(self):
+        # The summary log contains bad things, a test failure is bad.
+        step = FauxStep()
+        observer = SubunitObserver(step)
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('failure: foo [')
+        observer.outLineReceived(']')
+        self.assertIn('summary', step.logs)
+        self.assertIn('FAILURE: foo\n', step.logs['summary'][0])
+
+    def test_tests_with_errors_are_reported(self):
+        # The summary log contains bad things, a test error is bad.
+        step = FauxStep()
+        observer = SubunitObserver(step)
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('error: foo [')
+        observer.outLineReceived(']')
+        self.assertIn('summary', step.logs)
+        self.assertIn('ERROR: foo\n', step.logs['summary'][0])
+
+    def test_failure_details_are_reported(self):
+        # The details provided with a test failure are logged.
+        step = FauxStep()
+        observer = SubunitObserver(step)
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('error: foo [')
+        observer.outLineReceived('DETAILS')
+        observer.outLineReceived(']')
+        self.assertIn('summary', step.logs)
+        self.assertIn('DETAILS', step.logs['summary'][0])
+
+
+class TestSubunitObserverErrorFormatting(unittest.TestCase):
+    """The format_error method is used to make presentable errors."""
+
+    def test_formatted_errors_contain_the_worker_id(self):
+        observer = SubunitObserver(FauxStep())
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('tags: worker-99')
+        self.assertIn('worker-99', observer.format_error('', '', ''))
+
+
+class TestWorkerTag(unittest.TestCase):
+    """Each test should be tagged with the ID of the worker that ran it."""
+
+    def test_current_worker_id_finds_tag(self):
+        # If the currently running test has a worker ID, current_worker_id
+        # will return it.
+        observer = SubunitObserver(FauxStep())
+        observer.outLineReceived('test: foo')
+        observer.outLineReceived('tags: worker-99')
+        self.assertEqual('worker-99', observer.current_worker_id())
+
+    def test_current_worker_id_complains_when_no_worker_tag_is_present(self):
+        # If the currently running test does not have a worker ID,
+        # current_worker_id will return a fake ID that includes a hint that
+        # there may be a bug.
+        observer = SubunitObserver(FauxStep())
+        observer.outLineReceived('test: foo')
+        self.assertEqual(
+            'unknown worker (bug in our subunit output?)',
+            observer.current_worker_id())
+
+
+class TestDescribe(unittest.TestCase):
+    """The "describe" function describes the current test run."""
+
+    def test_bland_description(self):
+        # If nothing has happened yet, then the description doesn't say much.
+        status = FauxStepStatus()
+        self.assertEqual(['no test results'], describe(status))
+
+    def test_not_done(self):
+        # If some tests have run but they aren't done yet, the description
+        # says so.
+        status = FauxStepStatus()
+        status.setStatistic('tests-total', 42)
+        self.assertIn('(still running)', describe(status))
+
+    def test_done(self):
+        # If the tests have run and are all done, the description doesn't say
+        # that they're still running.
+        status = FauxStepStatus()
+        status.setStatistic('tests-total', 42)
+        self.assertNotIn('(still running)', describe(status, done=True))
+
+    def test_failing_tests_described(self):
+        # If some tests have failed, they are included in the description.
+        status = FauxStepStatus()
+        status.setStatistic('tests-total', 42)
+        status.setStatistic('tests-failures', 3)
+        self.assertIn('3 failures', describe(status))
+
+    def test_erroring_tests_described(self):
+        # If some tests have generated errors, they are included in the
+        # description.
+        status = FauxStepStatus()
+        status.setStatistic('tests-total', 42)
+        status.setStatistic('tests-errors', 6)
+        self.assertIn('6 errors', describe(status))
+
+    def test_base_description_is_left_alone(self):
+        # The base description passed into "describe" isn't mutated.
+        status = FauxStepStatus()
+        status.setStatistic('tests-total', 42)
+        status.setStatistic('tests-errors', 6)
+        base_description = ['base']
+        describe(status, base_description)
+        # Even though a description was generated the base description wasn't
+        # touched.
+        assert len(base_description) == 1
+        assert len(describe(status, base_description)) > 1
+        self.assertSequenceEqual(['base'], base_description)
+
+    def test_passing_tests_are_calculated(self):
+        # The number of passing is calculated from the total, failures, and
+        # errors.
+        status = FauxStepStatus()
+        status.setStatistic('tests-total', 42)
+        status.setStatistic('tests-failures', 3)
+        status.setStatistic('tests-errors', 6)
+        self.assertIn('33 passed', describe(status))
 
 
 if __name__ == '__main__':

=== modified file 'bzrbuildbot/test.py'
--- bzrbuildbot/test.py	2012-04-13 17:02:46 +0000
+++ bzrbuildbot/test.py	2012-04-27 20:56:18 +0000
@@ -1,6 +1,16 @@
+from bzrbuildbot import subunittest
+import buildbot.process.buildstep
 import buildbot.steps.shell
 
-from bzrbuildbot.subunittest import SubunitObserver as TestObserver
+
+class TestObserver(
+        subunittest.SubunitObserver,
+        buildbot.process.buildstep.LogLineObserver):
+
+    def __init__(self, *args, **kws):
+        buildbot.process.buildstep.LogLineObserver.__init__(
+            self, *args, **kws)
+        subunittest.SubunitObserver.__init__(self)
 
 
 class Test(buildbot.steps.shell.ShellCommand):
@@ -16,23 +26,5 @@
         self.addLogObserver('stdio', TestObserver())
 
     def describe(self, done=False):
-        description = buildbot.steps.shell.ShellCommand.describe(self, done)[:]
-        if self.step_status.hasStatistic('tests-total'):
-            total = self.step_status.getStatistic("tests-total", 0)
-            failures = self.step_status.getStatistic("tests-failures", 0)
-            errors = self.step_status.getStatistic("tests-errors", 0)
-            passed = total - (failures + errors)
-
-            if total:
-                description.append('%d tests' % total)
-            if passed:
-                description.append('%d passed' % passed)
-            if failures:
-                description.append('%d failures' % failures)
-            if errors:
-                description.append('%d errors' % errors)
-            if not done:
-                description.extend(['(through', 'last', 'completed', 'layer)'])
-        else:
-            description.append("no test results")
-        return description
+        description = buildbot.steps.shell.ShellCommand.describe(self, done)
+        return subunittest.describe(self.step_status, description, done)

=== modified file 'master.cfg'
--- master.cfg	2012-04-03 18:04:36 +0000
+++ master.cfg	2012-04-27 20:56:18 +0000
@@ -118,10 +118,6 @@
         command=['sudo', '/usr/local/bin/launchpad-lxc-build'],
         description=['building', 'in', 'base', 'lxc', 'container'],
         descriptionDone=['build', 'in', 'base', 'lxc', 'container']))
-    fac.addStep(bzrbuildbot.shell.ShellCommand(
-        command=['sudo', '/usr/local/bin/launchpad-lxc-test', '-t', 'stories/gpg'],
-        description=['running', 'tests', 'in', 'lxc', 'container'],
-        descriptionDone=['ran', 'tests', 'in', 'lxc', 'container']))
 
 launchpad_test_factory = factory.BuildFactory()
 # default timeout for checkout is 20 minutes (20*60)
@@ -133,7 +129,7 @@
     bzrbuildbot.test.Test(
         timeout=6*60*60,
         env={'TEMP': properties.WithProperties('%(build_dir)s/temp')},
-        command=['testr', 'run', '--parallel']))
+        command=['testr', 'run', '--parallel', '--subunit', '--full-results']))
 
 from buildbot.config import BuilderConfig
 c['builders'] = [