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. >> >> 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. >> >> 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*. 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. 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. >> >> Zach >> >> _______________________________________________ >> lldb-dev mailing list >> lldb-dev@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >> >> > > > -- > -Todd >
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev