> On Oct 7, 2015, at 10:05 AM, Zachary Turner via lldb-dev 
> <lldb-dev@lists.llvm.org> wrote:
> 
> Jim, Greg,
> 
> Can I get some feedback on this?  I would like to start enforcing this moving 
> forward.  I want to make sure we're in agreement.
> 
> On Mon, Oct 5, 2015 at 12:30 PM Todd Fiala <todd.fi...@gmail.com> wrote:
> IMHO that all sounds reasonable.
> 
> FWIW - I wrote some tests for the test system changes I put in (for the 
> pure-python impl of timeout support), and in the process, I discovered a race 
> condition in using a python facility that there really is no way I would have 
> found anywhere near as reasonably without having added the tests.  (For those 
> of you who are test-centric, this is not a surprising outcome, but I'm adding 
> this for those who may be inclined to think of it as an afterthought).
> 
> -Todd
> 
> On Mon, Oct 5, 2015 at 11:24 AM, Zachary Turner via lldb-dev 
> <lldb-dev@lists.llvm.org> wrote:
> On Fri, Sep 11, 2015 at 11:42 AM Jim Ingham <jing...@apple.com> wrote:
> I have held from the beginning that the only tests that should be written 
> using HandleCommand are those that explicitly test command behavior, and if 
> it is possible to write a test using the SB API you should always do it that 
> way for the very reasons you cite.  Not everybody agreed with me at first, so 
> we ended up with a bunch of tests that do complex things using HandleCommand 
> where they really ought not to.  I'm not sure it is worth the time to go 
> rewrite all those tests, but we shouldn't write any new tests that way.
> 
> I would like to revive this thread, because there doesn't seem to be 
> consensus that this is the way to go.  I've suggested on a couple of reviews 
> recently that people put new command api tests under a new top-level folder 
> under tests, and so far the responses I've gotten have not indicated that 
> people are willing to do this.
> 
> Nobody chimed in on this thread with a disagreement, which indicates to me 
> that we are ok with moving this forward.  So I'm reviving this in hopes that 
> we can come to agreement.  With that in mind, my goal is:
> 
> 1) Begin enforcing this on new CLs that go in.  We need to maintain a 
> consistent message and direction for the project, and if this is a "good 
> idea", then it should be applied and enforced consistently.  Command api 
> tests should be the exception, not the norm.

You mean API tests should be the norm right? I don't want people submitting 
command line tests like "file a.out", "run", "step". I want the API to be used. 
Did you get this reversed?
> 
> 2) Begin rejecting or reverting changes that go in without tests.  I 
> understand there are some situations where tests are difficult.  Core dumps 
> and unwinding come to mind.  There are probably others.  But this is the 
> exception, and not the norm.  Almost every change should go in with tests.

As long as it can be tested reasonably I am fine with rejecting changes going 
in that don't have tests.
> 
> 3) If a CL cannot be tested without a command api test due to limitations of 
> the SB API, require new changes to go in with a corresponding SB API change.

One issue here is I don't want stuff added to the SB API just so that it can be 
tested. The SB API must remain clean and consistent and remain an API that 
makes sense for debugging. I don't want internal goo being exposed just so we 
can test things. If we run into this a lot, we might need to make an alternate 
binary that can test internal unit tests. We could make a 
lldb_internal.so/lldb_internal.dylib/lldb_internal.dll that can be linked to by 
internal unit tests and then those unit tests can be run as part of the testing 
process. So lets keep the SB API clean and sensible with no extra fluff, and 
find a way to tests internal stuff in a different way.

>  I know that people just want to get their stuff done, but I dont' feel is an 
> excuse for having a subpar testing situation.  For the record, I'm not 
> singling anyone out.  Everyone is guilty, including me.  I'm offering to do 
> my part, and I would like to be able to enforce this at the project level.  
> As with #2, there are times when an SB API isn't appropriate or doesn't make 
> sense.  We can figure that out when we come to it.

We should do built in unit tests like some things already do if they can't or 
shouldn't be in the SB API as stated above.

>  But I believe a large majority of these command api tests go in the way they 
> do because there is no corresponding SB API yet.  And I think the change 
> should not go in without designing the appropriate SB API at the same time.

Only if it makes sense for the SB API, yes.

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

Reply via email to