[CMake] Unify and relayer testing

[JDevlieghere]

Summary

This patch restructures part of LLDB's testing configuration:

1. I moved the test dependencies up the chain so every dotest dependency becomes a lit dependency as well. It wouldn't make sense for dotest to have other dependencies when it's being run by lit. Lit on the other hand can still specify extra dependencies.
2. I replaced as much generator expressions with variables as possible. This is consistent with the rest of LLVM and doesn't break generators that support multiple targets (MSVC, Xcode). This wasn't a problem before, but now we need to expand the dotest arguments in the lit configuration and there's only one test suite even with multiple targets.
3. I moved lldb-dotest into it's own directory under utils since there's no need anymore for it to located under test/.

[JDevlieghere]

test/CMakeLists.txt

129

We'll have to do the same thing here, but the debugserver is more pressing as it's keeping GreenDragon red (http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/)

[stella.stamenova]

test/CMakeLists.txt

138

I am wondering if it's possible to make the logic here simpler. Since we need to handle each of the properties that can contain TARGET_FILE, it is starting to get rather complicated

I think you can actually use LLDB_DOTEST_ARGS_STR for the check-lldb-single target (since it creates a project that should be correctly substituted), so the only other place that could create issues is the lldb-dotest script. Since the tests are run as part of lit now, do we still need the lldb-dotest script?

[JDevlieghere]

test/CMakeLists.txt

138

I started out that route, but how would that work for check-lldb-single with multiple targets? I'm not worried about lldb-dotest, we can do the same trick as for llvm-lit(see llvm/utils/llvm-lit/CMakeLists.txt).

[stella.stamenova]

test/CMakeLists.txt

138

I cannot speak for other generators that support multiple configurations (though I suspect it is similar), but for Visual Studio, if the path to a tool contained CMAKE_CFG_INTDIR (which is $(Configuration)), when the project was built, $(Configuration) would be replaced with the configuration that is currently being used which is exactly what we want anyway.

Obviously, some additional testing will be required to verify that this is the case always.

[JDevlieghere]

• Relayer and unify test dependencies between lit and dotest
• Extract lldb dotest to utils.
• Replace generator expressions with configuration variables

[JDevlieghere]

@stella.stamenova Would you mind applying this locally and verify if it works for the MSVC generator? It seems to work for the Xcode generator but it's always possible that I missed something.

[labath]

Looks fine to me, but I don't use multi-config generators. I'll let you and Stella work out the details there.

[stella.stamenova]

I am going to run the tests after you make the last few changes. I want to make sure the compiler path is no longer being overwritten by the test cmake before I run them.

test/CMakeLists.txt

82–86

I think you can get rid of LLDB_EXECUTABLE_PATH_ARGS now and just set these properties in LDLB_TEST_COMMON_ARGS instead.

95

You actually don't need this anymore. When this was being created per configuration, we needed to overwrite the value of the compiler, but regardless of whether it is custom, it will point to the correct path. You can remove the entire if/else.

138

Why not change the path where it is set originally? Then you won't need the if/else here.

[JDevlieghere]

• Address review feedback Stella

[stella.stamenova]

The tests passed, but check-single was not created correctly. I added a comment above.

test/CMakeLists.txt

155

LLDB_DOTEST_ARGS are passed here, but they were not in the generated project file when I tested this change. I suspect this has to do with setting the PARENT_SCOPE a couple of lines before. Why do we need that?

[JDevlieghere]

test/CMakeLists.txt

155

We need it in order to have the arguments available in the lit subdirectory. Did it work without the scope? We could eliminate the need for the PARENT_SCOPE by using a global property:

SET_PROPERTY(GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY "${LLDB_DOTEST_ARGS}")

and

GET_PROPERTY(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)

[aprantl]

Is there a way to land a partial version of this patch to un-block the green dragon bots and keep iterating on the check-single target separately? Or do other bots depend on check-single working correctly? I'm really uneasy about the fact that we are loosing signal from the bots while they are red. (This is not meant to say that one platform is more important than another, I'm just searching for a way to get all the bots functioning again as quickly as possible, and "no" is an acceptable answer :-).

[stella.stamenova]

In its current version, the patch won't *break* the windows build (the previous change broke the build as well as tests), so if it will unblock the bots, Jonas could check it in and then fix check-single in a follow up patch. I think it needs to be explicit if this goes in as-is that it IS breaking check-single though.

[aprantl]

If that's okay with you I would suggest landing this as is to unblock the bots, and we'll keep working on fixing the check-single target ASAP, too.

[stella.stamenova]

If by as-is you mean with a big comment in the config file before the creation of check-single that it is broken, then yes, let's get the bots unblocked and then fix the issue.

[aprantl]

If by as-is you mean with a big comment in the config file before the creation of check-single that it is broken, then yes, let's get the bots unblocked and then fix the issue.

Sounds good. Thanks!

[JDevlieghere]

I didn't realize check-lldb-single was also broken for ninja/make. Using the property indeed fixed it, so we can land this without the caveat.

[labath]

I remember hearing some people use the check-lldb-single target, but it had
to do with running the tests against some remote devices/stubs that did not
handle multiple connections very well.

However, the way it seems to me, with the transition to lit, this target
will have to go away sooner or later (it still invokes dotest directly). If
there is a substantial effort involved in making it work, then maybe the
time to drop it is now. It will still be possible to run the tests in this
mode by invoking dotest directly. (and if this is something that we really
need to support in the future, we should identify who these people are, and
what requirements they have).

[JDevlieghere]

update diff pro forma

[stella.stamenova]

I still need to verify this works with VS on Windows, but feel free to check in to unblock the bots

test/CMakeLists.txt

set(LLDB_TEST_COMMON_ARGS
   -u CXXFLAGS
   -u CFLAGS
   )
 
 # We need two properties here, because they are used for different purposes. When we are generating
 # one file per configuration for lldb-dotest, we want the paths to be configuration specific. However,
 # when we are generating a single lit file, the file itself should not be per configuration and the paths
 # contained inside should be generic also.
 set(LLDB_EXECUTABLE_PATH_ARGS
   --executable $<TARGET_FILE:lldb>
   --dsymutil $<TARGET_FILE:dsymutil>
   )
 set(LLDB_EXECUTABLE_PATH_ARGS_STR
   --executable ${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb${CMAKE_EXECUTABLE_SUFFIX}
   --dsymutil ${LLVM_RUNTIME_OUTPUT_INTDIR}/dsymutil${CMAKE_EXECUTABLE_SUFFIX}
   -C ${LLDB_TEST_C_COMPILER}
   )
 
 # There's an additional complication which is that when the compiler is NOT a custom compiler, we need to
 # make sure to get the configuration specific path as well
 if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER)
   list(APPEND LLDB_EXECUTABLE_PATH_ARGS -C $<TARGET_FILE:clang>)
 else()
   list(APPEND LLDB_EXECUTABLE_PATH_ARGS -C ${LLDB_TEST_C_COMPILER})
 endif()
 
 if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
   # All tests are currently flaky on Windows, so rerun them all once when they fail.
   set(LLDB_TEST_COMMON_ARGS ${LLDB_TEST_COMMON_ARGS} --rerun-all-issues)
 

 if(LLDB_CODESIGN_IDENTITY)
   list(APPEND LLDB_TEST_COMMON_ARGS --codesign-identity "${LLDB_CODESIGN_IDENTITY}")
 endif()
 
 # The framework path is passed to the test arguments as $<TARGET_FILE_DIR:liblldb>. This won't work in the
 # LLDB_DOTEST_ARGS_STR when using a generator that supports multiple configurations such as Visual Studio,
 # but since the framework is currently confined to Darwin/Apple, we can leave it as is.
 if(LLDB_BUILD_FRAMEWORK)
   list(APPEND LLDB_TEST_COMMON_ARGS --framework $<TARGET_FILE_DIR:liblldb>)
 endif()
 
 if (NOT ${CMAKE_SYSTEM_NAME} MATCHES "Windows|Darwin")
   list(APPEND LLDB_TEST_COMMON_ARGS
     --env ARCHIVER=${CMAKE_AR} --env OBJCOPY=${CMAKE_OBJCOPY})
 endif()
 
-# In some cases, DEBUGSERVER_PATH is expressed as $<TARGET_FILE:debugserver>. This won't work in the
-# LLDB_DOTEST_ARGS_STR when using a generator that supports multiple configurations such as Visual Studio,
-# but since debugserver is currently confined to Darwin/Apple, we can leave it as is.
 if(CMAKE_HOST_APPLE)
+  if (DEBUGSERVER_PATH STREQUAL "$<TARGET_FILE:debugserver>")
+    list(APPEND LLDB_EXECUTABLE_PATH_ARGS --server ${DEBUGSERVER_PATH})
+    list(APPEND LLDB_EXECUTABLE_PATH_ARGS_STR --server ${LLVM_RUNTIME_OUTPUT_INTDIR}/debugserver${CMAKE_EXECUTABLE_SUFFIX})
+  else()
     list(APPEND LLDB_TEST_COMMON_ARGS --server ${DEBUGSERVER_PATH})
   endif()
+endif()
 
 if(SKIP_DEBUGSERVER)
   list(APPEND LLDB_TEST_COMMON_ARGS --out-of-tree-debugserver)
 endif()
 
 set(LLDB_DOTEST_ARGS ${LLDB_TEST_COMMON_ARGS};${LLDB_EXECUTABLE_PATH_ARGS};${LLDB_TEST_USER_ARGS})
 set(LLDB_DOTEST_ARGS_STR ${LLDB_TEST_COMMON_ARGS};${LLDB_EXECUTABLE_PATH_ARGS_STR};${LLDB_TEST_USER_ARGS})
 
 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_custom_target(check-lldb)
 
 # Generate a wrapper for dotest.py in the bin directory.