[Lldb-commits] [PATCH] add tests for lldb-mi

[dawn at burble.org]

Thank you for your review!

On Mon, Nov 24, 2014 at 11:18:40AM +0000, Abid, Hafiz wrote:
> Hi Dawn,
> I had a quick look at the tests and I think those will be useful addition. I have
> the following questions though.
> 
> 1. I am wondering why have you restricted tests to darwin only? 

because I only tested on OSX and didn't want to break any builds.

> I have tried them
> on Linux and they seem to work. 

Great!  I was hoping you/someone would give them a try! :)

> Although 'interrupt' test gave a timeout error. 
> I will try to figure out why that test fails on Linux.

Hmm, sounds like a Linux-specific bug - works fine on OSX (just retested to
confirm).

> 2. Why you are logging input/output is each test. Can we do without them?

This was based on similar tests (that is, tests which invoke lldb manually).  
Run the following to see:
    grep "Turn on logging for input" `find . -name "*.py"`

> 3. It will be useful to also test command which have some initial digits like
>    25-insert-break as IDEs sends MI command in that pattern.

yes, tests for that as well as other commands should be added.  This was
intended to be a starting place from which to expand on.
 
> 4. I think it will be useful to test '-gdb-exit' instead of 'quit'. Although 
>    there is a little bug in lldb-mi for this case. I will send a fix for it.

The reason why "quit" is required is because "-gdb-exit" doesn't actually exit
lldb-mi on OSX, so both "-gdb-exit" *and* "quit" are needed.

Enabling the ioctl code (patch submitted) fixes this issue on OSX, but
the polling is not ideal, so we(*) also have a patch that uses "select" with
some additional code required to fix "quit" but there was a limit to how
many unreviewed patches I wanted to flood lldb-dev with :)

(*) we == Embarcadero; we have an IDE that uses gdb's MI commands.

> 5. I think we have the copyright notice in the C files although I am not sure 
>    of the policies.

This was taken from another test where they added the copyright only to
main.c, so if it's wrong here, it's wrong there too :)
(see ./functionalities/breakpoint/breakpoint_command/a.c)

> If there are no more comments from community then I think the tests are good 
> enough to be committed. The things that I mentioned above can be fixed with time.
> If you don't have commit access then please let me know. I will commit them for
> you.

I used to be able to commit but committing hasn't worked for me in years
and it's not been high on my priority list to figure out why.  So please
commit?  If it gets too annoying let me know and I'll try to find someone who
can help me (don't even know who to talk to about that).

So please commit, and enable the Linux tests while you're at it? 
Don't delete the empty lines after "-exec-run" or the "quit" commands tho;
we need those on OSX until a ioctl/select/poll patch is accepted.

Thanks,
-Dawn