Skip tests that are failed on linux with gcc-4.9.2

[chying]

Summary

• add decorator functions to skip test on specific os, architecture and version of comipler
• skip failing test with gcc-4.9.2 on linux

[sivachandra]

test/lldbtest.py

765

Could we just name this function "skipIf" and use it directly to decorate tests? May be we could eliminate all skipIf* decorators in favor of this, but I have not thought through that thoroughly. Atleast, it seems to me that things like skipIfLinuxGcc, skipIfGcc and skipIfClang can be eliminated. You will ofcourse have to change the logic in the 'fn' function below. And ofcourse you need not have to remove/modify the existing decorators in this patch itself.

One thing we might loose is the ability to "git grep xxx" to get all tests decorated with xxx.

[chying]

test/lldbtest.py

765

yes, this function could be used to decorate tests directly as is.
Like @skipIf("bn", "linux", "gcc", ['>=','4.9'], ['i386','x86'])

My understanding is that by providing a various of wrapper functions like skipIfLinuxGcc, skipIfClang, we got better readability of code, and also as you mentioned, it's easier to find all tests decorated with certain compiler or host.
Maybe it's similar reasoning behind ExpectedFailureGcc/Icc/Clang

We could definitely re-factor current skipif* functions to call this function instead of having common code in each function.

[sivachandra]

test/lldbtest.py

765

Please note that I am not taking ownership of this review. I do not have strong opinions about the changes and would like others to chime in and drive.

IMO, @skipIfLinuxGcc('bugnumber', ['>=', '4.9'], ['i386']) is not any more readable than @skipIf("bn", "linux", "gcc", ['>=','4.9'], ['i386','x86']).

Also, one can still git grep, but have to put slightly more effort. For example, instead of doing

git grep skipIfLinux

one will now have to do

git grep "skipIf(.*linux.*)"

[labath]

Slightly off topic, but I would like to ask what is the guideline for
using skipIf vs. expectedFailure? Do we have one? (If not maybe we
should make one)

I was under the impression that expectedFailureXXX is to be used for
tests that are failing due to a bug in LLDB, whereas skipIfXXX are for
situations where the test simply does not apply to the platform
environment in question (e.g. debugserver tests on linux, etc). If
that is the case, then these tests should be XFAILed rather then
skipped.
Is that so ?

For the readability issue, I would propose the following syntax:

@skipIf("bugnumber", lambda x: compilerIs("gcc>=4.9") &&
platformIs("linux") && architectureIs("aarch64", "x86_64"))

I think this would be more versatile that the previous solutions, and
you don't need to litter the code with so many decorators.
What do you think about that?

cheers,
pl

[chying]

I think your understanding is correct, based on pytest documentation:
"A skip means that you expect your test to pass unless the environment (e.g. wrong Python interpreter, missing dependency) prevents it to run. And xfail means that your test can run but you expect it to fail because there is an implementation problem."

Do we know if all these failures are caused by lldb bugs or possibly gcc-4.9.2 bugs?

Another difference I would like to point out is that, the tests will still be run if marked as xfail.
And sometimes it takes longer to run failing tests. For example, TestMiExec.py takes 164s to complete with 4 errors(with gcc4.9.2 and i386). It makes the overall test time longer. That's why I chose skip over xfail here.

Using lambda functions is a good idea. We could potentially combine multiple decorators to one.
For example, @skipIf('bn', lambda:(platform(['FreeBSD'])) or (platform(['linux']) and compiler('gcc')))

Please let me know if anyone has different ideas.

[emaste]

We could definitely re-factor current skipif* functions to call this function instead of having common code in each function.

Yes, we should at least do that. When I added FreeBSD support to the tests I spent some time to refactor the expectedFailure cases to avoid the copypasta, but for some reason didn't take on the skipIf tests. I don't have a strong opinion on skipIfFreeBSD vs skipIf(... "FreeBSD" ...)

I was under the impression that expectedFailureXXX is to be used for tests that are failing due to a bug in LLDB, whereas skipIfXXX are for situations where the test simply does not apply to the platform environment in question (e.g. debugserver tests on linux, etc).

