Cleanup a few more PR36048 skips

[dblaikie]

Summary

Hopefully these were also fixed bi r343545 /
7fd4513920d2fed533ad420976529ef43eb42a35

[aprantl]

Nice!

[teemperor]

Small note: gmodules test are never run on Linux, so you actually have to run them on macOS (or I think FreeBSD) to know whether the tests work.

[dblaikie]

In D112165#3100608, @teemperor wrote:

Small note: gmodules test are never run on Linux, so you actually have to run them on macOS (or I think FreeBSD) to know whether the tests work.

Yeah, I'll admit I didn't test this, but seemed consistent with the other changes/cleanup - did this cause any breakage you know of?

Could gmodules be tested on Linux? (I had originally really hoped/tried to encourage gmodules to be implemented in a way that'd be compatible with Split DWARF but it never quite got there, unfortunately... would've made more overlap in functionality/testability/portability, I think)

(I should setup a build environment on my Macbook Pro, but haven't got around to it as yet)

[teemperor]

In D112165#3100929, @dblaikie wrote:

In D112165#3100608, @teemperor wrote:

Small note: gmodules test are never run on Linux, so you actually have to run them on macOS (or I think FreeBSD) to know whether the tests work.

Yeah, I'll admit I didn't test this, but seemed consistent with the other changes/cleanup - did this cause any breakage you know of?

Could gmodules be tested on Linux? (I had originally really hoped/tried to encourage gmodules to be implemented in a way that'd be compatible with Split DWARF but it never quite got there, unfortunately... would've made more overlap in functionality/testability/portability, I think)

(I should setup a build environment on my Macbook Pro, but haven't got around to it as yet)

There was just one test on macOS with data-formatter-cpp having defining something that is also in a system header (already fixed that). Also I think watching Green Dragon is enough. There would be a macOS CI if anyone actually cared about Green Dragon being always green.

The tests could in theory be run on Linux just fine, the problem is they just won't test anything. For better or worse (actually, just for the worse), the gmodules testing pretty much relies on re-running all normal API tests on -fmodules -fcxx-modules -gmodules -fimplicit-module-maps compiled test sources. But only about 5 of the 1000 API tests even define a module. So the remaining 995 tests just rerun their test as-is unless they include by accident a system header that has a modulemap on the system. No Linux distribution I know comes with modulemaps for its libc so no gmodules functionality will be exercised on the other 995 tests. So gmodules was just disabled as it just made the test suite longer without any benefit. Also I think there were some problems with enabling -fmodules on setups where we had a libc++ module but no modularized libc on the system. I think @labath knows the rationale best.

[labath]

Actually, I've successfully purged most of this from my memory. However, what Raphael said is consistent with what little I remember.

It would be better if the gmodules tests had their own (semi-)dedicated tests that were independent of the system libraries. Then, they could run pretty much anywhere.

[dblaikie]

In D112165#3102055, @teemperor wrote:

In D112165#3100929, @dblaikie wrote:

In D112165#3100608, @teemperor wrote:

Small note: gmodules test are never run on Linux, so you actually have to run them on macOS (or I think FreeBSD) to know whether the tests work.

Yeah, I'll admit I didn't test this, but seemed consistent with the other changes/cleanup - did this cause any breakage you know of?

Could gmodules be tested on Linux? (I had originally really hoped/tried to encourage gmodules to be implemented in a way that'd be compatible with Split DWARF but it never quite got there, unfortunately... would've made more overlap in functionality/testability/portability, I think)

(I should setup a build environment on my Macbook Pro, but haven't got around to it as yet)

There was just one test on macOS with data-formatter-cpp having defining something that is also in a system header (already fixed that). Also I think watching Green Dragon is enough. There would be a macOS CI if anyone actually cared about Green Dragon being always green.

The tests could in theory be run on Linux just fine, the problem is they just won't test anything. For better or worse (actually, just for the worse), the gmodules testing pretty much relies on re-running all normal API tests on -fmodules -fcxx-modules -gmodules -fimplicit-module-maps compiled test sources. But only about 5 of the 1000 API tests even define a module. So the remaining 995 tests just rerun their test as-is unless they include by accident a system header that has a modulemap on the system. No Linux distribution I know comes with modulemaps for its libc so no gmodules functionality will be exercised on the other 995 tests. So gmodules was just disabled as it just made the test suite longer without any benefit. Also I think there were some problems with enabling -fmodules on setups where we had a libc++ module but no modularized libc on the system. I think @labath knows the rationale best.

ooh, yeah - that dose sound like things could benefit from more intentional testing - be faster for Apple folks (rather than running the whole test suite over again in another config) and portable (so better for everyone)...

lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py

class CppDataFormatterTestCase(TestBase):
     mydir = TestBase.compute_mydir(__file__)
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break at.
         self.line = line_number('main.cpp', '// Set break point at this line.')
 
-    @skipIf(debug_info="gmodules",
-            bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048")
     def test_with_run_command(self):
         """Test that that file and class static variables display correctly."""
         self.build()
         self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
         lldbutil.run_break_set_by_file_and_line(
             self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True)
 

lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py

class GlobalsDataFormatterTestCase(TestBase):
     mydir = TestBase.compute_mydir(__file__)
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break at.
         self.line = line_number('main.cpp', '// Set break point at this line.')
 
-    @skipIf(debug_info="gmodules",
-            bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048")
     def test_with_run_command(self):
         """Test that that file and class static variables display correctly."""
         self.build()
         self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
         lldbutil.run_break_set_by_file_and_line(
             self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True)
 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py

def setUp(self):
         self.line2 = line_number('main.cpp',
                                  '// Set second break point at this line.')
         self.line3 = line_number('main.cpp',
                                  '// Set third break point at this line.')
         self.line4 = line_number('main.cpp',
                                  '// Set fourth break point at this line.')
 
     @add_test_categories(["libc++"])
-    @skipIf(debug_info="gmodules",
-            bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048")
     def test_with_run_command(self):
         """Test that that file and class static variables display correctly."""
         self.build()
         self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
         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(