On Tue, Apr 5, 2016 at 4:59 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Sun, Apr 3, 2016 at 8:58 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: >> The fact that the 32 new tests are nearly identical suggests strongly >> that the testing should instead either be table-driven or be done via >> for-loops to systematically cover all cases. Not only would either of >> these approaches be easier to digest, but they would make it easy to >> tell at a glance if any cases were missing. See [2] for an example of >> how the tests can be table-driven, and see the bottom of [3] for an >> example of using for-loops to test systematically (though you'd need >> to use nested for-loops rather than just the one in the example). >> >> I'm leaning toward systematic testing via nested for-loops as the more >> suitable of the two approach for this particular application. > I hadn't thought of this before. I also believe using for-loops will make it more clear, crisp and will avoid the effort of going through the whole patch to find out if some test is missing. > By the way, while this would be a nice change, it doesn't necessarily > have to be part of this series, and could be done as a follow-up by > you or someone else. > > (The other changes suggested in the same review, however, ought to be > fixed in this series; in particular, simplifying the "setup" test and > making the first test after "setup" consistent with the remaining > tests.) I will include it this series only as it will be a bit easier for me to keep a track of the previous reviews. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html