Add ability to specify running LLDB API testsuite by json description

[ita-sc]

Hi

This patch adds ability to set up lldb dotest.py command with json format:

[
 {
    "test_pack_name": "check-lldb-one", # <- mandatory
    "test_pack_desc": "description for test pack", # <- mandatory
    "test_compiler": "clang", # <- optional
    "test_arch": "x86", # <- optional
    "test_simulator": "tvos", # <- optional
    "extra_flags": [ # <- optional
      "-static"
    ],
    "skip_categories": [ # <- optional
      "lldb-vscode"
    ],
    "xfail_categories": [ # <- optional
      "watchpoint"
    ],
    "check_all": false # <- mandatory, if we should run it with check-all target
 },
 {...}
]

As result, we will run dotest with additional options
from test_simulator: tvos: --apple-sdk appletvsimulator --platform-name tvos-simulator
from test_compiler: --compiler=clang
from test_arch: --arch x86
from extra_flags: we compile our test binaries with --static.

Note: this patch also adds ability to pass several extra flags to
dotest. It is done by this syntax: start:--option1:option2.

Also we pass skip and xfail categories to dotest.py script

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (ita-sc)

Changes

Hi

This patch adds ability to set up lldb dotest.py command with json format:

[
 {
    "test_pack_name": "check-lldb-one", # <- mandatory
    "test_pack_desc": "description for test pack", # <- mandatory
    "test_compiler": "clang", # <- optional
    "test_arch": "x86", # <- optional
    "test_simulator": "tvos", # <- optional
    "extra_flags": [ # <- optional
      "-static"
    ],
    "skip_categories": [ # <- optional
      "lldb-vscode"
    ],
    "xfail_categories": [ # <- optional
      "watchpoint"
    ],
    "check_all": false # <- mandatory, if we should run it with check-all target
 },
 {...}
]

As result, we will run dotest with additional options
from test_simulator: tvos: --apple-sdk appletvsimulator --platform-name tvos-simulator
from test_compiler: --compiler=clang
from test_arch: --arch x86
from extra_flags: we compile our test binaries with --static.

Note: this patch also adds ability to pass several extra flags to
dotest. It is done by this syntax: start:--option1:option2.

Also we pass skip and xfail categories to dotest.py script

---

Full diff: https://github.com/llvm/llvm-project/pull/89764.diff

3 Files Affected:

• (modified) lldb/packages/Python/lldbsuite/test/dotest.py (+6-1)
• (modified) lldb/test/API/CMakeLists.txt (+67)
• (modified) lldb/test/API/lit.cfg.py (+28)

diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py
index 2ec4a840b91675..1f70c6579a48a8 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -336,7 +336,12 @@ def parseOptionsAndInitTestdirs():
         )
 
     if args.E:
-        os.environ["CFLAGS_EXTRAS"] = args.E
+        args_list = args.E.split(":")
+        # Remove first 'start' argument, as we need it was needed to pass as argument to '-E'
+        if (args_list[0] == "start"):
+          args_list = args_list[1:]
+
+        os.environ["CFLAGS_EXTRAS"] = " ".join(args_list)
 
     if args.dwarf_version:
         configuration.dwarf_version = args.dwarf_version
diff --git a/lldb/test/API/CMakeLists.txt b/lldb/test/API/CMakeLists.txt
index 9196f54ce1ae32..2c368daf1a7665 100644
--- a/lldb/test/API/CMakeLists.txt
+++ b/lldb/test/API/CMakeLists.txt
@@ -195,3 +195,70 @@ add_lit_testsuite(check-lldb-api "Running lldb api test suite"
   ${CMAKE_CURRENT_BINARY_DIR}
   EXCLUDE_FROM_CHECK_ALL
   DEPENDS lldb-api-test-deps)
