Merge lp:~vila/canonical-identity-provider/runner-upstreamed into lp:canonical-identity-provider/release

[Vincent Ladeuil]

Remove duplication in test runner concurrency support.

Cleaning after implement concurrent tests, I tried to align with sca, this third step is remove duplication with sca.

This required some refactoring all over the place but the end result is that each project shouldn't have to include boilerplate anymore but focus on the specifics:

- how to isolate the test suite itself (which is where most of the settings common to all tests go),

- whatever alternate runners are defined as long as they provide a command to stream back test results

The ugly duck is bzr+ssh://bazaar.launchpad.net/~vila/+junk/ols-tests-django which doesn't fit uci-tests (no django) nor u1-test-utils (wrong django now that 1.9 is out) but is compatible with django 1.6 (sca) and django 1.7 (sso) which make it fun to test ;)

This requires a new uci-tests: https://code.launchpad.net/~vila/canonical-identity-provider/dependencies/+merge/280891

[Natalia Bidart (nataliabidart)]

As long as revno 10 of ols-tests-django works with 1.8, this looks great!

review: Approve

[Vincent Ladeuil (vila)]

Yes it does ;)

=== modified file 'config-manager.txt'
--- config-manager.txt	2015-12-09 11:56:41 +0000
+++ config-manager.txt	2016-01-07 14:41:41 +0000
@@ -5,6 +5,7 @@
 ./branches/django-pgtools        bzr+ssh://bazaar.launchpad.net/~canonical-isd-hackers/django-pgtools/trunk;revno=8
 ./branches/django-piston         bzr+ssh://bazaar.launchpad.net/~ubuntuone-pqm-team/django-piston/stable;revno=6
 ./branches/django-saml2-idp      bzr+ssh://bazaar.launchpad.net/~ubuntuone-pqm-team/django-saml2-idp/stable;revno=68
+./branches/ols-tests-django        bzr+ssh://bazaar.launchpad.net/~vila/+junk/ols-tests-django;revno=10
 ./branches/python-openid         bzr+ssh://bazaar.launchpad.net/~ubuntuone-pqm-team/python-openid/stable;revno=1989
 ./branches/ssoclient             bzr+ssh://bazaar.launchpad.net/~canonical-isd-hackers/canonical-identity-provider/ssoclient;revno=7
 

=== added symlink 'lib/olstestsdjango'
=== target is u'../branches/ols-tests-django/olstestsdjango'
=== modified file 'requirements_devel.txt'
--- requirements_devel.txt	2015-12-21 22:51:15 +0000
+++ requirements_devel.txt	2016-01-07 14:41:41 +0000
@@ -26,6 +26,6 @@
 testscenarios==0.4
 testtools==0.9.39
 u1-test-utils==0.5
-ucitests==0.1.8
+ucitests==0.2.1
 wsgi-intercept==0.5.1
 -r requirements_docs.txt

=== modified file 'src/testing/runner.py'
--- src/testing/runner.py	2015-11-26 09:10:03 +0000
+++ src/testing/runner.py	2016-01-07 14:41:41 +0000
@@ -1,4 +1,3 @@
-import optparse
 import os
 import shutil
 import subprocess
@@ -9,14 +8,8 @@
 from django.utils import unittest
 from django.test import runner as dj_runner
 from gargoyle import gargoyle
-import subunit
-import testtools
-from ucitests import (
-    filters,
-    results,
-    runners,
-    suites,
-)
+from olstestsdjango import runner
+from ucitests import suites
 
 try:
     import cProfile as profile
@@ -68,169 +61,6 @@
                 connection.creation.destroy_test_db(old_name, verbosity=1)
 
 
