WIP: [LIT] Have lit run the lldb test suite

[JDevlieghere]

Summary

This is the initial attempt (v1) to run the lldb test suite with lit, using the custom LLDB test format. Here every`Test*.py` is seen as a single test by lit. As suggested by pavel this circumvents lldb-dotest (though the configuration file now contains part of that logic).

[JDevlieghere]

This isn't meant to be checked-in as is, however I'm looking for feedback as early as possible.

There are currently two problems with the current diff:

• ./bin/llvm-lit ../llvm/tools/lldb/lit/Suite/ doesn't work, and I haven't figured out why yet.
• We'd run the (do)test-suite twice, once as part lit and once as part of check-lldb.

[JDevlieghere]

• Have lldb test format live in lldb repo

[labath]

In D45333#1058916, @JDevlieghere wrote:

This isn't meant to be checked-in as is, however I'm looking for feedback as early as possible.

There are currently two problems with the current diff:

• ./bin/llvm-lit ../llvm/tools/lldb/lit/Suite/ doesn't work, and I haven't figured out why yet.
• We'd run the (do)test-suite twice, once as part lit and once as part of check-lldb.

I think this is quite close to a working diff. The "running twice" issue can be fixed easily, I presume, and the rest of the issues I see are just minor details.

For the llvm-lit issue, I think it would be best if it worked like:

llvm-lit  .../lldb/packages/Python/lldbsuite/test

as then (I hope) I could also specify some folder of that and run only the tests under that subfolder. However, I think we could live without that, initially.

lit/Suite/lit.cfg

17 ↗

