Page MenuHomePhabricator

[lldb] Use explicit lldb commands on tests

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



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.

Diff Detail

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
353–354 ↗(On Diff #231632)

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

aadsm updated this revision to Diff 302180.Nov 1 2020, 3:54 PM

Use set and rebase

clayborg accepted this revision.Nov 2 2020, 3:45 PM
This revision is now accepted and ready to land.Nov 2 2020, 3:45 PM
lanza resigned from this revision.Aug 13 2021, 11:51 AM
wallace resigned from this revision.Aug 13 2021, 11:55 AM