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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to