Yes, this is consistent with my understanding, with the caveat that we also (ab)use skip for tests that take excessively long to fail / timeout, or that consistently crash (as that case is not conveniently handled by the infrastructure).

[vharron]

Please make suggested changes and submit.

test/api/multithreaded/TestMultithreaded.py

36

Agreeing with previous comments, these should be expectedFailureLinuxGcc

Since Gcc tests aren't run on OSX, should we assume these are just expectedFailureGcc?

test/functionalities/inferior-crashing/TestInferiorCrashing.py

82

Did you forget brackets?

archs=["i386"]

[chying]

Mark these tests with xfail instead of skip

-Add decorator to xfail specified platform/compiler/architecture

[chying]

In D8851#158407, @vharron wrote:

Since Gcc tests aren't run on OSX, should we assume these are just expectedFailureGcc?

Since the code will check against target platform, and we don't intend to xfail those tests with Android target, I added new xfail function to take platform and compiler and architecture.

[sivachandra]

test/lldbtest.py

759

If this is dead code currently, I would suggest you use it atleast once. TestVectorTypesFormatting is a good candidate that I know of.

[chying]

Add one usage of the new skipIf decorator as suggested.
Add more description of decorator usage.

test/api/multithreaded/TestMultithreaded.py

def test_breakpoint_callback(self):
         """Test the that SBBreakpoint callback is invoked when a breakpoint is hit. """
         self.build_and_test('driver.cpp test_breakpoint_callback.cpp',
                             'test_breakpoint_callback')
 
     @skipIfi386
     @skipIfRemote
     @skipIfLinuxClang # buildbot clang version unable to use libstdc++ with c++11
     @skipIfNoSBHeaders
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["x86_64"])
     def test_sb_api_listener_event_description(self):
         """ Test the description of an SBListener breakpoint event is valid."""
         self.build_and_test('driver.cpp listener_test.cpp test_listener_event_description.cpp',
                             'test_listener_event_description')
         pass
 
     @skipIfi386
     @skipIfRemote
     @skipIfLinuxClang # buildbot clang version unable to use libstdc++ with c++11
     @skipIfNoSBHeaders
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["x86_64"])
     def test_sb_api_listener_event_process_state(self):
         """ Test that a registered SBListener receives events when a process
             changes state.
         """
         self.build_and_test('driver.cpp listener_test.cpp test_listener_event_process_state.cpp',
                             'test_listener_event_process_state')
         pass
 
 
     @skipIfi386
     @skipIfRemote
     @skipIfLinuxClang # buildbot clang version unable to use libstdc++ with c++11
     @skipIfNoSBHeaders
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["x86_64"])
     def test_sb_api_listener_resume(self):
         """ Test that a process can be resumed from a non-main thread. """
         self.build_and_test('driver.cpp listener_test.cpp test_listener_resume.cpp',
                             'test_listener_resume')
         pass
 
     def build_and_test(self, sources, test_name, args = None):
         """ Build LLDB test from sources, and run expecting 0 exit code """

test/expression_command/test/TestExprs.py

class BasicExprCommandsTestCase(TestBase):
     @unittest2.expectedFailure("llvm.org/pr17135 <rdar://problem/14874559> APFloat::toString does not identify the correct (i.e. least) precision.")
     def test_floating_point_expr_commands(self):
         self.build_and_run()
 
         self.expect("expression 2.234f",
             patterns = ["\(float\) \$.* = 2\.234"])
         # (float) $2 = 2.234
 
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_many_expr_commands(self):
         self.build_and_run()
 
         self.expect("expression 2",
             patterns = ["\(int\) \$.* = 2"])
         # (int) $0 = 1
 
         self.expect("expression 2ull",
def test_many_expr_commands(self):
         # (const char *) $7 = ...
 
         self.expect("expression argv[0]",
             substrs = ["(const char *)", 
                        "a.out"])
         # (const char *) $8 = 0x... "/Volumes/data/lldb/svn/trunk/test/expression_command/test/a.out"
 
     @python_api_test
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_evaluate_expression_python(self):
         """Test SBFrame.EvaluateExpression() API for evaluating an expression."""
         self.buildDefault()
 
         exe = os.path.join(os.getcwd(), "a.out")
 
         target = self.dbg.CreateTarget(exe)
         self.assertTrue(target, VALID_TARGET)

test/functionalities/data-formatter/data-formatter-synth/TestDataFormatterSynth.py

class SynthDataFormatterTestCase(TestBase):
     @skipUnlessDarwin
     @dsym_test
     def test_with_dsym_and_run_command(self):
         """Test data formatter commands."""
         self.buildDsym()
         self.data_formatter_commands()
 
     @dwarf_test
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["x86_64","i386"])
     def test_with_dwarf_and_run_command(self):
         """Test data formatter commands."""
         self.buildDwarf()
         self.data_formatter_commands()
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)

