Improve CTest logging

master/custom_steps.py

@@ -0,0 +1,89 @@
+import xml.etree.ElementTree as Xml
+
+from buildbot.process.buildstep import BuildStepFailed, BuildStep, ShellMixin
+from buildbot.steps.worker import CompositeStepMixin
+from twisted.internet import defer
+
+__all__ = ['CTest']
+
+
+class CTest(ShellMixin, CompositeStepMixin, BuildStep):
+    name = 'ctest'
+
+    def __init__(self, *, build_config, jobs=None, tests=None, exclude_tests=None, labels=None, exclude_labels=None,
+                 **kwargs):
+        kwargs['command'] = [
+            'ctest',
+            '--build-config', build_config,
+            *(['--parallel', str(jobs)] if jobs else []),
+            *(['--tests-regex', '|'.join(tests)] if tests else []),
+            *(['--exclude-regex', '|'.join(exclude_tests)] if exclude_tests else []),
+            *(['--label-regex', '|'.join(labels)] if labels else []),
+            *(['--label-exclude', '|'.join(exclude_labels)] if exclude_labels else []),
+            '--output-on-failure',
+            '--test-action', 'Test',
+            '--no-compress-output'
+        ]
+
+        kwargs = self.setupShellMixin(kwargs)
+        super().__init__(**kwargs)
+
+    @defer.inlineCallbacks
+    def run(self):
+        # Remove any leftover log files (if they exist)
+        yield self.runRmdir(f'{self.workdir}/Testing', abandonOnFailure=False)
+
+        # Run CTest
+        cmd = yield self.makeRemoteShellCommand()
+        yield self.runCommand(cmd)
+
+        # Upload the XML log from the CTest run
+        xml_results = yield self.runGlob(f'{self.workdir}/Testing/*/*.xml')
+        if len(xml_results) != 1:
+            raise BuildStepFailed(f'Expected to find a single XML file. Got: {xml_results}')
+
+        ctest_log = yield self.getFileContentFromWorker(xml_results[0], abandonOnFailure=True)
+
+        # Parse the result, collecting test failures into more convenient logs.
+        root = Xml.fromstring(ctest_log)
+
+        for test in root.findall(".//Test[@Status='failed']"):
+            log = yield self.addLog(test.findtext('Name'))
+            self.write_xml(test,
+                           ("./Results/NamedMeasurement[@name='Environment']/Value", log.addHeader),
+                           ("./Results/NamedMeasurement[@name='Command Line']/Value", log.addHeader),
+                           ("./Results/Measurement/Value", log.addStdout),
+                           ("./Results/NamedMeasurement[@name='Fail Reason']/Value", log.addStderr))
+            yield log.finish()
+
+        skipped = root.findall(".//Test[@Status='notrun']")
+        if skipped:
+            log = yield self.addLog('skipped')
+            for test in skipped:
+                log.addStdout(f'{test.findtext("Name")}\n')
+                self.write_xml(test,
+                               ("./Results/NamedMeasurement[@name='Environment']/Value", log.addHeader),
+                               ("./Results/NamedMeasurement[@name='Command Line']/Value", log.addHeader),
+                               ("./Results/Measurement/Value", log.addStdout),
+                               indent=2)
+                log.addStdout('\n')
+            yield log.finish()
+
+        return cmd.results()
+
+    def write_xml(self, test, *sections, indent=0):
+        for node, log in sections:
+            text = test.findtext(node)
+            text = self.clean_text(text, indent=indent)
+            log(text)
+
+    @staticmethod
+    def clean_text(text, *, indent=0):
+        indent = " " * indent
+        text = text or ''
+        if 'Regex=[' in text:  # clean up annoying CTest output
+            text = text.replace('\n]', ']\n')
+        text = text.strip()
+        text = text.replace('\n', f'\n{indent}')
+        text = f'{indent}{text}\n'
+        return text

master/master.cfg

