jingham added a comment.

Note that ProcessGDBRemote (or maybe the client) keeps a history array of 
packets sent.  Greg added that long ago so that when you enabled the packet log 
you would get the packet history up to the time you enabled it, and not just 
the new packets.  That has often come  in quite handy.  And having a SB API 
like:

bool SBProcess::HasRemoteTrafficLog();
SBStringList SBProcess::DumpRemoteTraffic(<some kind of checkpoint>);

doesn't seem like a bad API.  I could imagine some really horribly geeky 
debugger UI that would want to have a packet traffic window.  Then using the 
checkpoint, you could get the strings from event to event and look for the 
packets you expected.  That seems a lot of work for testing this bit of code, 
however.

Short of that, Pavel's last suggestion seems right for testing that the  
contents of the Process's m_unix_signals_sp is correctly copied into the ignore 
signals packet you are going to send.  The higher level tests already ensure 
that we are correctly filling m_unix_signals_sp from commands or SB API's, but 
this last step is uncovered.  So if you move the API to the communication 
client, and pass in the signals, you can then test that last step in isolation. 
 And it seems perfectly reasonable for the Communication client to be doing the 
actual work of copying the signals over.

Another bit that isn't tested is that if you change the signals in the process, 
you will trigger resending the packet.  You could test more of this path if:

(a) you had the UnixSignals class be the one that returns the array of signals 
to ignore rather than it being freestanding logic in the ProcessGDBRemote 
class; the bit of logic that checks pass, stop & notify seems more appropriate 
in the signals class anyway.

(b) change the m_last_signals_version logic to a "HasChanged" and 
"SetHasChanged(bool changed)" API vended by UnixSignals. SetHasChanged would be 
called passing false in the Process::SendSignalsToIgnoreIfNeeded, that seems 
pretty clearly right.  And you probably have to call SetHasChanged(true) to 
force things for the launch case, but that should be straightforward.

Then you could test all most of the chain by making your own UnixSignals and 
passing that to the GDBRemoteCommunicationClient.  If you change it and pass it 
in again, you could ensure that the right packet is sent.  That leave out that 
Process handles the SetHasChanged properly but it gets pretty close.


https://reviews.llvm.org/D30520



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to