Re: [Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-26 Thread Pavel Labath via lldb-commits
On 23 February 2018 at 15:42, Jim Ingham wrote: > BTW, one thing I like about writing dotest.py tests is that it is easy to > craft fairly rich failure messages so if you get errors on systems you don't > have access to or are dealing with something that fails intermittently

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-26 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL326112: Add lldb-test breakpoint command and convert the case-sensitivity test to useā€¦ (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

Re: [Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-23 Thread Jim Ingham via lldb-commits
Sure, as long as we aren't turning the output of the "break set" or other commands into test API - which your version of this lit test does not - then this sort of test seems fine to me. When we were starting out and building up the set of tests available I at least tended to err on the side

Re: [Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-23 Thread Pavel Labath via lldb-commits
On 23 February 2018 at 11:24, Jim Ingham wrote: > To be fair, you could probably have made the dotest.py version of the test > close to as fast by not running a process. The old test was testing that we > got a location for the breakpoint AND hit it. The latter was probably

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-23 Thread Vedant Kumar via Phabricator via lldb-commits
vsk accepted this revision. vsk added a comment. I like that we can customize the command substitutions and the way we dump internal state in this test mode. This offers some added flexibility over the batch mode, IMO. LGTM, thanks! https://reviews.llvm.org/D43686

Re: [Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-23 Thread Jim Ingham via lldb-commits
To be fair, you could probably have made the dotest.py version of the test close to as fast by not running a process. The old test was testing that we got a location for the breakpoint AND hit it. The latter was probably overkill. But the two tests aren't testing the same thing. > On Feb

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-23 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. (and thanks for saving 1 minutes and 30 seconds of my life multiplied by the many times I run the test suite per day). https://reviews.llvm.org/D43686 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-23 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. I was going to suggest the same thing Zach suggested, but I think this fine as is. LGTM. The fact the test is more concise is definitely a win, but I don't think this is the main reason for

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D43686#1017577, @zturner wrote: > Curious if we need lldb-test for this. Could we get by with just using lldb > in batch mode? Obviously I'm not opposed to adding whatever we need to > lldb-test, just want to make sure it's something that

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-23 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. This test has the advantage over running lldb in batch mode that it isolates the tests you write using it from the output format of the break command. The lldb test suite used to check the output of break all over the place, and that meant when I went to change that

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-23 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment. Curious if we need lldb-test for this. Could we get by with just using lldb in batch mode? Obviously I'm not opposed to adding whatever we need to lldb-test, just want to make sure it's something that can't already be done with just lldb. Comment

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

2018-02-23 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: davide, zturner, jingham. The command takes two input arguments: a module to use as a debug target and a file containing a list of commands. The command will execute each of the breakpoint commands in the file and dump the breakpoint state