(On Diff #141608)

The testcases symlink thingy will not work on windows. I think you'll have to put the full path here.

lit/Suite/lldbtest.py

45 ↗

(On Diff #141608)

I think -p should match agains the full file name (i.e., you should be able to just pass testFile here).

46 ↗

(On Diff #141608)

It looks like executeCommand accepts a list of args for a command as well, so it might be best to keep cmd as such.

[JDevlieghere]

• Feedback Pavel
• Makes check-lldb invoke *only* lit.

[JDevlieghere]

The only outstanding issue is calling llvm-lit with the path to the test suite. Does anybody with more lit experience know how we can solve this?

lit/Suite/lldbtest.py

46 ↗

(On Diff #141608)

Jup, the (now removed) print statement was just to make it easier for me to debug.

[davide]

Sorry for getting in here late. This seems to be a great improvement on the state of the art, and given it's only enabled for the CMake build, I see little harm not going forward with it.
In particular this gives us:

1. Proper SIGTERM handling. This was a longstanding problem where you hit ctrl+c and dotest keeps spawning process. I tested this locally and can confirm it works
2. Some tests are failing with timeouts. because lldb sets a timeout. The new strategy sets a timeout of 0 so it kind of makes this problem going away. OTOH, we can set a timeout per-directory in case it's really needed.

FWIW, I'm not particularly worried about not being able to run this on single tests. I used lldb-dotest for the purpose for a while and it works like a charm. I'm inclined to get this in and back port it to our downstream consumer (swift) as it would be of great benefit there too.

[labath]

This looks good.

Do you have any plans on how will this work with the XCode build (which is kind of a prerequisite to start removing stuff from dotest)?

[JDevlieghere]

In D45333#1065332, @labath wrote:

This looks good.

Do you have any plans on how will this work with the XCode build (which is kind of a prerequisite to start removing stuff from dotest)?

I haven't spend too much time worrying about Xcode yet. Before we can remove anything we'll need to make sure that we have feature parity, at least for the driver part. I think that'll give us enough time to come up with a solution. :-)

[alberto_magni]

Hi Jonas,

This change is causing a cmake generation-time error on Windows when using Visual Studio as generator:
-G "Visual Studio 15 2017 Win64" -Thost=x64

The error is:

CMake Error in tools/lldb/test/CMakeLists.txt:
  Evaluation file to be written multiple times for different configurations
  or languages with different content:

    D:/build/master_win/tools/lldb/test/../lit/Suite/lit.site.cfg


CMake Error in tools/lldb/test/CMakeLists.txt:
  Evaluation file to be written multiple times for different configurations
  or languages with different content:

    D:/build/master_win/tools/lldb/test/../lit/Suite/lit.site.cfg


CMake Error in tools/lldb/test/CMakeLists.txt:
  Evaluation file to be written multiple times for different configurations
  or languages with different content:

    D:/build/master_win/tools/lldb/test/../lit/Suite/lit.site.cfg

… and so on

Notice everything runs fine when using Ninja as a Generator and clang-cl.exe as compiler:

-G Ninja -DCMAKE_CXX_COMPILER=clang-cl.exe -DCMAKE_C_COMPILER=clang-cl.exe

Would it be possible for you to repro this ?
Let me know if I can help with this.

[ted]

@JDevlieghere I'm seeing the same issue as @alberto_magni . The Visual Studio cmake generator gives errors. We're reverting it internally, but it needs to be fixed or reverted ASAP.

The issue could be this set of code from test/CMakeLists.txt:

configure_file(
  ${CMAKE_CURRENT_SOURCE_DIR}/../lit/Suite/lit.site.cfg.in
  ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg
  )
file(GENERATE
  OUTPUT
  ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg
  INPUT
  ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg
  )

The equivalent from clang/test/CMakeLists.txt is:

configure_lit_site_cfg(
  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
  MAIN_CONFIG
  ${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.py
  )

configure_lit_site_cfg(
  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.py.in
  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg.py
  MAIN_CONFIG
  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.cfg.py
  )

[stella.stamenova]

Since Visual Studio as a Generator supports multiple build configurations, each of the CMakeLists.txt that correspond to lit tests also need to include handling of the build mode unless you can guarantee that all the properties are set correctly before test/CMakeLists.txt is processed. It would be safer, of course, to not rely on build order and always set the correct properties. This is to create project files that allow the build mode to be specified at build/compile time of the project rather than configuration time when CMake is called.

So you will need to add something along the lines of (Note: I think these are all the properties you will need, but I have not tested the change, so you will need to make sure this covers all the properties):

if (CMAKE_CFG_INTDIR STREQUAL ".")
  set(LLVM_BUILD_MODE ".")
else ()
  set(LLVM_BUILD_MODE "%(build_mode)s")
endif ()

string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR ${LLVM_RUNTIME_OUTPUT_INTDIR})

I suspect you will also need to add handling for the custom c/cxx compiler options. You can look at: lldb/lit/lit.site.cfg.in and lldb/lit/CMakeLists.txt for the details.

lldb/trunk/lit/Suite/lit.cfg

+# -*- Python -*-
+
+# Configuration file for the 'lit' test runner.
+
+import os
+
+import lit.formats
+
+# name: The name of this test suite.
+config.name = 'lldb-Suite'
+
+# suffixes: A list of file extensions to treat as test files.
+config.suffixes = ['.py']
+
+# test_source_root: The root path where tests are located.
+# test_exec_root: The root path where tests should be run.
+config.test_source_root = os.path.join(config.lldb_src_root, 'packages','Python','lldbsuite','test')
+config.test_exec_root = config.test_source_root
+
+# Build dotest command.
+dotest_cmd = [config.dotest_path, '-q']
+dotest_cmd.extend(config.dotest_args_str.split(';'))
+
+# Load LLDB test format.
+sys.path.append(os.path.join(config.lldb_src_root, "lit", "Suite"))
+import lldbtest
+
+# testFormat: The test format to use to interpret tests.
+config.test_format = lldbtest.LLDBTest(dotest_cmd)

lldb/trunk/lit/Suite/lit.site.cfg.in

+@LIT_SITE_CFG_IN_HEADER@
+
+config.test_exec_root = "@LLVM_BINARY_DIR@"
+config.llvm_src_root = "@LLVM_SOURCE_DIR@"
+config.llvm_obj_root = "@LLVM_BINARY_DIR@"
+config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
+config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_build_mode = "@LLVM_BUILD_MODE@"
+config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
+config.lldb_obj_root = "@LLDB_BINARY_DIR@"
+config.lldb_src_root = "@LLDB_SOURCE_DIR@"
+config.target_triple = "@TARGET_TRIPLE@"
+config.python_executable = "@PYTHON_EXECUTABLE@"
+config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
+config.dotest_args_str = "@LLDB_DOTEST_ARGS@"
+
+# Support substitution of the tools and libs dirs with user parameters. This is
+# used when we can't determine the tool dir at configuration time.
+try:
+    config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
+    config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
+    config.llvm_build_mode = config.llvm_build_mode % lit_config.params
+except KeyError as e:
+    key, = e.args
+    lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
+
+# Let the main config do the real work.
+lit_config.load_config(config, "@LLDB_SOURCE_DIR@/lit/Suite/lit.cfg")

lldb/trunk/lit/Suite/lldbtest.py

+from __future__ import absolute_import
+import os
+
+import subprocess
+import sys
+
+import lit.Test
+import lit.TestRunner
+import lit.util
+from lit.formats.base import TestFormat
+
+
+class LLDBTest(TestFormat):
+    def __init__(self, dotest_cmd):
+        self.dotest_cmd = dotest_cmd
+
+    def getTestsInDirectory(self, testSuite, path_in_suite, litConfig,
+                            localConfig):
+        source_path = testSuite.getSourcePath(path_in_suite)
+        for filename in os.listdir(source_path):
+            # Ignore dot files and excluded tests.
+            if (filename.startswith('.') or filename in localConfig.excludes):
+                continue
+
+            # Ignore files that don't start with 'Test'.
+            if not filename.startswith('Test'):
+                continue
+
+            filepath = os.path.join(source_path, filename)
+            if not os.path.isdir(filepath):
+                base, ext = os.path.splitext(filename)
+                if ext in localConfig.suffixes:
+                    yield lit.Test.Test(testSuite, path_in_suite +
+                                        (filename, ), localConfig)
+
+    def execute(self, test, litConfig):
+        if litConfig.noExecute:
+            return lit.Test.PASS, ''
+
+        if test.config.unsupported:
+            return (lit.Test.UNSUPPORTED, 'Test is unsupported')
+
+        testPath, testFile = os.path.split(test.getSourcePath())
+        cmd = self.dotest_cmd + [testPath, '-p', testFile]
+
+        try:
+            out, err, exitCode = lit.util.executeCommand(
+                cmd,
+                env=test.config.environment,
+                timeout=litConfig.maxIndividualTestTime)
+        except lit.util.ExecuteCommandTimeoutException:
+            return (lit.Test.TIMEOUT, 'Reached timeout of {} seconds'.format(
+                litConfig.maxIndividualTestTime))
+
+        if exitCode:
+            return lit.Test.FAIL, out + err
+
+        passing_test_line = 'RESULT: PASSED'
+        if passing_test_line not in out and passing_test_line not in err:
+            msg = ('Unable to find %r in dotest output:\n\n%s%s' %
+                   (passing_test_line, out, err))
+            return lit.Test.UNRESOLVED, msg
+
+        return lit.Test.PASS, ''

lldb/trunk/test/CMakeLists.txt

 add_python_test_target(check-lldb-single
   ${LLDB_SOURCE_DIR}/test/dotest.py
   "--no-multiprocess;${LLDB_DOTEST_ARGS}"
   "Testing LLDB with args: ${LLDB_DOTEST_ARGS}"
   )
 
 # If tests crash cause LLDB to crash, or things are otherwise unstable, or if machine-parsable
 # output is desired (i.e. in continuous integration contexts) check-lldb-single is a better target.
-add_python_test_target(check-lldb
-  ${LLDB_SOURCE_DIR}/test/dotest.py
-  "-q;${LLDB_DOTEST_ARGS}"
-  "Testing LLDB (parallel execution, with a separate subprocess per test)"
-  )
+add_custom_target(check-lldb)
 
 # Generate a wrapper for dotest.py in the bin directory.
 # We need configure_file to substitute variables.
 configure_file(
   lldb-dotest.in
   ${CMAKE_CURRENT_BINARY_DIR}/lldb-dotest.configured
   )
 # We need this to expand the generator expressions.
 file(GENERATE
   OUTPUT
   $<TARGET_FILE_DIR:lldb>/lldb-dotest
   INPUT
   ${CMAKE_CURRENT_BINARY_DIR}/lldb-dotest.configured
   )
 # Make this a custom target.
 add_custom_target(lldb-dotest)
 add_dependencies(lldb-dotest ${LLDB_TEST_DEPS})
 
+configure_file(
+  ${CMAKE_CURRENT_SOURCE_DIR}/../lit/Suite/lit.site.cfg.in
+  ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg
+  )
+file(GENERATE
+  OUTPUT
+  ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg
+  INPUT
+  ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg
+  )
+
 # If we're building with an in-tree clang, then list clang as a dependency
 # to run tests.
 if (TARGET clang)
   add_dependencies(check-lldb clang)
 endif()
 
 # LLD is required to link test executables on Windows.
 if (CMAKE_SYSTEM_NAME MATCHES "Windows")