@@ -27,15 +27,16 @@ from buildbot.steps.shell import SetPropertyFromCommand, ShellCommand
 from buildbot.steps.source.git import Git
 from buildbot.steps.source.github import GitHub
 from buildbot.steps.transfer import FileUpload
-from buildbot.steps.worker import MakeDirectory, SetPropertiesFromEnv
-from buildbot.steps.worker import RemoveDirectory
+from buildbot.steps.worker import MakeDirectory, SetPropertiesFromEnv, RemoveDirectory
 from buildbot.worker import Worker
 from buildbot.www.auth import UserPasswordAuth
 from buildbot.www.authz import Authz
 from buildbot.www.authz.roles import RolesFromUsername
 from buildbot.www.hooks.github import GitHubEventHandler
 from twisted.internet import defer
 
+from custom_steps import CTest
+
 # This is the dictionary that the buildmaster pays attention to. We also use
 # a shorter alias to save typing.
 c = BuildmasterConfig = {}
@@ -183,6 +184,11 @@ class Purpose(Enum):
     llvm_nightly = 3
 
 
+class BuildSystem(Enum):
+    make = 0
+    cmake = 1
+
+
 class BuilderType:
     """A class to encapsulate the settings for a specific Builder.
        (Do not confuse with CMake's 'BUILD_TYPE', which is something else.)
@@ -201,7 +207,7 @@ class BuilderType:
        setup. (If we ever need to do so, compiler should be added to this.)
     """
 
-    def __init__(self, arch, bits, os, halide_branch, llvm_branch, purpose, cmake=True):
+    def __init__(self, arch, bits, os, halide_branch, llvm_branch, purpose, buildsystem=BuildSystem.cmake):
         assert arch in ['arm', 'x86']
         assert bits in [32, 64]
         assert os in ['linux', 'windows', 'osx']
@@ -212,7 +218,7 @@ class BuilderType:
         self.os = os
         self.halide_branch = halide_branch
         self.llvm_branch = llvm_branch
-        self.cmake = cmake
+        self.buildsystem = buildsystem
         self.purpose = purpose
 
         if self.halide_branch:
@@ -271,7 +277,7 @@ class BuilderType:
             # so append that here for clarity
             a.append(f'llvm{LLVM[self.llvm_branch].version.major}')
         a.append(self.halide_target())
-        a.append('cmake' if self.cmake else 'make')
+        a.append(self.buildsystem.name)
         return '-'.join(a)
 
     def builder_label(self):
@@ -686,7 +692,7 @@ def add_env_setup_step(factory, builder_type):
         # for now, pending NVidia investigation.
         env['CUDA_CACHE_DISABLE'] = '1'
 