test/functionalities/data-formatter/vector-types/TestVectorTypesFormatting.py

class VectorTypesFormattingTestCase(TestBase):
     @dsym_test
     def test_with_dsym_and_run_command(self):
         """Check that vector types format properly"""
         self.buildDsym()
         self.propagate_test_commands()
 
     # rdar://problem/14035604
     @dwarf_test
-    @skipIfGcc # gcc don't have ext_vector_type extension
+    @skipIf(compiler='gcc') # gcc don't have ext_vector_type extension
     def test_with_dwarf_and_run_command(self):
         """Check that vector types format properly"""
         self.buildDwarf()
         self.propagate_test_commands()
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)

test/functionalities/inferior-crashing/TestInferiorCrashing.py

def test_inferior_crashing_expr_dwarf(self):
         self.inferior_crashing_expr()
 
     @skipUnlessDarwin
     def test_inferior_crashing_step_dsym(self):
         """Test that lldb functions correctly after stepping through a crash."""
         self.buildDsym()
         self.inferior_crashing_step()
 
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_inferior_crashing_step_dwarf(self):
         """Test that stepping after a crash behaves correctly."""
         self.buildDwarf()
         self.inferior_crashing_step()
 
     @skipUnlessDarwin
     def test_inferior_crashing_step_after_break_dsym(self):
         """Test that stepping after a crash behaves correctly."""
         self.buildDsym()
         self.inferior_crashing_step_after_break()
 
     @skipIfFreeBSD # llvm.org/pr16684
     def test_inferior_crashing_step_after_break_dwarf(self):
         """Test that lldb functions correctly after stepping through a crash."""
         self.buildDwarf()
         self.inferior_crashing_step_after_break()
 
     @skipUnlessDarwin
     def test_inferior_crashing_expr_step_and_expr_dsym(self):
         """Test that lldb expressions work before and after stepping after a crash."""
         self.buildDsym()
         self.inferior_crashing_expr_step_expr()
 
     @expectedFailureFreeBSD('llvm.org/pr15989') # Couldn't allocate space for the stack frame
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_inferior_crashing_expr_step_and_expr_dwarf(self):
         """Test that lldb expressions work before and after stepping after a crash."""
         self.buildDwarf()
         self.inferior_crashing_expr_step_expr()
 
     def set_breakpoint(self, line):
         lldbutil.run_break_set_by_file_and_line (self, "main.c", line, num_expected_locations=1, loc_exact=True)
 

test/functionalities/inline-stepping/TestInlineStepping.py

def test_with_dsym_and_python_api(self):
         """Test stepping over and into inlined functions."""
         self.buildDsym()
         self.inline_stepping()
 
     @python_api_test
     @dwarf_test
     @expectedFailureFreeBSD('llvm.org/pr17214')
     @expectedFailureIcc # Not really a bug.  ICC combines two inlined functions.
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_with_dwarf_and_python_api(self):
         """Test stepping over and into inlined functions."""
         self.buildDwarf()
         self.inline_stepping()
 
     @skipUnlessDarwin
     @python_api_test
     @dsym_test
     def test_step_over_with_dsym_and_python_api(self):
         """Test stepping over and into inlined functions."""
         self.buildDsym()
         self.inline_stepping_step_over()
 
     @python_api_test
     @dwarf_test
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_step_over_with_dwarf_and_python_api(self):
         """Test stepping over and into inlined functions."""
         self.buildDwarf()
         self.inline_stepping_step_over()
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)

