On 10/30/2013 11:37 PM, Robert Collins wrote: > This is a bit of a social norms thread.... > > I've been consistently asking for tests in reviews for a while now, > and I get the occasional push-back. I think this falls into a few > broad camps: > > A - there is no test suite at all, adding one in unreasonable > B - this thing cannot be tested in this context (e.g. functional tests > are defined in a different tree) > C - this particular thing is very hard to test > D - testing this won't offer benefit > E - other things like this in the project don't have tests > F - submitter doesn't know how to write tests > G - submitter doesn't have time to write tests > > Now, of these, I think it's fine not add tests in cases A, B, C in > combination with D, and D. > > I don't think E, F or G are sufficient reasons to merge something > without tests, when reviewers are asking for them. G in the special > case that the project really wants the patch landed - but then I'd > expect reviewers to not ask for tests or to volunteer that they might > be optional. > > Now, if I'm wrong, and folk have different norms about when to accept > 'reason X not to write tests' as a response from the submitter - > please let me know!
I've done a lot of thinking around this topic [1][2] and really it comes down to this: everything can be tested and should be. There is an argument to A, but that goes beyond the scope of our use case I think. If I hear B, I would suspect the tests aren't unit tests, but are functional/integration tests (a common problem in OpenStack). Functional tests are brittle and usually have painful setup sequences. The other cases fall into the -1 camp for me. Tests required. That said, recently I was -1'ed for not updating a test, because I added code that didn't change the program flow, but introduced a new call. According to my rules, that didn't need a test, but I agreed with the logic that people would be upset if the call wasn't made (it was a notification). So a test was added. Totally valid argument. TL;DR: Tests are always required. We need to fix our tests to be proper unit tests and not functional/integration tests so it's easy to add new ones. -S [1] http://www.sandywalsh.com/2011/06/effective-units-tests-and-integration.html [2] http://www.sandywalsh.com/2011/08/pain-of-unit-tests-and-dynamically.html > -Rob > _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev