Hey all,

I've been looking at some of the test failures on Windows and this led me to 
realize that there are at least several tests that are duplicated between 
lldb-suite and the lit tests. This appears to have been on purpose circa 2016 
as a proof of concept for moving tests from lldb-suite to lit. I think this is 
confusing and we should pick a set (lit or lldb-suite) and remove the second 
set. Also, if we decide to stick with the lit tests, I think they will need to 
be updated as right now they are not all functioning as expected.

For example, the test TestCallStdStringFunction exists both for lit and 
lldb-suite. It is expected to fail on Windows because windows does not 
correctly support expressions. However, the test fails in lldb-suite and 
*passes* in lit and it passes for the wrong reason (see details below). I 
suspect there may be other places in the duplicated tests where we think we've 
checked something, but we really haven't validated it correctly.

My suggestion is that we remove the lit versions of the duplicated tests rather 
than fixing them as the lldb-suite set appears to be working correctly.

Thanks,
-Stella

P.S. Here are the details on TestCallStdStringFunction:

Here is what the test attempts to do:

breakpoint set --file call-function.cpp --line 52
run
print str
# CHECK: Hello world
print str.c_str()
# CHECK: Hello world

In the lldb-suite version these CHECKs would have verified the output of the 
print immediately, but because of how lit works, these are verified together at 
the end. Since the executable itself prints "Hello World" a couple of times, 
even though the print expressions fail, "Hello World" can be found twice in the 
output, so the test succeeds.

Moreover, the test sets a breakpoint that it expects to hit before calling the 
two print statements. In the lldb-suite version, the test verifies that the 
breakpoint was set, this version doesn't and it happens to fail to set the 
breakpoint. So when the test is calling "print", the executable has already run 
through the end, so even if expressions worked correctly on windows, this would 
have failed since we would have made the call after the executable finished. At 
the very least, this test needs an additional CHECK statement to verify that 
either the breakpoint was set or it was hit.

Looking at the other duplicated tests, we have the potential for similar 
issues. They also all use CHECK rather then CHECK-DAG, so we should at least 
update them to use CHECK-DAG.
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Reply via email to