[Lldb-commits] [PATCH] D54731: [lit] Enable the use of custom user-defined lit commands

Tue Nov 27 08:57:30 PST 2018

aprantl requested changes to this revision.
aprantl added a comment.
This revision now requires changes to proceed.

> Currently lit supports running shell commands through the use of the RUN:  prefix. This patch allows individual test suites to install their own run handlers that can do things other than run shell commands.  RUN commands still work as they always do, just that now if a different kind of command appears it will be appropriately sequenced along with the run command.

I'm not convinced that this is the best direction to evolve the LLDB testsuite to. Let me know if I'm missing something; I'm willing to be convinced otherwise :-)

It sounds like the problem you want to solve is having a more flexible build system for tests, and the ability to run python code as part of a tests. That is exactly the feature set that `dotest.py` provides. Tests are written in fully flexible Python, and in order to compile inferiors, we can fan out to a dedicated build system that is really good at compiling programs, namely `make`.

I don't see how any of your stated goals couldn't be achieved within the existing `Makefile.rules`. Establishing a second, parallel way of doing something similar would only serve to bifurcate the test infrastructure and make maintenance a lot harder in the future. It also makes the system more difficult to explain to new developers.

Specifically:

> The commands the user installs can execute arbitrary Python code.

`dotest.py` already does that. Currently, we are using `lit.py` as a test scheduler and `dotest.py` as an LLDB-specific test harness. I think that's reasonable design.

> As such, they can in theory write directly to stdout or stderr, but a well-behaved command should return its stdout and stderr from the function so that this can be reported to the user in a manner consistent with output from RUN lines.
> 
> The motivating use case for this is being able to provide a richer and more powerful syntax by which to compile test programs in LLDB tests. Currently everything is based off of substitutions and explicitly shell commands, but this is problematic when you get into interesting compilation scenarios.

I disagree with this statement. Building tests is done in an explicit, portable build system: `make`. I don't think it is a good idea to *also* add all the complexity of the `dotest.py` tests to the lit-based tests. Lit-based tests are very useful for certain (specifically non-interactive) use-cases, but if you need more build system support, or need to more complex test logic, I'd rather use `make`+`dotest.py`.

> For example, one could imagine wanting to write a test that tested the behavior of the debugger with optimized code. Each driver has different sets of flags that control the optimization behavior.

This is mostly a solved problem with our Makefile system.

> Another example is in cross-compilation scenarios. Certain types of PDB tests don't need to run a process, so the tests can be run anywhere, but they need to be linked with special flags to avoid pulling in system libraries.
> 
> We can try to make substitutions for all of these cases, but it will quickly become unwieldy and you will end up with a command line like: RUN: %cxx %debug %opt %norun, and this still isn't as flexible as you'd like.
> 
> With this patch, we could (in theory) do the compilation directly from Python. Instead of a shell command like above, we could write something like:
> 
> COMPILE: source=%p/Inputs/foo.cpp \
>  COMPILE:     mode=debug \
>  COMPILE:     opt=none \
>  COMPILE:     link=no \
>  COMPILE:     output=%t.o \
>  COMPILE:     clean=yes
>  and let the function figure out how best to do this for each platform. This is similar in spirit to how LLDB's `dotest.py` already works with its platform specific builders, but the mechanism here is general enough that it can be used for anything a test suite wants, not just compiling.

In the dotest tests you generally don't need to write explicit build commands at all. All the platform-specific build logic is implemented once in Makefile.rules and the individual tests merely specify what source files need to be built and whether you want o build a binary or a shared library.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54731/new/

https://reviews.llvm.org/D54731