valentinagiusti added a comment. Thanks for the review! You can find my replies inline.
================ Comment at: packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/TestMPXRegisters.py:27 @@ +26,3 @@ + + @skipIfiOSSimulator + @skipIf(compiler="clang") ---------------- labath wrote: > Do we really need the ios simulator decorator here? Is this naturally skipped if all OSs are skipped except for linux? ================ Comment at: packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/TestMPXRegisters.py:29 @@ +28,3 @@ + @skipIf(compiler="clang") + @expectedFailureAll(oslist=["linux"], compiler="gcc", compiler_version=["<", "5"]) + @skipIf(archs=no_match(['amd64', 'i386', 'x86_64'])) ---------------- labath wrote: > I presume this is XFAIL because the compiler does not have the required > features. If that is true then a "skip" result would be more appropriate. True, here a skip would be better. ================ Comment at: packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/TestMPXRegisters.py:30 @@ +29,3 @@ + @expectedFailureAll(oslist=["linux"], compiler="gcc", compiler_version=["<", "5"]) + @skipIf(archs=no_match(['amd64', 'i386', 'x86_64'])) + def test_mpx_registers_with_example_code(self): ---------------- labath wrote: > It shouldn't be necessary to specify `amd64` here. I know some old code does > that, but now we have code in `lldbtest.py` which automatically remaps it to > `x86_64`. ok, I'll remove it then! ================ Comment at: packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/TestMPXRegisters.py:43 @@ +42,3 @@ + + self.runCmd('settings set target.inline-breakpoint-strategy always') + self.addTearDownHook( ---------------- labath wrote: > Why is this necessary? (Also it looks like your cleanup function is the same > as the setup) Sorry this is something I should have cleaned up. ================ Comment at: packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/TestMPXRegisters.py:50 @@ +49,3 @@ + + self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT, + substrs = ["stop reason = breakpoint 1."]) ---------------- labath wrote: > So, this test will fail if run on hardware which does not have the registers > you are testing now (as far as I can tell, that's pretty much all of it). We > should detect that situation (the inferior already has code for that, > apparently), and skip the test. Something like: > ``` > if inferior_exited_with_minus_1: > self.skipTest("blah blah") > ``` > > Good point! I will add this Repository: rL LLVM https://reviews.llvm.org/D24187 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits