[WIP] Move lldb-mi interpreter tests to LIT

[apolyakov]

Summary

In this patch I move some of interpreter tests from python to LIT(I will move all interpreter test if these are OK). It's a WIP since I want to get your opinion about tests like target-list.test. As you may see, in this test we must run lldb-mi without passing an executable, it means that we can't pass it to lldb-mi through LIT. As a solution, I hardcoded executable name as a.exe. What do you think about this approach? We already have one test with hardcoded executable name - lit/tools/lldb-mi/breakpoint/break-insert.test, but I want to be sure that it's OK.

[stella.stamenova]

lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test

4

One thing to consider here is that any extra parameters passed with -E to the test suite will not propagate to lit at the moment.

I ran into this with some internal testing where we need to pass parameters to the compiler - all of the lldb suite tests (c++ and c) build correctly, but any lit tests that directly invoke the compiler do not because the parameters do not get propagated all the way.

[apolyakov]

lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test

4

Could you give an example of extra parameters? I didn't see them before so I don't completely understand you.

[stella.stamenova]

lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test

4

-E "--sysroot=path/to/sys/root -lc++abi -lunwind"

[apolyakov]

lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test

4

Ok, I think we won't use something like it here. Thank you.

[aprantl]

lit/tools/lldb-mi/interpreter/cli-support/target-list.test

5

That's totally fine, we just need to choose a name that works on all platforms, and a.exe does.

18

Does lldb-mi echo the comment lines? If yes, you need to be careful that FileCheck doesn't match against the input, e.g., by adding {{^}} to the beginning of each CHECK command. If it doesn't, then that's fine.

[apolyakov]

lit/tools/lldb-mi/interpreter/cli-support/target-list.test

18

It doesn't. I passed # CHECK: some_text and got only:

^done
(gdb)

[stella.stamenova]

lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test

4

I think you misunderstood my concern - let's say I have a machine on which I run these tests for a particular architecture that depends on passing these arguments to the tests, so that clang (cxx) correctly complies c++ files. *Before* your change, these arguments would have been propagated to the test in the lldb suite and the code would have build correctly. *After* your change, the code will no longer build correctly.

Essentially, by making these tests lit tests, you are removing support for passing these arguments to the compiler (since the lldb suite supports them and lit does not). It might still be worth making these lit tests for the sake of other benefits, but then such targets will end up having to XFAIL the tests.

If these tests really need to become lit tests and invoke the compiler, I think you also need to add support for passing these arguments to the compiler.

[aprantl]

lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test

4

I think the best way to solve this is by adding the platform-specific flags to the expansion of %cxx in the lit configuration. Would that work here?

[labath]

lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test

4

I am wondering how much do we actually need the lldb-mi tests to run on exotic (non-host) platforms/architectures. My take on these tests is that they should test the functionality from the lldb-mi protocol, and down (only) to the SB API calls. Anything below SB is a black box, which is assumed to be working (tested via other means). In particular, these tests should not care about whether the binary is built with libc++abi or split-dwarf or something else. The default debuggable executable produced by the given compiler should be enough.

In such a world, there is not much value in running the tests on other architectures, as (hopefully) all of those differences are hidden inside liblldb. The existing lldb-mi tests already do not support all the features that other dotest tests do (e.g. running the tests remotely). If getting this working it's just the matter of adding something to the expansion of %cxx, then sure, why not... But otherwise I would not spend too much time engineering a very generic solution.

lit/tools/lldb-mi/interpreter/cli-support/settings-set-target-run-after.test

13

Try:

setting set -- target.run-args --your --args --with --dashes

[probinson]

lit/tools/lldb-mi/interpreter/cli-support/target-list.test

5

