On Fri, Feb 08, 2013 at 11:04:09AM -0800, Anthony Xu wrote: > But it is unfair developer should write a complete unit test for class if the > unit test doesn't exist and he modifies the class, because he may not > understand class well, he may not be able to write a unit test for the class.
That's not what I suggested. What I suggested was that the minimum is to create the scaffolding required to test the new code paths. If someone doesn't know enough about how a class works to test their own changes, should they be making that change? This is obviously not going to always apply, but I think that we can use our subjective judgement to know if modifications are significant or not. If they are significant, they would have required enough of an understanding of the class to help setup the structure for testing it, OR the developer should take the opportunity to learn it. Now if you *also* have the time and knowledge to add tests for the pre-existing code, then that's a bonus that helps all of us out. > I think we should file a bug for each class if unit test for this class > doesn't exist to add unit test for the class, and assign them to proper > people or volunteers. I disagree, because of the statement above. Significant changes to a class should be made with enough of an understanding of how the class works to create tests that cover the new / changed code path. > I checked other feature merges, unit test for new class were added, but few > of them have unit test for modified class, in the case , it is unfair to this > feature. Sorry, but I'm not trying to single you out. It's time for us to do this, period. You just happened to be one with a merge that got noticed. Please don't take it personally. We let a bunch of things go by for 4.1, even though we had all agreed that unit tests were a requirement for new features before we started working on 4.1. The IP clearance issues became the major problem area, so I didn't focus as much on checking for unit tests. Now that we're moving on to new features for post-4.1, I think we actually need to apply the practices that we all agreed on. Frankly, we should be self policing on this topic. Gerrit will help, but folks need to actually embrace and execute on the goal of adding tests with changes. -chip
