[lldb-dev] [Lldb-commits] [lldb] r248028 - Make libc++ tests skip themselves if libc++ is not actually loaded in the target
Todd Fiala via lldb-dev
lldb-dev at lists.llvm.org
Wed Oct 21 08:11:15 PDT 2015
I'm in favor of (b). The less user-required setup to do the right thing on
a test suite, the better IMHO. Those actively trying to make sure one or
another c++ library is getting tested will be looking for the output to
validate which std c++ lib(s) ran.
-Todd
On Wed, Oct 21, 2015 at 3:47 AM, Pavel Labath <labath at google.com> wrote:
> [moving this to lldb-dev for more visibility.]
>
> Sorry, I was in a hurry yesterday, so I did not explain myself fully. Let
> me try to elaborate.
>
> > What I am trying to avoid here is getting useless noise in build
> automation where test cases proclaim their failure, which however tells us
> nothing of value as the issue is simply “libc++ not available” vs. “data
> formatters broken” That is an ability I would strongly like to preserve
>
> I agree that we should have an ability to skip libc++ tests if the library
> is not available (similarly for go, etc.). However, I believe there should
> be as little "magic" in that as possible. Otherwise you run into the
> opposite problem where a test passing could mean "everything is ok" or "our
> auto-skipping magic has gone wrong", which I would argue is a worse
> situation than the first one.
>
> Currently, we have a lot of magic in here:
> - self.build() silently builds an executable with a different library even
> though libc++ was requested. (otherwise you wouldn't even be able to list
> the modules of the executable)
> - the test decides to skip itself without giving any indication about
> (sure, it shows up in the "skipped tests" list, but a lot of other stuff
> appears there as well. and I would argue that this is a different
> situation: usually we do skips based on the OS, architecture, or other
> "immutable" characteristics. Presence of libc++ is something that can be
> usually changed by simply installing a package.)
>
> I'd like to propose a few alternative solutions to achieve both of these
> objectives:
>
> a) Use the existing category system for this: libc++ tests get tagged as
> such and the user has to explicitly disable this category to avoid running
> the tests. Potentially, add some code to detect when the user is running
> the test suite without libc++ installed and abort the run with some message
> like "No libc++ detected on your system. Please install libc++ or disable
> libc++ tests with `--disable-category libc++`".
>
> I like this solution because it is easily achievable and has no magic
> involved. However, it requires an action from the user (which I think is a
> good thing, but I see how others may disagree). That's what I meant for
> asking about your "use case". Would you be able to fit it inside this
> framework?
>
> b) Use category tagging as in (a), but auto-disable this category when
> libc++ is missing and print a big fat warning "No libc++ detected, libc++
> tests will not run". What is different from the current state is that this
> libc++ detection would work centrally, which would give us the possibility
> to print the warning (e.g. right before the "Ran XXX test suites" message")
> I like this a bit less, as it is still automatic, but I am fine with it,
> as it would give a clear visual indication of what is happening.
>
>
> What do you think? Do you have another proposal?
>
> I can implement either of those, but I was to get some feedback first..
>
> pl
>
>
>
> On 20 October 2015 at 19:05, Enrico Granata <egranata at apple.com> wrote:
>
>>
>> On Oct 20, 2015, at 10:43 AM, Pavel Labath <labath at google.com> wrote:
>>
>> Hi Enrico,
>>
>> Could you explain what was the motivation behind this change?
>>
>>
>> As per title of the commit, in a process that is using a standard c++
>> library other than libc++, these tests are noise - of course the libc++
>> tests aren’t gonna pass if you don’t have libc++
>>
>> I am asking because, I have just learned that this commit has caused
>> all libc++ tests to be skipped on linux*, silently decreasing test
>> coverage on linux. I would like to replace this with some other
>> mechanism, which is not prone to accidental silent skips, like a
>> dotest flag to skip libc++ or something, but I'd like to understand
>> the original motivation first.
>>
>> pl
>>
>> * the problem seems to be that on linux, we do not have the list of
>> modules until we actually start the process, so this code will not
>> find the library, as it runs before that.
>>
>>
>> The solution might then be to run the process, and then
>> skip_if_library_missing
>> I think we avoid trying to compile the test inferior entirely if we can’t
>> find libc++ however, so you might first want to check if a.out exists at
>> all, and only then proceed all the way to the first breakpoint being hit
>>
>> If is considered a bug then
>> we can look into that separately, but I'd still like to avoid these
>> kinds of skips.
>>
>>
>> What I am trying to avoid here is getting useless noise in build
>> automation where test cases proclaim their failure, which however tells us
>> nothing of value as the issue is simply “libc++ not available” vs. “data
>> formatters broken”
>> That is an ability I would strongly like to preserve
>>
>>
>> On 18 September 2015 at 21:12, Enrico Granata via lldb-commits
>> <lldb-commits at lists.llvm.org> wrote:
>>
>> Author: enrico
>> Date: Fri Sep 18 15:12:52 2015
>> New Revision: 248028
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=248028&view=rev
>> Log:
>> Make libc++ tests skip themselves if libc++ is not actually loaded in the
>> target
>>
>>
>> Modified:
>>
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py
>>
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py
>>
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
>>
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
>>
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/TestDataFormatterLibccMultiMap.py
>>
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/multiset/TestDataFormatterLibcxxMultiSet.py
>>
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
>>
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
>>
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py
>>
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/vbool/TestDataFormatterLibcxxVBool.py
>>
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/vector/TestDataFormatterLibcxxVector.py
>> lldb/trunk/test/lldbutil.py
>>
>> Modified:
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py
>> (original)
>> +++
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py
>> Fri Sep 18 15:12:52 2015
>> @@ -36,6 +36,8 @@ class InitializerListTestCase(TestBase):
>> """Test that that file and class static variables display
>> correctly."""
>> self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
>>
>> + lldbutil.skip_if_library_missing(self, self.target(),
>> lldbutil.PrintableRegex("libc\+\+"))
>> +
>> bkpt =
>> self.target().FindBreakpointByID(lldbutil.run_break_set_by_source_regexp
>> (self, "Set break point at this line."))
>>
>> self.runCmd("run", RUN_SUCCEEDED)
>>
>> Modified:
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py
>> (original)
>> +++
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py
>> Fri Sep 18 15:12:52 2015
>> @@ -37,6 +37,8 @@ class LibcxxIteratorDataFormatterTestCas
>> """Test that libc++ iterators format properly."""
>> self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
>>
>> + lldbutil.skip_if_library_missing(self, self.target(),
>> lldbutil.PrintableRegex("libc\+\+"))
>> +
>> lldbutil.run_break_set_by_file_and_line (self, "main.cpp",
>> self.line, num_expected_locations=-1)
>>
>> self.runCmd("run", RUN_SUCCEEDED)
>>
>> Modified:
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
>> (original)
>> +++
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
>> Fri Sep 18 15:12:52 2015
>> @@ -2,7 +2,7 @@
>> Test lldb data formatter subsystem.
>> """
>>
>> -import os, time
>> +import os, time, re
>> import unittest2
>> import lldb
>> from lldbtest import *
>> @@ -39,6 +39,8 @@ class LibcxxListDataFormatterTestCase(Te
>> def data_formatter_commands(self):
>> """Test that that file and class static variables display
>> correctly."""
>> self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
>> +
>> + lldbutil.skip_if_library_missing(self, self.target(),
>> lldbutil.PrintableRegex("libc\+\+"))
>>
>> lldbutil.run_break_set_by_file_and_line (self, "main.cpp",
>> self.line, num_expected_locations=-1)
>> lldbutil.run_break_set_by_file_and_line (self, "main.cpp",
>> self.line2, num_expected_locations=-1)
>>
>> Modified:
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
>> (original)
>> +++
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
>> Fri Sep 18 15:12:52 2015
>> @@ -35,6 +35,8 @@ class LibcxxMapDataFormatterTestCase(Tes
>> """Test that that file and class static variables display
>> correctly."""
>> self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
>>
>> + lldbutil.skip_if_library_missing(self, self.target(),
>> lldbutil.PrintableRegex("libc\+\+"))
>> +
>> bkpt =
>> self.target().FindBreakpointByID(lldbutil.run_break_set_by_source_regexp
>> (self, "Set break point at this line."))
>>
>> self.runCmd("run", RUN_SUCCEEDED)
>>
>> Modified:
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/TestDataFormatterLibccMultiMap.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/TestDataFormatterLibccMultiMap.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/TestDataFormatterLibccMultiMap.py
>> (original)
>> +++
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/TestDataFormatterLibccMultiMap.py
>> Fri Sep 18 15:12:52 2015
>> @@ -34,6 +34,8 @@ class LibcxxMultiMapDataFormatterTestCas
>> def data_formatter_commands(self):
>> """Test that that file and class static variables display
>> correctly."""
>> self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
>> +
>> + lldbutil.skip_if_library_missing(self, self.target(),
>> lldbutil.PrintableRegex("libc\+\+"))
>>
>> bkpt =
>> self.target().FindBreakpointByID(lldbutil.run_break_set_by_source_regexp
>> (self, "Set break point at this line."))
>>
>>
>> Modified:
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/multiset/TestDataFormatterLibcxxMultiSet.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/multiset/TestDataFormatterLibcxxMultiSet.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/multiset/TestDataFormatterLibcxxMultiSet.py
>> (original)
>> +++
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/multiset/TestDataFormatterLibcxxMultiSet.py
>> Fri Sep 18 15:12:52 2015
>> @@ -34,6 +34,8 @@ class LibcxxMultiSetDataFormatterTestCas
>> def data_formatter_commands(self):
>> """Test that that file and class static variables display
>> correctly."""
>> self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
>> +
>> + lldbutil.skip_if_library_missing(self, self.target(),
>> lldbutil.PrintableRegex("libc\+\+"))
>>
>> bkpt =
>> self.target().FindBreakpointByID(lldbutil.run_break_set_by_source_regexp
>> (self, "Set break point at this line."))
>>
>>
>> Modified:
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
>> (original)
>> +++
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
>> Fri Sep 18 15:12:52 2015
>> @@ -34,6 +34,8 @@ class LibcxxSetDataFormatterTestCase(Tes
>> def data_formatter_commands(self):
>> """Test that that file and class static variables display
>> correctly."""
>> self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
>> +
>> + lldbutil.skip_if_library_missing(self, self.target(),
>> lldbutil.PrintableRegex("libc\+\+"))
>>
>> bkpt =
>> self.target().FindBreakpointByID(lldbutil.run_break_set_by_source_regexp
>> (self, "Set break point at this line."))
>>
>>
>> Modified:
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
>> (original)
>> +++
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
>> Fri Sep 18 15:12:52 2015
>> @@ -38,6 +38,8 @@ class LibcxxStringDataFormatterTestCase(
>> """Test that that file and class static variables display
>> correctly."""
>> self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
>>
>> + lldbutil.skip_if_library_missing(self, self.target(),
>> lldbutil.PrintableRegex("libc\+\+"))
>> +
>> lldbutil.run_break_set_by_file_and_line (self, "main.cpp",
>> self.line, num_expected_locations=-1)
>>
>> self.runCmd("run", RUN_SUCCEEDED)
>>
>> Modified:
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py
>> (original)
>> +++
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py
>> Fri Sep 18 15:12:52 2015
>> @@ -39,6 +39,8 @@ class LibcxxUnorderedDataFormatterTestCa
>> """Test that that file and class static variables display
>> correctly."""
>> self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
>>
>> + lldbutil.skip_if_library_missing(self, self.target(),
>> lldbutil.PrintableRegex("libc\+\+"))
>> +
>> lldbutil.run_break_set_by_source_regexp (self, "Set break point
>> at this line.")
>>
>> self.runCmd("run", RUN_SUCCEEDED)
>>
>> Modified:
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/vbool/TestDataFormatterLibcxxVBool.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/vbool/TestDataFormatterLibcxxVBool.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/vbool/TestDataFormatterLibcxxVBool.py
>> (original)
>> +++
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/vbool/TestDataFormatterLibcxxVBool.py
>> Fri Sep 18 15:12:52 2015
>> @@ -37,6 +37,8 @@ class LibcxxVBoolDataFormatterTestCase(T
>> """Test that that file and class static variables display
>> correctly."""
>> self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
>>
>> + lldbutil.skip_if_library_missing(self, self.target(),
>> lldbutil.PrintableRegex("libc\+\+"))
>> +
>> lldbutil.run_break_set_by_file_and_line (self, "main.cpp",
>> self.line, num_expected_locations=-1)
>>
>> self.runCmd("run", RUN_SUCCEEDED)
>>
>> Modified:
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/vector/TestDataFormatterLibcxxVector.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/vector/TestDataFormatterLibcxxVector.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/vector/TestDataFormatterLibcxxVector.py
>> (original)
>> +++
>> lldb/trunk/test/functionalities/data-formatter/data-formatter-stl/libcxx/vector/TestDataFormatterLibcxxVector.py
>> Fri Sep 18 15:12:52 2015
>> @@ -34,6 +34,8 @@ class LibcxxVectorDataFormatterTestCase(
>> def data_formatter_commands(self):
>> """Test that that file and class static variables display
>> correctly."""
>> self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
>> +
>> + lldbutil.skip_if_library_missing(self, self.target(),
>> lldbutil.PrintableRegex("libc\+\+"))
>>
>> bkpt =
>> self.target().FindBreakpointByID(lldbutil.run_break_set_by_source_regexp
>> (self, "break here"))
>>
>>
>> Modified: lldb/trunk/test/lldbutil.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/lldbutil.py?rev=248028&r1=248027&r2=248028&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/test/lldbutil.py (original)
>> +++ lldb/trunk/test/lldbutil.py Fri Sep 18 15:12:52 2015
>> @@ -7,6 +7,7 @@ They can also be useful for general purp
>> import lldb
>> import os, sys
>> import StringIO
>> +import re
>>
>> # ===================================================
>> # Utilities for locating/checking executable programs
>> @@ -957,3 +958,38 @@ def get_signal_number(signal_name):
>> return signal_number
>> # No remote platform; fall back to using local python signals.
>> return getattr(signal, signal_name)
>> +
>> +class PrintableRegex(object):
>> + def __init__(self, text):
>> + self.regex = re.compile(text)
>> + self.text = text
>> +
>> + def match(self, str):
>> + return self.regex.match(str)
>> +
>> + def __str__(self):
>> + return "%s" % (self.text)
>> +
>> + def __repr__(self):
>> + return "re.compile(%s) -> %s" % (self.text, self.regex)
>> +
>> +def skip_if_callable(test, callable, reason):
>> + if callable(test) == True:
>> + test.skipTest(reason)
>> + return True
>> + return False
>> +
>> +def skip_if_library_missing(test, target, library):
>> + def find_library(target, library):
>> + for module in target.modules:
>> + filename = module.file.GetFilename()
>> + if isinstance(library, str):
>> + if library == filename:
>> + return False
>> + elif hasattr(library, 'match'):
>> + if library.match(filename):
>> + return False
>> + return True
>> + def find_library_callable(test):
>> + return find_library(target, library)
>> + return skip_if_callable(test, find_library_callable, "could not find
>> library matching '%s' in target %s" % (library, target))
>>
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>>
>>
>> Thanks,
>> *- Enrico*
>> 📩 egranata@.com ☎️ 27683
>>
>>
>
--
-Todd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20151021/e0dec056/attachment-0001.html>
More information about the lldb-dev
mailing list