Yea, removing them is probably fine. On Tue, Jul 17, 2018 at 11:14 AM Stella Stamenova <sti...@microsoft.com> wrote:
> 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