+
+if(LLDB_API_JSON_CONFIG)
+  file(READ ${LLDB_API_JSON_CONFIG} API_TEST_CONFIGS)
+  set(IDX 0)
+  while(true)
+    macro(set_test_info out_var key)
+      string(JSON ${out_var} ERROR_VARIABLE ERROR_VAR GET ${API_TEST_CONFIGS} ${IDX} ${key})
+      if (NOT ${ERROR_VAR} MATCHES .*NOTFOUND)
+        break()
+      endif()
+    endmacro()
+
+    set_test_info(TEST_NAME test_pack_name)
+    set_test_info(TEST_DESC test_pack_desc)
+    set_test_info(CHECK_ALL_NEEDED check_all)
+
+    set(TO_PARAMS "" CACHE STRING "internal variable to be given to lit config params" FORCE)
+    # function to add separate param to run lit.cfg with.
+    function(add_param key substr)
+      string(JSON RES ERROR_VARIABLE ERROR_VAR GET ${API_TEST_CONFIGS} ${IDX} ${key})
+      if (${ERROR_VAR} MATCHES .*NOTFOUND)
+	      list(APPEND TO_PARAMS "${substr}=${RES}")
+      endif()
+      set(TO_PARAMS ${TO_PARAMS} PARENT_SCOPE)
+    endfunction()
+    # function to add a list of params to run lit.cfg with.
+    function (add_list_param key substr)
+      set(LIST_IDX 0)
+      set(FULL_LINE_TO_ADD "")
+      while (true)
+        string(JSON RES ERROR_VARIABLE ERROR_VAR GET ${API_TEST_CONFIGS} ${IDX} ${key} ${LIST_IDX})
+	if (${RES} MATCHES .*NOTFOUND)
+          break()
+	endif()
+        list(APPEND FULL_LINE_TO_ADD "${RES}")
+        MATH(EXPR LIST_IDX "${LIST_IDX} + 1")
+      endwhile()
+      if (NOT "${FULL_LINE_TO_ADD}" STREQUAL "")
+	string (REPLACE ";" ":" FULL_LINE_TO_ADD_STR "${FULL_LINE_TO_ADD}")
+        list(APPEND TO_PARAMS "${substr}=${FULL_LINE_TO_ADD_STR}")
+        set(TO_PARAMS ${TO_PARAMS} PARENT_SCOPE)
+      endif()
+    endfunction()
+
+    add_param(test_compiler "test_compiler")
+    add_param(test_arch "test_arch")
+    add_param(test_simulator "lldb-run-with-simulator")
+    add_list_param(extra_flags "extra_flags")
+    add_list_param(skip_categories "skip_categories")
+    add_list_param(xfail_categories "xfail_categories")
+
+    # Create test targets
+    if (CHECK_ALL_NEEDED)
+      add_lit_testsuite(${TEST_NAME}  ${TEST_DESC}
+	    ${CMAKE_CURRENT_BINARY_DIR}
+	    PARAMS ${TO_PARAMS}
+            DEPENDS lldb-api-test-deps)
+    else()
+      add_lit_testsuite(${TEST_NAME}  ${TEST_DESC}
+	    ${CMAKE_CURRENT_BINARY_DIR}
+	    PARAMS ${TO_PARAMS}
+            EXCLUDE_FROM_CHECK_ALL
+            DEPENDS lldb-api-test-deps)
+    endif()
+    MATH(EXPR IDX "${IDX}+1")
+  endwhile()
+endif()
diff --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index 9d6775917e1370..29fd8442133f75 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -213,6 +213,11 @@ def delete_module_cache(path):
 if is_configured("test_arch"):
     dotest_cmd += ["--arch", config.test_arch]
 
+lldb_test_arch_param = lit_config.params.get("test_arch")
+if lldb_test_arch_param:
+    dotest_cmd += ["--arch", lldb_test_arch_param]
+
+
 if is_configured("lldb_build_directory"):
     dotest_cmd += ["--build-dir", config.lldb_build_directory]
 
@@ -230,6 +235,11 @@ def delete_module_cache(path):
 if is_configured("test_compiler"):
     dotest_cmd += ["--compiler", config.test_compiler]
 
+lldb_test_compiler_param = lit_config.params.get("test_compiler")
+if lldb_test_compiler_param:
+    dotest_cmd += ["--compiler", lldb_test_compiler_param]
+
+
 if is_configured("dsymutil"):
     dotest_cmd += ["--dsymutil", config.dsymutil]
 
@@ -293,6 +303,24 @@ def delete_module_cache(path):
     # already configured.
     dotest_cmd.extend(shlex.split(config.dotest_lit_args_str))
 
+lldb_test_extra_flags = lit_config.params.get("extra_flags")
+if lldb_test_extra_flags:
+    # FIXME: dotest does not support several -E options.
+    # FIXME: start in the beginning is a workaround to make sure command not start with '--'
+    dotest_cmd += ["-E", rf'start:{lldb_test_extra_flags}']
+
+
+lldb_test_skip_categories = lit_config.params.get("skip_categories")
+if lldb_test_skip_categories:
+    for skip_category in lldb_test_skip_categories.split(":"):
+        print(skip_category)
+        dotest_cmd += ["--skip-category", skip_category]
+
+lldb_test_xfail_categories = lit_config.params.get("xfail_categories")
+if lldb_test_xfail_categories:
+    for xfail_category in lldb_test_xfail_categories.split(":"):
+        dotest_cmd += ["--xfail-category", xfail_category]
+
 # Load LLDB test format.
 sys.path.append(os.path.join(config.lldb_src_root, "test", "API"))
 import lldbtest

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[bulbazord]

This change does 2 things, both of which are quite substantial on their own. Can you break them up into two changes to make it easier to review?

Higher level feedback:
Please add some documentation for these new changes. The change to dotest.py is minor but a short line about it somewhere (perhaps in the test suite docs?) would be helpful. As for the JSON Config, there is documentation on common build options where this might be useful.

Some feedback on the individual changes:

1. Changing dotest so that it can accept multiple things for -E. You solved this by inserting a special token (start:) that tells dotest to interpret it differently. Could you not instead change the action for -E to be append? That way E is already a list of arguments. I see that start is there to prevent an issue when the argument starts with -- right? Is that an issue if you add quotes? What about if you do something like -E "--first -Second --third"? Does that work?

2. It looks like you're expected to pass the JSON config file into CMake which does some processing on it to fill out lit.cfg right? Does this mean I'd have to reconfigure if I change the JSON at all? I think it would be easier if we could instead teach dotest.py to read the JSON configuration. The CMake code would then either accept a user-provided JSON file or build a default one.

[JDevlieghere]

I'm curious to understand the motivation behind this a bit more. Specifically, how are the JSON files different from creating separate lit.site.cfg.py for the different test suite? Downstream we generate separate separate lit.site.cfg.py for different test runs, and then point lit towards them. Could you achieve the same with adding another configuration layer in between?