Merge lp:~allenap/maas/parallel-tests-xunit into lp:~maas-committers/maas/trunk

[Gavin Panella]

Add test.parallel arguments to specify subprocesses and results.

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

[Gavin Panella (allenap)]

Thanks!

=== modified file 'Makefile'
--- Makefile	2017-02-03 08:45:37 +0000
+++ Makefile	2017-02-04 22:01:56 +0000
@@ -226,7 +226,7 @@
 	utilities/isolated-make-test
 
 test: bin/test.parallel
-	@bin/test.parallel
+	@bin/test.parallel --subprocess-per-core
 
 test-serial: $(strip $(test-scripts))
 	@bin/maas-region makemigrations --dry-run --exit && exit 1 ||:

=== modified file 'buildout.cfg'
--- buildout.cfg	2017-01-26 15:57:00 +0000
+++ buildout.cfg	2017-02-04 22:01:56 +0000
@@ -38,6 +38,7 @@
   fixtures
   hypothesis
   ipdb
+  junitxml
   nose
   nose-timer
   postgresfixture

=== modified file 'src/maastesting/parallel.py'
--- src/maastesting/parallel.py	2017-01-25 16:07:58 +0000
+++ src/maastesting/parallel.py	2017-02-04 22:01:56 +0000
@@ -15,6 +15,7 @@
 import threading
 import unittest
 
+import junitxml
 from maastesting.utils import content_from_file
 import subunit
 import testtools
@@ -166,22 +167,40 @@
     return split
 
 
-def print_test(test, status, start_time, stop_time, tags, details):
-    testid = "<none>" if test is None else test.id()
-    duration = (stop_time - start_time).total_seconds()
-    message = "%s: %s (%0.2fs)" % (status.upper(), testid, abs(duration))
-    print(message, flush=True)
-
-
-def test(suite):
-    parts = max(2, os.cpu_count() - 2)
-    split = make_splitter(parts)
+def make_human_readable_result(stream):
+    """Make a result that emits messages intended for human consumption."""
+
+    def print_result(test, status, start_time, stop_time, tags, details):
+        testid = "<none>" if test is None else test.id()
+        duration = (stop_time - start_time).total_seconds()
+        message = "%s: %s (%0.2fs)" % (status.upper(), testid, abs(duration))
+        print(message, file=stream, flush=True)
+
+    return testtools.MultiTestResult(
+        testtools.TextTestResult(stream, failfast=False, tb_locals=False),
+        testtools.TestByTestResult(print_result))
+
+
+def make_subunit_result(stream):
+    """Make a result that emits a subunit stream."""
+    return subunit.TestProtocolClient(stream)
+
+
+def make_junit_result(stream):
+    """Make a result that emits JUnit-compatible XML results."""
+    return junitxml.JUnitXmlResult(stream)
+
+
+def test(suite, result, processes):
+    """Test `suite`, emitting results to `result`.
+
+    :param suite: The test suite to run.
+    :param result: The test result to which to report.
+    :param processes: The number of processes to split up tests amongst.
+    :return: A boolean signalling success or not.
+    """
+    split = make_splitter(processes)
     suite = testtools.ConcurrentTestSuite(suite, split)
-    result = testtools.MultiTestResult(
-        testtools.TestByTestResult(print_test),
-        testtools.TextTestResult(
-            sys.stdout, failfast=False, tb_locals=False),
-    )
 
     result.startTestRun()
     try:
@@ -189,14 +208,62 @@
     finally:
         result.stopTestRun()
 