Not %t/a.exe ? I thought CWD is sometimes not writeable (or might be the working copy, and we don't want to leave trash there).

[stella.stamenova]

lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test

4

Re: passing the arguments in %cxx
This might work, but now we have to pass arguments in two different ways and this will quickly get confusing, especially if there are other tests that need the same. In general, I think having *one* way to pass arguments to the tests is simplest from a user perspective.

Re: running the tests on only some platforms
As long as the tests don't need to run on all platforms, I think it's fine if they don't support passing additional arguments to the compiler. In that case, I would say the tests should be marked so that they will only run on platforms where we know they will work. Right now there's only a couple of lit LLDB tests that make use of %cxx (and most only run on Windows), but it took me a non-trivial amount of time going through the test infrastructure to figure out when and why arguments were passed (or not). If someone down the line is setting up these tests and they start failing even though they are passing arguments, they'll end up having to redo all of that investigation and there will be no record of it right now.

[apolyakov]

Abandoned since the patch is outdated too much.

lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test

+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# RUN: %cxx -o %t %p/inputs/main.cpp -g
+# RUN: %lldbmi %t < %s | FileCheck %s
+
+# Test lldb-mi CLI support. 'breakpoint set' command.
+
+# Check that we have a valid target created via '%lldbmi %t'.
+# CHECK: ^done
+
+breakpoint set --name main
+# CHECK: ^done
+# CHECK: =breakpoint-created,bkpt={number="1"
+
+-exec-run
+# CHECK: ^running
+# CHECK: =breakpoint-modified,bkpt={number="1"
+# CHECK: *stopped,reason="breakpoint-hit"
+
+-gdb-exit

lit/tools/lldb-mi/interpreter/cli-support/inputs/main.cpp

+//===-- main.cpp ------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include <cstdio>
+
+int
+main(int argc, char const *argv[])
+{
+    printf("argc=%d\n", argc);
+    for (int i = 0; i < argc; ++i)
+        printf("argv[%d]=%s\n", i, argv[i]);
+    return 0;
+}

lit/tools/lldb-mi/interpreter/cli-support/lit.local.cfg

+config.suffixes = ['.test']

lit/tools/lldb-mi/interpreter/cli-support/settings-set-target-run-after.test

+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# RUN: %cxx -o %t %p/inputs/main.cpp -g
+# RUN: %lldbmi %t < %s | FileCheck %s
+
+# Test lldb-mi CLI support. 'setting set target.run-args' command after than
+# target was created.
+
+# Check that we have a valid target created via '%lldbmi %t'.
+# CHECK: ^done
+
+# FIXME: --arg1 causes an error.
+setting set target.run-args arg1 "2nd arg" third_arg fourth="4th arg"
+# CHECK: ^done
+
+-exec-run
+# CHECK: ^running
+# CHECK: @"argc=5\r\n"
+
+-gdb-exit

lit/tools/lldb-mi/interpreter/cli-support/settings-set-target-run-before.test

+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# We should hardcode executable name since at the moment of running of
+# lldb-mi the name must be known.
+# RUN: %cxx -o a.exe %p/inputs/main.cpp -g
+# RUN: %lldbmi < %s | FileCheck %s
+
+# Test lldb-mi CLI support. 'setting set target.run-args' command before than
+# target was created.
+
+# FIXME: --arg1 causes an error.
+setting set target.run-args arg1 "2nd arg" third_arg fourth="4th arg"
+# CHECK: ^done
+
+-file-exec-and-symbols a.exe
+# CHECK: ^done
+
+-exec-run
+# CHECK: ^running
+# CHECK: @"argc=5\r\n"
+
+-gdb-exit

lit/tools/lldb-mi/interpreter/cli-support/target-create.test

+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# We should hardcode executable name since at the moment of running of
+# lldb-mi the name must be known.
+# RUN: %cxx -o a.exe %p/inputs/main.cpp -g
+# RUN: %lldbmi < %s | FileCheck %s
+
+# Test lldb-mi CLI support. 'target create' command.
+
+target create a.exe
+# CHECK: ^done
+
+-break-insert -f main
+# CHECK: ^done,bkpt={number="1"
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
+-gdb-exit

lit/tools/lldb-mi/interpreter/cli-support/target-list.test

+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# We should hardcode executable name since at the moment of running of
+# lldb-mi the name must be known.
+# RUN: %cxx -o a.exe %p/inputs/main.cpp -g
+# RUN: %lldbmi < %s | FileCheck %s
+
+# Test lldb-mi CLI support. 'target list' command.
+
+target list
+# CHECK: ~"No targets.\n"
+
+-file-exec-and-symbols a.exe
+# CHECK: ^done
+
+target list
+# CHECK: ~"Current targets:\n* target #0: {{.*}}a.exe{{.*}}\n"
+
+-gdb-exit