[lldb-dev] [Lldb-commits] [lldb] r248028 - Make libc++ tests skip themselves if libc++ is not actually loaded in the target
Pavel Labath via lldb-dev
lldb-dev at lists.llvm.org
Wed Oct 21 03:47:00 PDT 2015
[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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20151021/6129666b/attachment-0001.html>
More information about the lldb-dev
mailing list