-        # We don't ever want an Abort, Rerty, Ignore dialog in our tests
+        # We don't ever want an Abort, Retry, Ignore dialog in our tests
         env['HL_DISABLE_WINDOWS_ABORT_DIALOG'] = '1'
 
     factory.addStep(SetProperties(
@@ -1018,68 +1024,53 @@ def add_halide_cmake_test_steps(factory, builder_type):
             test for test in test_labels if not is_time_critical_test(test)]
         exclusive_test_labels = [test for test in test_labels if is_time_critical_test(test)]
 
-        if len(parallel_test_labels):
-            test_set = '|'.join(parallel_test_labels)
-            # Note that we pass cmd as a single string deliberately,
-            # to avoid buildbot escaping issues with the | char
-            cmd = ' '.join(['ctest',
-                            '--build-config', 'Release',
-                            '--output-on-failure',
-                            '--label-regex', '"%s"' % test_set,
-                            '--parallel', '%d' % parallelism])
+        if parallel_test_labels:
+            test_set = ', '.join(parallel_test_labels)
 
             # Build up some special cases to exclude
-            exclude_regex = []
+            exclude_tests = []
             if builder_type.os == 'windows' or builder_type.os == 'linux':
                 # TODO: disable lens_blur on windows for now due to
-                # https://bugs.llvm.org/show_bug.cgi?id=46176
-                # and also due to windows testbots having inadequate GPU RAM
-                #
-                # And also on Linux due to inadequate GPU RAM
-                exclude_regex.append('interpolate')
-                exclude_regex.append('lens_blur')
-                exclude_regex.append('unsharp')
+                #   https://bugs.llvm.org/show_bug.cgi?id=46176
+                #   and also due to Windows testbots having inadequate GPU RAM
+                #   and also due to Linux testbots having inadequate GPU RAM
+                exclude_tests.append('interpolate')
+                exclude_tests.append('lens_blur')
+                exclude_tests.append('unsharp')
 
             if builder_type.os == 'linux' or builder_type.bits == 32:
                 # TODO: disable tutorial_lesson_12_using_the_gpu (both C++ and python) on
                 # linux and 32-bit
-                exclude_regex.append('tutorial_lesson_12')
+                exclude_tests.append('tutorial_lesson_12')
 
             if builder_type.arch == 'arm' or builder_type.bits == 32:
                 # TODO: disable lesson_19 on arm32
                 # https://github.com/halide/Halide/issues/5224
-                exclude_regex.append('tutorial_lesson_19')
-
-            if exclude_regex:
-                cmd = cmd + ' --exclude-regex "%s"' % ('|'.join(exclude_regex))
+                exclude_tests.append('tutorial_lesson_19')
 
             factory.addStep(
-                ShellCommand(name='Test %s Halide_TARGET=%s' % (test_set, halide_target),
-                             description='Test %s Halide_TARGET=%s' % (
-                                 test_set, halide_target),
-                             locks=[performance_lock.access('counting')],
-                             workdir=build_dir,
-                             env=env,
-                             timeout=3600,
-                             command=cmd))
-
-        if len(exclusive_test_labels):
-            test_set = '|'.join(exclusive_test_labels)
-            # Note that we pass cmd as a single string deliberately,
-            # to avoid buildbot escaping issues with the | char
-            cmd = ' '.join(['ctest',
-                            '--build-config', 'Release',
-                            '--output-on-failure',
-                            '--label-regex', '"%s"' % test_set])
+                CTest(name='Test %s Halide_TARGET=%s' % (test_set, halide_target),
+                      description='Test %s Halide_TARGET=%s' % (test_set, halide_target),
+                      locks=[performance_lock.access('counting')],
+                      workdir=build_dir,
+                      env=env,
+                      timeout=3600,
+                      build_config='Release',
+                      labels=parallel_test_labels,
+                      exclude_tests=exclude_tests,
+                      jobs=parallelism))
+
+        if exclusive_test_labels:
+            test_set = ', '.join(exclusive_test_labels)
             factory.addStep(
-                ShellCommand(name='Test %s Halide_TARGET=%s' % (test_set, halide_target),
-                             description='Test %s Halide_TARGET=%s' % (
-                                 test_set, halide_target),
-                             locks=[performance_lock.access('exclusive')],
-                             workdir=build_dir,
-                             env=env,
-                             timeout=3600,
-                             command=cmd))
+                CTest(name='Test %s Halide_TARGET=%s' % (test_set, halide_target),
+                      description='Test %s Halide_TARGET=%s' % (test_set, halide_target),
+                      locks=[performance_lock.access('exclusive')],
+                      workdir=build_dir,
+                      env=env,
+                      timeout=3600,
+                      build_config='Release',
+                      labels=exclusive_test_labels))
 
         if do_apps:
             apps_build_dir = get_halide_build_path("apps")
@@ -1112,30 +1103,22 @@ def add_halide_cmake_test_steps(factory, builder_type):
             # to make full use of all cores, and running in parallel will just slow
             # things down.
 
-            # Note that we pass apps_test_cmd as a single string deliberately,
-            # to avoid buildbot escaping issues with the | char
-            apps_test_cmd = ' '.join(['ctest',
-                                      '--build-config', 'Release',
-                                      '--output-on-failure',
-                                      '--label-exclude', 'slow_tests'])
-
-            exclude_regex = []
+            exclude_tests = []
             if builder_type.os == 'windows':
                 # TODO: disable lens_blur_filter on windows for now due to
                 # https://github.com/halide/Halide/issues/5552