-    raise SystemExit(0 if result.wasSuccessful() else 2)
-
-
-argument_parser = argparse.ArgumentParser(description=__doc__)
-
-
-def main():
-    args = argument_parser.parse_args()  # noqa
+    return result.wasSuccessful()
+
+
+def make_argument_parser():
+    """Create an argument parser for the command-line."""
+    parser = argparse.ArgumentParser(description=__doc__, add_help=False)
+    parser.add_argument(
+        "-h", "--help", action="help", help=argparse.SUPPRESS)
+
+    core_count = os.cpu_count()
+
+    def parse_subprocesses(string):
+        try:
+            processes = int(string)
+        except ValueError:
+            raise argparse.ArgumentTypeError(
+                "%r is not an integer" % string)
+        else:
+            if processes < 1:
+                raise argparse.ArgumentTypeError(
+                    "%d is not 1 or greater" % processes)
+            else:
+                return processes
+
+    args_subprocesses = parser.add_mutually_exclusive_group()
+    args_subprocesses.add_argument(
+        "--subprocesses", metavar="N", action="store", type=parse_subprocesses,
+        dest="subprocesses", default=max(2, core_count - 2), help=(
+            "The number of testing subprocesses to run concurrently. This "
+            "defaults to the number of CPU cores available minus 2, but not "
+            "less than 2. On this machine the default is %(default)s."))
+    args_subprocesses.add_argument(
+        "--subprocess-per-core", action="store_const", dest="subprocesses",
+        const=core_count, help=(
+            "Run one test process per core. On this machine that would mean "
+            "that up to %d testing subprocesses would run concurrently."
+            % core_count))
+
+    args_output = parser.add_argument_group("output")
+    args_output.add_argument(
+        "--emit-human", dest="result_factory", action="store_const",
+        const=make_human_readable_result, help="Emit human-readable results.")
+    args_output.add_argument(
+        "--emit-subunit", dest="result_factory", action="store_const",
+        const=make_subunit_result, help="Emit a subunit stream.")
+    args_output.add_argument(
+        "--emit-junit", dest="result_factory", action="store_const",
+        const=make_junit_result, help="Emit JUnit-compatible XML.")
+    args_output.set_defaults(
+        result_factory=make_human_readable_result)
+
+    return parser
+
+
+def main(args=None):
+    args = make_argument_parser().parse_args(args)
     lock = threading.Lock()
     suite = unittest.TestSuite((
         # Run the indivisible tests first. These will each consume a worker
@@ -211,7 +278,11 @@
         TestScriptDivisible(lock, "bin/test.region"),
         TestScriptDivisible(lock, "bin/test.testing"),
     ))
-    test(suite)
+    result = args.result_factory(sys.stdout)
+    if test(suite, result, args.subprocesses):
+        raise SystemExit(0)
+    else:
+        raise SystemExit(2)
 
 
 if __name__ == '__main__':

=== modified file 'src/maastesting/tests/test_parallel.py'
--- src/maastesting/tests/test_parallel.py	2017-01-23 16:36:20 +0000
+++ src/maastesting/tests/test_parallel.py	2017-02-04 22:01:56 +0000
@@ -5,15 +5,150 @@
 
 __all__ = []
 
-import types
+import os
+import random
+from unittest.mock import ANY
 
+import junitxml
+from maastesting import parallel
+from maastesting.fixtures import CaptureStandardIO
+from maastesting.matchers import (
+    DocTestMatches,
+    MockCalledOnceWith,
+    MockNotCalled,
+)
 from maastesting.testcase import MAASTestCase
