What you are describing is what testing literature refers to as criteria for equivalence classes. There is some level of judgment to that, yes.
Yep yep, to be sure. I'm just generally trying to encourage the community behavior towards being both selective & thorough about testing. I have noticed you doing this (not just in this review) and I am very appreciative of the principles; when it comes to understanding what a test is trying to do, keeping the unnecessary fluff out is very helpful. You have no idea how many times I've had to suss out the intent of a (usually comment-free) test after it broke when we merged it into our tree. Fortunately that sort of thing has been happening less often, now that more of our changes have been integrated upstream, but still, it's great to have tests that are very focused…. ….when they are tests for a bugfix or other comparatively small change. I have to say when it comes to a new-feature kind of patch, I would rather have the test err on the side of completeness. This is partly based on the experience of introducing the 'optnone' attribute to Clang, which IIRC popped up with new and surprising cases two or three times after its introduction. More thorough tests up front could easily have prevented those surprises. Now here I am again, not with a new attribute but seriously expanding the applicability of an attribute, and would like to apply previous experience and start out with what I think should be a moderately complete test. If you're unwilling to accept that argument and insist on minimal upstream tests, okay; I can take what I've done and migrate it into our private tests, and leave behind only the minimal upstream test. It will leave me with the test I think the feature needs, leaves upstream with the minimal test you prefer, and if something breaks it will just take a little longer to get that feedback. Let me know. Thanks, --paulr
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
