[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)

[Pavel Labath via lldb-commits]

labath wrote:

> > That bug shows we're not able to create a test for a very simple thing (an unaligned stack pointer) that works reliably for everyone.
> 
> Honestly, based on [#101710 (comment)](https://github.com/llvm/llvm-project/issues/101710#issuecomment-2291180977), I'd say that the mentioned test has a problem of too many redundant dependencies, rather than that it shows the backside of tests universality.

I'm afraid I don't know what you mean by that. How would you go about removing those dependencies?

> 
> > In the end, that also reduces test coverage, because people will start to (even proactively) add REQUIRES: my-system to their tests to avoid breaking other people's CI.
> 
> LLDB tests will still somehow depend on at least three targets in the foreseeable future (Linux/Windows/Darwin). If a test fails or is not supported on Linux target, it's unlikely that it will work on Linux target with remote debugging. So I don't see a perspective of having test coverage reduced because of `UNSUPPORTED: remote-` annotations if they appear.
> 
> > This is important as it slows down the velocity of all developers (old and new).
> 
> I see a situation of a development velocity slowdown if a test the developer wrote is not supposed to work for remote debugging, but is missing a proper annotation for that. For other cases, remote debugging support is claimed as an LLDB feature. If tests are written/executed using the LLDB command line, remote execution shouldn't hinder their execution.
> 
> BTW, most of the cases where tests should've been disabled for remote debugging were the cases where platform mismatch happened. Tests were already disabled for "system-windows", but they were attempted to be run against Linux target. Probably, "system-X" semantics should be redefined to indicate a target platform instead of a host platform, or a feature like "target-system-xxx" should be introduced to show that a test should/shouldn't be run on a Windows/Darwin target regardless of the host. I think this will eliminate the need for `UNSUPPORTED: remote-` annotations for most of the cases.
> 
> Considering the complexity of setting up a remote debugging environment, public buildbots with it will be available https://discourse.llvm.org/t/rfc-lldb-support-remote-run-of-shell-tests/80072/4, https://lab.llvm.org/staging/#/builders/195.
> 
> Also, the case when no new features are added, but existing ones are changed, shouldn't be neglected. In such cases, it is hard to justify disabling tests without finding out the reason.
> 
> > For me, this just sends the wrong message/sets the wrong incentive.
> 
> From my point of view, the current incentive given is that remote debugging compatibility may be ignored when new commits are made since it's something rare enough/hard to test properly (because testing with it requires writing an API test). The ability to run Shell tests remotely (and regular testing) may reduce the barrier for those who want to take remote debugging into account in their patches.

I think the main difference here is that you think of the Shell tests as kind of an end to end test which verifies that a certain (user-facing) debugger feature is works the way its supposed to. I don't. I think that's what the API tests are for. I think the Shell tests should verify that a specific piece of debugger code (which is most likely not directly accessible by the user) does what it is supposed to do. I.e., they're more akin to a unit test (which happens to be driven by shell commands).

If that sounds too abstract, let me give you an example. And ideal "end to end" test would be "this source code, compiled with the same compiler that the end user will use, running on a machine most similar to the one the user would use produces this debugger output". An ideal unit test would be "feeding these bytes (e.g. DWARF opcodes) into this function produces this result". The difference is that in the second case you want to make the test as specific as possible, whereas in the first one you want to make it as variable as possible (so that people can use the environment they care about).

This creates an inherent conflict. If you want the test to be runnable or useful remotely, it needs to be written in the first way. This means compiling for the target of the day, and doing while making sure you don't bake in any assumptions that are only true about the target you work on is very hard. **That** is what ends up creating `REQUIRES` clauses I was talking about. It's not that someone will write a test that doesn't run remotely (although that can happen too). It's that someone will write a test that only works on (e.g.) a mac (maybe local, maybe also remote), and then that test will get a `REQUIRES: mac` clause. Now, when I'm touching the related (or even unrelated code), I may break this test, but I'll have no way of knowing because I'm testing on linux. In the worst case I'll find out only after I submit the patch, in the best case, I'll after I copy my changes to a mac machine to test them out. In either case, it will slow my work down. **That** is the velocity I'm talking about and I **think** this is also the "wrong direction" that Jonas is talking about as well.

OTOH, if someone writes a test that verifies the operation of some feature on a mac **even if the test runs on linux**, then we can be all sure that the feature will stay working. Yes, this creates a risk that the feature will not work on linux (or maybe just remote linux?), but that's why we have API tests, which can verify that the most important features work in all configurations. And there's nothing stopping downstream for adding additional end-to-end tests (ones which might not even fit into the API test suite) for additional validation for the use cases they are supporting.

https://github.com/llvm/llvm-project/pull/95986