Subject: Re: [PATCH v3 3/3] test-config: add usage examples for non-callback query functions

From: Tanay Abhra <[email-masked]>
Date: Thu, 26 Jun 2014 14:10:08 +0530

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 <[email-masked]> 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.
>