Hi, I thought about adding a test*.sh file after sending the series. No worries, I will rectify it in the next patch. Also, I have read all your comments. Thanks for the review. Cheers, Tanay Abhra. On 6/25/2014 4:49 PM, Eric Sunshine wrote: > On Mon, Jun 23, 2014 at 6:11 AM, Tanay Abhra <tanayabh@xxxxxxxxx> wrote: >> Add different usage examples for 'git_config_get_string' and >> `git_config_get_string_multi`. They will serve as documentation >> on how to query for config values in a non-callback manner. > > This is a good start, but it's not fully what Matthieu was suggesting > when he said that you should prove to other developers, by way of > reproducible tests, that your changes work. What he meant, was that > you should write a test-config program which exposes (as a runnable > command) the new config C API you've added, and then write tests which > exercise that API and implementation exhaustively. > > For example, take a look at test-string-list.c and > t/t0063-string-list.sh. The C program does no checking itself. It > merely exposes the C API via command-line arguments, such as "split", > "filter", etc. The test script then employs that program to perform > the actual testing in a reproducible and (hopefully) exhaustive > fashion. Because t/t0063-string-list.sh is part of the test suite, the > string-list tests are run regularly by many developers. It's not just > something that someone might remember to run once in a while. > > Contrary to your commit message and the comment in the program itself, > the purpose of test-config is not to serve as documentation or to > provide examples of usage. (Real documentation is better suited for > those purposes.) Instead, test-config should exist in support of a > real test script in t/ which is run regularly. The new script you add > to t/ should exercise the exposed C API as exhaustively as possible. > This means checking each possible state: for instance, (1) when a key > is absent, (2) when a value is boolean (NULL), (3) one non-boolean > (non-NULL) value, (4) multiple values, etc. Moreover, it should check > expected success _and_ expected failure modes. Check not only that it > returns expected values, but that it fails when appropriate. > > More below. > -- 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