clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
So you should revert any clang format changes that are not in modified lines of
source, mostly revert a lot of lines in lldb-vscode.cpp.
================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:19
-
- @skipIfWindows
- @skipUnlessDarwin
- @skipIfRemote
- def test_modules_event(self):
- program_basename = "a.out.stripped"
+ def get_modules(self, program_basename):
program= self.getBuildArtifact(program_basename)
----------------
Name should be more descriptive. Maybe "setup_test_common" is a better name
================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:19
-
- @skipIfWindows
- @skipUnlessDarwin
- @skipIfRemote
- def test_modules_event(self):
- program_basename = "a.out.stripped"
+ def get_modules(self, program_basename):
program= self.getBuildArtifact(program_basename)
----------------
clayborg wrote:
> Name should be more descriptive. Maybe "setup_test_common" is a better name
So all of these tests can re-use this function if we switch it up a bit. Here
is what I was thinking:
```
def run_test(self, symbol_basename, expect_debug_info_size):
program_basename = "a.out.stripped"
program = self.getBuildArtifact(program_basename)
self.build_and_launch(program)
functions = ['foo']
breakpoint_ids = self.set_function_breakpoints(functions)
self.assertEquals(len(breakpoint_ids), len(functions), 'expect one
breakpoint')
self.continue_to_breakpoints(breakpoint_ids)
active_modules = self.vscode.get_active_modules()
program_module = active_modules[program_basename]
self.assertIn(program_basename, active_modules, '%s module is in active
modules' % (program_basename))
self.assertIn('name', program_module, 'make sure name is in module')
self.assertEqual(program_basename, program_module['name'])
self.assertIn('path', program_module, 'make sure path is in module')
self.assertEqual(program, program_module['path'])
self.assertTrue('symbolFilePath' not in program_module, 'Make sure
a.out.stripped has no debug info')
self.assertEqual('Symbols not found.', program_module['symbolStatus'])
symbols_path = self.getBuildArtifact(symbol_basename)
self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" "%s"' %
(program, symbols_path)))
def checkSymbolsLoaded():
active_modules = self.vscode.get_active_modules()
program_module = active_modules[program_basename]
return 'Symbols loaded.' == program_module['symbolStatus']
def checkSymbolsLoadedWithSize():
active_modules = self.vscode.get_active_modules()
program_module = active_modules[program_basename]
symbolsStatus = program_module['symbolStatus']
symbol_regex = re.compile(r"Symbols loaded.
\([0-9]+(\.[0-9]*)?[KMG]?B\)")
return symbol_regex.match(program_module['symbolStatus'])
if expect_debug_info_size:
self.waitUntil(checkSymbolsLoadedWithSize)
else:
self.waitUntil(checkSymbolsLoaded)
```
Then your tests would be:
```
@skipIfWindows
@skipIfRemote
def test_module_event(self):
# Mac or linux.
# On mac, if we load a.out as our symbol file, we will use DWARF with .o
files and we will
# have debug symbols, but we won't see any debug info size because all of
the DWARF
# sections are in .o files.
# On other platforms, we expect a.out to have debug info, so we will expect
a size.
expect_debug_info_size = platform.system() != 'Darwin'
return run_test("a.out", expect_debug_info_size)
@skipIfWindows
@skipUnlessDarwin
@skipIfRemote
def test_module_event_dsym(self):
# Darwin only test with dSYM file.
# On mac, if we load a.out.dSYM as our symbol file, we will have debug
symbols and we
# will have DWARF sections added to the module, so we will expect a size.
return run_test("a.out.dSYM", True)
```
This should cover both mac and non windows correctly.
================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:20
+ def get_modules(self, program_basename):
program= self.getBuildArtifact(program_basename)
self.build_and_launch(program)
----------------
add a space after program:
```
program = self.getBuildArtifact(program_basename)
```
================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:32
+ @skipIfWindows
+ @skipUnlessDarwin
+ @skipIfRemote
----------------
Remove @skipUnlessDarwin here. This should work on linux.
================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:59
+ # it will end up using the executable and the .o files as the debug
info.
+ # We won't see any debug information size for this case since all of
the debug info sections are in the .o files.
+ self.assertEqual('Symbols loaded.', program_module['symbolStatus'])
----------------
wrap comment to 80 chars
================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:335
+ debug_info_size += section.GetFileByteSize();
+ if (section_name.startswith(".apple") || section_name.startswith("__apple"))
+ debug_info_size += section.GetFileByteSize();
----------------
yes, merge the if statements
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83731/new/
https://reviews.llvm.org/D83731
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