test/functionalities/memory/read/TestMemoryRead.py

class MemoryReadTestCase(TestBase):
     @skipUnlessDarwin
     @dsym_test
     def test_memory_read_with_dsym(self):
         """Test the 'memory read' command with plain and vector formats."""
         self.buildDsym()
         self.memory_read_command()
 
     @dwarf_test
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_memory_read_with_dwarf(self):
         """Test the 'memory read' command with plain and vector formats."""
         self.buildDwarf()
         self.memory_read_command()
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)

test/lldbtest.py

 
 def expectedFailureCompiler(compiler, compiler_version=None, bugnumber=None):
     if compiler_version is None:
         compiler_version=['=', None]
     def fn(self):
         return compiler in self.getCompiler() and self.expectedCompilerVersion(compiler_version)
     return expectedFailure(fn, bugnumber)
 
+# provide a function to xfail on defined oslist, compiler version, and archs
+# if none is specified for any argument, that argument won't be checked and thus means for all
+# for example,
+# @expectedFailureAll, xfail for all platform/compiler/arch,
+# @expectedFailureAll(compiler='gcc'), xfail for gcc on all platform/architecture
+# @expectedFailureAll(bugnumber, ["linux"], "gcc", ['>=', '4.9'], ['i386']), xfail for gcc>=4.9 on linux with i386
+def expectedFailureAll(bugnumber=None, oslist=None, compiler=None, compiler_version=None, archs=None):
+    def fn(self):
+        return ((oslist is None or self.getPlatform() in oslist) and
+                (compiler is None or (compiler in self.getCompiler() and self.expectedCompilerVersion(compiler_version))) and
+                self.expectedArch(archs))
+    return expectedFailure(fn, bugnumber)
+
 # to XFAIL a specific clang versions, try this
 # @expectedFailureClang('bugnumber', ['<=', '3.4'])
 def expectedFailureClang(bugnumber=None, compiler_version=None):
     return expectedFailureCompiler('clang', compiler_version, bugnumber)
 
 def expectedFailureGcc(bugnumber=None, compiler_version=None):
     return expectedFailureCompiler('gcc', compiler_version, bugnumber)
 
def wrapper(*args, **kwargs):
         compiler = self.getCompiler()
         platform = sys.platform
         if "clang" in compiler and "linux" in platform:
             self.skipTest("skipping because Clang is used on Linux")
         else:
             func(*args, **kwargs)
     return wrapper
 
+# provide a function to skip on defined oslist, compiler version, and archs
+# if none is specified for any argument, that argument won't be checked and thus means for all
+# for example,
+# @skipIf, skip for all platform/compiler/arch,
+# @skipIf(compiler='gcc'), skip for gcc on all platform/architecture
+# @skipIf(bugnumber, ["linux"], "gcc", ['>=', '4.9'], ['i386']), skip for gcc>=4.9 on linux with i386
+
+# TODO: refactor current code, to make skipIfxxx functions to call this function
+def skipIf(bugnumber=None, oslist=None, compiler=None, compiler_version=None, archs=None):
+    def fn(self):
+        return ((oslist is None or self.getPlatform() in oslist) and
+                (compiler is None or (compiler in self.getCompiler() and self.expectedCompilerVersion(compiler_version))) and
+                self.expectedArch(archs))
+    return skipTestIfFn(fn, bugnumber, skipReason="skipping because os:%s compiler: %s %s arch: %s"%(oslist, compiler, compiler_version, archs))
+
+def skipTestIfFn(expected_fn, bugnumber=None, skipReason=None):
+    def skipTestIfFn_impl(func):
+        @wraps(func)
+        def wrapper(*args, **kwargs):
+            from unittest2 import case
+            self = args[0]
+            if expected_fn(self):
+               self.skipTest(skipReason)
+            else:
+                func(*args, **kwargs)
+        return wrapper
+    if callable(bugnumber):
+        return skipTestIfFn_impl(bugnumber)
+    else:
+        return skipTestIfFn_impl
+
 def skipIfGcc(func):
     """Decorate the item to skip tests that should be skipped if building with gcc ."""
     if isinstance(func, type) and issubclass(func, unittest2.TestCase):
         raise Exception("@skipIfGcc can only be used to decorate a test method")
     @wraps(func)
     def wrapper(*args, **kwargs):
         from unittest2 import case
         self = args[0]
def expectedCompiler(self, compilers):
             return True
 
         for compiler in compilers:
             if compiler in self.getCompiler():
                 return True
 
         return False
 
+    def expectedArch(self, archs):
+        """Returns True iff any element of archs is a sub-string of the current architecture."""
+        if (archs == None):
+            return True
+
+        for arch in archs:
+            if arch in self.getArchitecture():
+                return True
+
+        return False
+
     def getRunOptions(self):
         """Command line option for -A and -C to run this test again, called from
         self.dumpSessionInfo()."""
         arch = self.getArchitecture()
         comp = self.getCompiler()
         if arch:
             option_str = "-A " + arch
         else:

test/tools/lldb-mi/control/TestMiExec.py

 """
 Test lldb-mi -exec-xxx commands.
 """
 
 import lldbmi_testcase
 from lldbtest import *
 import unittest2
 
 class MiExecTestCase(lldbmi_testcase.MiTestCaseBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @lldbmi_test
     @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
     @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_lldbmi_exec_abort(self):
         """Test that 'lldb-mi --interpreter' works for -exec-abort."""
 
         self.spawnLldbMi(args = None)
 
         # Test that -exec-abort fails on invalid process
         self.runCmd("-exec-abort")
         self.expect("\^error,msg=\"Command 'exec-abort'\. Invalid process during debug session\"")
def test_lldbmi_exec_abort(self):
         # Test that program may be aborted again
         self.runCmd("-exec-abort")
         self.expect("\^done")
         self.expect("\*stopped,reason=\"exited-normally\"")
 
     @lldbmi_test
     @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
     @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_lldbmi_exec_arguments_set(self):
         """Test that 'lldb-mi --interpreter' can pass args using -exec-arguments."""
 
         self.spawnLldbMi(args = None)
 
         # Load executable
         self.runCmd("-file-exec-and-symbols %s" % self.myexe)
         self.expect("\^done")
def test_lldbmi_exec_arguments_set(self):
         #self.runCmd("-data-evaluate-expression argv[4]")
         #self.expect("\^done,value=\"fourth=\\\\\\\"4th arg\\\\\\\"\"")
         self.runCmd("-interpreter-exec command \"print argv[4]\"")
         self.expect("\"fourth=\\\\\\\"4th arg\\\\\\\"\"")
 
     @lldbmi_test
     @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
     @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_lldbmi_exec_arguments_reset(self):
         """Test that 'lldb-mi --interpreter' can reset previously set args using -exec-arguments."""
 
         self.spawnLldbMi(args = None)
 
         # Load executable
         self.runCmd("-file-exec-and-symbols %s" % self.myexe)
         self.expect("\^done")
def test_lldbmi_exec_next(self):
         # FIXME: no error is returned
         self.runCmd("-exec-next --frame 10")
         #self.expect("\^error: Frame index 10 is out of range")
 
     @lldbmi_test
     @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
     @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
     @expectedFailurei386 #xfail to get buildbot green, failing config: i386 binary running on ubuntu 14.04 x86_64
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_lldbmi_exec_next_instruction(self):
         """Test that 'lldb-mi --interpreter' works for instruction stepping."""
 
         self.spawnLldbMi(args = None)
 
         # Load executable
         self.runCmd("-file-exec-and-symbols %s" % self.myexe)
         self.expect("\^done")
def test_lldbmi_exec_step_instruction(self):
         # Test that an invalid --frame is handled
         # FIXME: no error is returned
         self.runCmd("-exec-step-instruction --frame 10")
         #self.expect("\^error: Frame index 10 is out of range")
 
     @lldbmi_test
     @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
     @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
+    @expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc", compiler_version=[">=","4.9"], archs=["i386"])
     def test_lldbmi_exec_finish(self):
         """Test that 'lldb-mi --interpreter' works for -exec-finish."""
 
         self.spawnLldbMi(args = None)
 
         # Load executable
         self.runCmd("-file-exec-and-symbols %s" % self.myexe)
         self.expect("\^done")