-class TestRunner(dj_runner.DiscoverRunner):
-
-    option_list = dj_runner.DiscoverRunner.option_list + (
-        optparse.make_option(
-            '-c', '--concurrency',
-            action='store', dest='concurrency', type='int',
-            default=1,
-            help='How many concurrent processes to run the tests.'),
-        optparse.make_option(
-            '-l', '--list',
-            action='store_true', dest='list_only',
-            default=False,
-            help='List test ids without running them.'),
-        optparse.make_option(
-            '-i', '--include',
-            action='store', dest='include',
-            default=None,
-            help='Tests matching this regexp will be run.'),
-        optparse.make_option(
-            '-x', '--exclude',
-            action='store', dest='exclude',
-            default=None,
-            help='Tests matching this regexp will not be run.'),
-        optparse.make_option(
-            '--load',
-            action='store', dest='load_file',
-            default=None,
-            help='A file name containing test ids to be run.'),
-        optparse.make_option(
-            '-f', '--format',
-            action='store', dest='format', type='choice',
-            choices=['text', 'subunit'], default='text',
-            help='Output format for the test results.'),
-        optparse.make_option(
-            '-s', '--sub-runner',
-            action='store', dest='sub_runner', type='choice',
-            choices=['fork', 'lxc'], default='fork',
-            help='The concurrent test runner to use.'),
-    )
-
-    def __init__(self, pattern=None, top_level=None, verbosity=1,
-                 interactive=True, failfast=False, concurrency=1,
-                 list_only=False, include=None, exclude=None,
-                 load_file=None, format=None,
-                 sub_runner=None,
-                 output=None,
-                 **kwargs):
-        super(TestRunner, self).__init__(
-            pattern, top_level, verbosity=verbosity, interactive=interactive,
-            failfast=failfast, **kwargs)
-        self.list_only = list_only
-        if output is None:
-            output = sys.stdout
-        self.output = output
-        if include is not None:
-            self.includes = include.split(':')
-        else:
-            self.includes = []
-        if exclude is not None:
-            self.excludes = exclude.split(':')
-        else:
-            self.excludes = []
-        self.concurrency = concurrency
-        self.load_file = load_file
-        self.format = format
-        self.sub_runner = sub_runner
-
-    def setup_databases(self, **kwargs):
-        """Override base class to defer the db setup.
-
-        The db setup id done in SSOTestSuite so each worker can use its own
-        db.
-
-        """
-        # Return an empty but valid value so teardown_databases does nothing
-        # when invoked from the base class run_suite().
-        return ([], [])
-
-    def build_suite(self, test_labels=None, extra_tests=None, **kwargs):
-        if not test_labels:
-            # Without this, only the tests in the acceptance folder
-            # will be tried to be run when no labels are specified.
-            test_labels = ('src',)
-        suite = super(TestRunner, self).build_suite(
-            test_labels, extra_tests, **kwargs)
-        suite = filters.include_regexps(self.includes, suite)
-        suite = filters.exclude_regexps(self.excludes, suite)
-        if self.load_file is not None:
-            # Load the file and filter the suite
-            with open(self.load_file, 'r') as f:
-                id_list = filters.TestIdList(filters.load_test_id_list(f))
-                suite = filters.include_id_list(id_list, suite)
-        wrapping_suite = SSOTestSuite(suite)
-        return wrapping_suite
-
-    def run_tests(self, test_labels, extra_tests=None, **kwargs):
-        # Override base class so we can list the tests without the costly
-        # setup.
-        if self.list_only:
-            suite = self.build_suite(test_labels, extra_tests)
-            for t in testtools.testsuite.iterate_tests(suite):
-                self.output.write(t.id() + '\n')
-            return 0
-        # Otherwise let the base class handle the setup and the run
-        return super(TestRunner, self).run_tests(test_labels, extra_tests=None,
-                                                 **kwargs)
-
-    def run_suite(self, suite, **kwargs):
-        initial_test_nb = suite.countTestCases()
-        if self.concurrency > 1:
-            if self.sub_runner == 'fork':
-                concurrent_runner = runners.run_suite_forked
-            elif self.sub_runner == 'lxc':
-                concurrent_runner = run_suite_in_container
-            concurrent_suite = testtools.ConcurrentTestSuite(
-                suite, runners.split_suite_for(self.concurrency,
-                                               concurrent_runner))
-        else:
-            concurrent_suite = suite
-        if self.format == 'text':
-            result = results.TextResult(sys.stdout,
-                                        verbosity=self.verbosity,
-                                        failfast=self.failfast)
-        else:
-            result = subunit.TestProtocolClient(sys.stdout)
-        result.startTestRun()
-        try:
-            concurrent_suite.run(result)
-            self.check_all_tests_executed(initial_test_nb, result)
-        except subprocess.CalledProcessError as e:
-            sys.stderr.write('{}'.format(e))
-            raise
-        finally:
-            result.stopTestRun()
-        return result
-
-    def check_all_tests_executed(self, initial_test_nb, result):
-        # In very rare failure modes, tests that should be run may not
-        # increment 'testsRun'. This sanity check will catch them.
-        try:
-            if initial_test_nb != result.testsRun:
-                raise AssertionError(
-                    'Expecting {}, got {} tests run'.format(
-                        initial_test_nb, result.testsRun))
-        except:
-            # Defined here to get a revealing class path without risking test
-            # discovery to pick us.
-            class RunSuiteIntegrity(unittest.TestCase):
-                # 'run' is just an existing method, we won't execute that test
-                runTest = unittest.TestCase.run
-
-            t = RunSuiteIntegrity('run')
-            result.errors.append((t, result._exc_info_to_string(sys.exc_info(),
-                                                                t)))
-            # The above avoid a spurious 'ERROR' on the output stream if we use
-            # the following (clearer and not relying on private details) call
-            # result.addError(RunSuiteIntegrity('run'), sys.exc_info())
-
-
-# FIXME: More of the following should be upstreamed to uci-{tests,vms}. Part of
-# it has been stolen from there to start with. -- vila 2015-06-09
-
-
 def subprocess_run(args):
     try:
         return subprocess.check_output(args,
@@ -244,88 +74,40 @@
         raise
 
 
-class TestInContainer(object):
-
-    def __init__(self, worker_name, test_list_path, worker_num):
-        super(TestInContainer, self).__init__()
-        self.worker_name = worker_name
-        self.test_list_path = test_list_path
-        self.worker_num = worker_num
-
-    def __call__(self, result=None):
-        # comply with unittests.TestCase API
-        return self.run(result)
-
-    def run(self, result):
+class SSOContainerSuite(suites.SubprocessedSuite):
+
+    def setUp(self):
+        super(SSOContainerSuite, self).setUp()
+        # FIXME: We use a file in the current directory as a cheap way to share
+        # it with the container which bind mount the user HOME. It would be
+        # better to upload it to the container instead but it's more
+        # involved. In the mean time, for debugging isolation issues, it may
+        # help to start with the 'self.test_list_path' file (taking a copy
+        # here) -- vila 2015-12-08
         here = os.getcwd()
         # Spawn the ephemeral container
+        self.worker_name = 'sso-worker-{:>02d}'.format(self.unique)
         subprocess_run(['uci-vms', 'start', self.worker_name])
-        try:
-            # Finish container setup (food for uci-vms there)
-            subprocess_run(['uci-vms', 'shell', self.worker_name,
-                            '(', 'cd', here, '&&',
-                            'make', 'start-db',
-                            ')'])
-            # Run the tests, connecting the pipes
-            cmd = ['uci-vms', 'shell', self.worker_name,
-                   # And now we run the tests inside the container
-                   '(', 'cd', here, '&&',
-                   'make', 'test',
-                   'ARGS="-f subunit --load {}"'.format(self.test_list_path),
-                   ')']
-            process = subprocess.Popen(cmd,
-                                       stdin=subprocess.PIPE,
-                                       stdout=subprocess.PIPE,
-                                       # line buffering for subunit
-                                       bufsize=1)
-            # run the subunit test that collect the real tests results
-            test = TestInSubprocess(process)
-            test.run(result)
-        finally:
-            # Whatever happens, cleanup.
-            # Note that for debugging isolation issues, it may help to start
-            # with the 'self.test_list_path' file, so commenting the following
-            # line will preserve all lists.
-            os.remove(self.test_list_path)
-            # And tear down the ephemeral container
-            subprocess_run(['uci-vms', 'stop', self.worker_name])
-
-
-class TestInSubprocess(subunit.ProtocolTestCase):
-
-    def __init__(self, process):
-        super(TestInSubprocess, self).__init__(process.stdout)
-        self.process = process
-        self.process.stdin.close()
-
-    def run(self, result):
-        try:
-            super(TestInSubprocess, self).run(result)
-        finally:
-            self.process.wait()
-
-
-def run_suite_in_container(unique, child_suite, parent_suite):
-    """Execute a test suite into a container.
-
-    :param unique: A number uniquely identifying the suite/container.
-
-    :param child_suite: A TestSuite object to be run in the container.
-
-    :param parent_suite: A TestSuite object where the parent test collecting
-        the child results should be added.
-    """
-    worker_name = 'sso-worker-{:>02d}'.format(unique)
-    # FIXME: We use a file in the current directory as a cheap way to share it
-    # with the container which bind mount the user HOME. It would be better to
-    # upload it to the container instead but it's more involved. -- vila
-    # 2015-06-09
-    test_list_path = '{}.test-list'.format(worker_name)
-    with open(test_list_path, 'w') as tl:
-        for t in filters.iter_flat(child_suite):
-            tl.write('{}\n'.format(t.id()))
-    test = TestInContainer(worker_name, test_list_path, unique)
-    parent_suite.addTest(test)
+        self.addCleanup(subprocess_run, ['uci-vms', 'stop', self.worker_name])
+        # Finish container setup (food for a make target?)
+        subprocess_run(['uci-vms', 'shell', self.worker_name,
+                        '(', 'cd', here, '&&',
+                        'make', 'start-db',
+                        ')'])
+        # Build the command running the tests, connecting the pipes
+        self.cmd = ['uci-vms', 'shell', self.worker_name,
+                    # And now we run the tests inside the container
+                    '(', 'cd', here, '&&',
+                    'make', 'test',
+                    'ARGS="-f subunit --load {}"'.format(self.test_list_path),
+                    ')']
+
+
+class TestRunner(runner.TestRunner):
+
+    split_suites = dict(fork=suites.ForkedSuite,
+                        lxc=SSOContainerSuite)
+    wrapping_test_suite = SSOTestSuite
 
 
 class ProfilingTestSuiteRunner(TestRunner):