-                exclude_regex.append('lens_blur_filter')
-
-            if exclude_regex:
-                apps_test_cmd += ' --exclude-regex "%s"' % ('|'.join(exclude_regex))
+                exclude_tests.append('lens_blur_filter')
 
             factory.addStep(
-                ShellCommand(name='Test apps for Halide_TARGET=%s' % halide_target,
-                             description='Test apps for Halide_TARGET=%s' % halide_target,
-                             locks=[performance_lock.access('exclusive')],
-                             workdir=apps_build_dir,
-                             env=env,
-                             timeout=3600,
-                             command=apps_test_cmd))
+                CTest(name='Test apps for Halide_TARGET=%s' % halide_target,
+                      description='Test apps for Halide_TARGET=%s' % halide_target,
+                      locks=[performance_lock.access('exclusive')],
+                      workdir=apps_build_dir,
+                      env=env,
+                      timeout=3600,
+                      build_config='Release',
+                      exclude_tests=exclude_tests,
+                      exclude_labels=['slow_tests']))
 
 
 def create_halide_make_factory(builder_type):
@@ -1231,7 +1214,7 @@ def create_halide_cmake_factory(builder_type):
 
 
 def create_halide_factory(builder_type):
-    if builder_type.cmake:
+    if builder_type.buildsystem == BuildSystem.cmake:
         return create_halide_cmake_factory(builder_type)
     else:
         return create_halide_make_factory(builder_type)
@@ -1250,8 +1233,8 @@ def get_interesting_halide_targets():
                 yield arch, bits, os
 
 
-def create_halide_builder(arch, bits, os, halide_branch, llvm_branch, purpose, cmake=True):
-    builder_type = BuilderType(arch, bits, os, halide_branch, llvm_branch, purpose, cmake=cmake)
+def create_halide_builder(arch, bits, os, halide_branch, llvm_branch, purpose, buildsystem=BuildSystem.cmake):
+    builder_type = BuilderType(arch, bits, os, halide_branch, llvm_branch, purpose, buildsystem)
     workers = builder_type.get_worker_names()
     builder = BuilderConfig(name=builder_type.builder_label(),
                             workernames=workers,
@@ -1292,11 +1275,11 @@ def create_halide_builders():
         # Create the builders for testing pull requests to main.
         yield create_halide_builder(arch, bits, os, HALIDE_MAIN, LLVM_MAIN, Purpose.halide_testbranch)
 
-    # Also test Makefiles on x86-linux & osx (but only on Halide master) to ensure they
+    # Also test Makefiles on x86-linux & osx (but only on Halide main) to ensure they
     # stay healthy. (Note: deliberately skip arm-linux, since they are the slowest bots.)
-    yield create_halide_builder('x86', 64, 'linux', HALIDE_MAIN, LLVM_MAIN, Purpose.halide_testbranch, cmake=False)
-    yield create_halide_builder('x86', 32, 'linux', HALIDE_MAIN, LLVM_MAIN, Purpose.halide_testbranch, cmake=False)
-    yield create_halide_builder('x86', 64, 'osx', HALIDE_MAIN, LLVM_MAIN, Purpose.halide_testbranch, cmake=False)
+    yield create_halide_builder('x86', 64, 'linux', HALIDE_MAIN, LLVM_MAIN, Purpose.halide_testbranch, BuildSystem.make)
+    yield create_halide_builder('x86', 32, 'linux', HALIDE_MAIN, LLVM_MAIN, Purpose.halide_testbranch, BuildSystem.make)
+    yield create_halide_builder('x86', 64, 'osx', HALIDE_MAIN, LLVM_MAIN, Purpose.halide_testbranch, BuildSystem.make)
 
     # Test pull requests for Halide master against the current and previous LLVM, for at least one target.
     for llvm_branch in LLVM: