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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to