-from testtools.matchers import IsInstance
-
-
-class TestSmoke(MAASTestCase):
-    """Trivial smoke test."""
-
-    def test_imports_cleanly(self):
-        from maastesting import parallel
-        self.assertThat(parallel, IsInstance(types.ModuleType))
+import subunit
+from testtools import (
+    ExtendedToOriginalDecorator,
+    MultiTestResult,
+    TestByTestResult,
+    TextTestResult,
+)
+from testtools.matchers import (
+    Equals,
+    Is,
+    IsInstance,
+    MatchesAll,
+    MatchesListwise,
+    MatchesStructure,
+)
+
+
+class TestSubprocessArguments(MAASTestCase):
+    """Tests for arguments that adjust subprocess behaviour."""
+
+    def setUp(self):
+        super(TestSubprocessArguments, self).setUp()
+        self.stdio = self.useFixture(CaptureStandardIO())
+        self.patch_autospec(parallel, "test")
+        parallel.test.return_value = True
+
+    def test__defaults(self):
+        sysexit = self.assertRaises(SystemExit, parallel.main, [])
+        self.assertThat(sysexit.code, Equals(0))
+        self.assertThat(parallel.test, MockCalledOnceWith(
+            ANY, ANY, max(os.cpu_count() - 2, 2)))
+
+    def test__subprocess_count_can_be_specified(self):
+        count = random.randrange(100, 1000)
+        sysexit = self.assertRaises(
+            SystemExit, parallel.main, ["--subprocesses", str(count)])
+        self.assertThat(sysexit.code, Equals(0))
+        self.assertThat(parallel.test, MockCalledOnceWith(ANY, ANY, count))
+
+    def test__subprocess_count_of_less_than_1_is_rejected(self):
+        sysexit = self.assertRaises(
+            SystemExit, parallel.main, ["--subprocesses", "0"])
+        self.assertThat(sysexit.code, Equals(2))
+        self.assertThat(parallel.test, MockNotCalled())
+        self.assertThat(self.stdio.getError(), DocTestMatches(
+            "usage: ... argument --subprocesses: 0 is not 1 or greater"))
+
+    def test__subprocess_count_non_numeric_is_rejected(self):
+        sysexit = self.assertRaises(
+            SystemExit, parallel.main, ["--subprocesses", "foo"])
+        self.assertThat(sysexit.code, Equals(2))
+        self.assertThat(parallel.test, MockNotCalled())
+        self.assertThat(self.stdio.getError(), DocTestMatches(
+            "usage: ... argument --subprocesses: 'foo' is not an integer"))
+
+    def test__subprocess_per_core_can_be_specified(self):
+        sysexit = self.assertRaises(
+            SystemExit, parallel.main, ["--subprocess-per-core"])
+        self.assertThat(sysexit.code, Equals(0))
+        self.assertThat(parallel.test, MockCalledOnceWith(
+            ANY, ANY, os.cpu_count()))
+
+    def test__subprocess_count_and_per_core_cannot_both_be_specified(self):
+        sysexit = self.assertRaises(SystemExit, parallel.main, [
+            "--subprocesses", "3", "--subprocess-per-core"])
+        self.assertThat(sysexit.code, Equals(2))
+        self.assertThat(parallel.test, MockNotCalled())
+        self.assertThat(self.stdio.getError(), DocTestMatches(
+            "usage: ... argument --subprocess-per-core: not allowed with "
+            "argument --subprocesses"))
+
+
+class TestEmissionArguments(MAASTestCase):
+    """Tests for arguments that adjust result emission behaviour."""
+
+    def setUp(self):
+        super(TestEmissionArguments, self).setUp()
+        self.stdio = self.useFixture(CaptureStandardIO())
+        self.patch_autospec(parallel, "test")
+        parallel.test.return_value = True
+
+    def test__results_are_human_readable_by_default(self):
+        sysexit = self.assertRaises(SystemExit, parallel.main, [])
+        self.assertThat(sysexit.code, Equals(0))
+        self.assertThat(parallel.test, MockCalledOnceWith(ANY, ANY, ANY))
+        _, result, _ = parallel.test.call_args[0]
+        self.assertThat(result, IsMultiResultOf(
+            IsInstance(TextTestResult), IsInstance(TestByTestResult)))
+
+    def test__results_can_be_explicitly_specified_as_human_readable(self):
+        sysexit = self.assertRaises(
+            SystemExit, parallel.main, ["--emit-human"])
+        self.assertThat(sysexit.code, Equals(0))
+        self.assertThat(parallel.test, MockCalledOnceWith(ANY, ANY, ANY))
+        _, result, _ = parallel.test.call_args[0]
+        self.assertThat(result, IsMultiResultOf(
+            IsInstance(TextTestResult), IsInstance(TestByTestResult)))
+
+    def test__results_can_be_specified_as_subunit(self):
+        sysexit = self.assertRaises(
+            SystemExit, parallel.main, ["--emit-subunit"])
+        self.assertThat(sysexit.code, Equals(0))
+        self.assertThat(parallel.test, MockCalledOnceWith(ANY, ANY, ANY))
+        _, result, _ = parallel.test.call_args[0]
+        self.assertThat(result, IsInstance(subunit.TestProtocolClient))
+        self.assertThat(result, MatchesStructure(
+            _stream=Is(self.stdio.stdout.buffer)))
+
+    def test__results_can_be_specified_as_junit(self):
+        sysexit = self.assertRaises(
+            SystemExit, parallel.main, ["--emit-junit"])
+        self.assertThat(sysexit.code, Equals(0))
+        self.assertThat(parallel.test, MockCalledOnceWith(ANY, ANY, ANY))
+        _, result, _ = parallel.test.call_args[0]
+        self.assertThat(result, IsInstance(junitxml.JUnitXmlResult))
+        self.assertThat(result, MatchesStructure(
+            _stream=Is(self.stdio.stdout)))
+
+
+def IsMultiResultOf(*results):
+    """Match a `MultiTestResult` wrapping the given results."""
+    return MatchesAll(
+        IsInstance(MultiTestResult),
+        MatchesStructure(
+            _results=MatchesListwise([
+                MatchesAll(
+                    IsInstance(ExtendedToOriginalDecorator),
+                    MatchesStructure(decorated=matcher),
+                    first_only=True)
+                for matcher in results
+            ]),
+        ),
+        first_only=True,
+    )

=== modified file 'utilities/check-imports'
--- utilities/check-imports	2017-01-26 18:46:16 +0000
+++ utilities/check-imports	2017-02-04 22:01:56 +0000
@@ -152,6 +152,7 @@
     "dns|dns.**",
     "fixtures|fixtures.**",
     "hypothesis|hypothesis.**",
+    "junitxml|junitxml.**",
     "maastesting|maastesting.**",
     "nose|nose.**",
     "postgresfixture|postgresfixture.**",