Page MenuHomePhabricator

[lldb] Use explicit lldb commands on tests
Needs RevisionPublic

Authored by aadsm on Dec 1 2019, 8:42 PM.

Details

Summary

Not a big deal but might be if in the future we had another command starting with br.
I found this because I had an lldbinit that added a breakpad command.

Event Timeline

aadsm created this revision.Dec 1 2019, 8:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2019, 8:42 PM
clayborg requested changes to this revision.Dec 2 2019, 9:17 AM

We shouldn't be running the test suite that allows your ~/.lldbinit file to be run. This can really hose up many things, so we should change the lldb-vscode test suite to not allow your ~/.lldbinit file to be loaded instead?

This revision now requires changes to proceed.Dec 2 2019, 9:17 AM
aadsm added a comment.Dec 2 2019, 11:03 AM

Yeah, that's what I do in D70882. I still think tests can be more robust by using explicit lldb command names but don't really care much :). I'm happy to abandon it.

jingham added a subscriber: jingham.Dec 2 2019, 4:02 PM

I don't think it is wise to use shortened command names like "br" in a test if you aren't testing shortest command string. We don't guarantee that we will keep all currently unique shortened command names unique in perpetuity...

lanza added a comment.Apr 30 2020, 5:07 PM

Sorry for bump to this old diff, but I agree with both Jim and Greg -- we shouldn't be importing your ~/.lldbinit, but tests shouldn't depend on there never being another br s. This change should land as breakpoint set.

clayborg added inline comments.Apr 30 2020, 6:31 PM
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
353–354

we might as well spell this out completely as "breakpoint set" just in case and nothing can currently replace or alias existing commands.