Re: [PATCH v10 3/3] commit: add a commit.verbose config variable

> On Sun, Mar 27, 2016 at 3:00 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> On Sun, Mar 27, 2016 at 9:04 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> On Sat, Mar 26, 2016 at 3:48 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>>>> +test_expect_success 'commit.verbose true and --verbose omitted' '
>>>> +       echo content >file2 &&
>>>> +       echo content >>file &&
>>>> +       git add file2 &&
>>>> +       git -c commit.verbose=true commit -F message &&
>>>> +       test_line_count = 1 out
>>>> +'
>>>
>>> Why is this test so utterly different than it was in v9 (even though
>>> the title is the same), and why is it so different from other tests
>>> below?
>>
>> This is because the "editor" in v9 checked for "# Changes"... While
>> this "editor" checks for 'diff --git'. And submodules don't give a
>> proper diff to verify (I tried this out and noticed this behavior by
>> tweaking some parts). In fact submodules don't give diff at all. But
>> they do give "# Changes"... So its important to setup up a little
>> before getting started. If this seems unnecessary, then should I move
>> all the tests which were introduced here above the submodule test?
>
> Let's ignore submodules when discussing this since they don't need to
> factor into the issue. What you are actually saying (and what took me
> a while to understand due to the "submodules" misdirection) is that
> you need to do some additional setup to test the "-v -v" cases. In
> particular, you need to introduce some change to the worktree which is
> not in the index.

> The typical way to satisfy this requirement (which doesn't require
> relocating tests) is to add a "setup" test before the tests which
> depend upon that additional setup, rather than adding that setup to
> the first test which needs it. Just about the simplest setup test
> which satisfies your needs is the following (inserted just before the
> first of the new tests):
>
>     test_expect_success 'setup -v -v' '
>         echo dirty >file
>     '
>
> And, then you can restore the "commit.verbose true and --verbose
> omitted" test to its simple form:
>
>     test_expect_success 'commit.verbose true and --verbose omitted' '
>         git -c commit.verbose=true commit --amend &&
>         test_line_count = 1 out
>     '

> By the way, now that commit.verbose is no longer a mere boolean,
> you're going to need some additional tests beyond the
> commit.verbose={true,false} ones you've already added. In particular,
> you should be testing commit.verbose with several numeric values to
> verify that it works as expected. For instance:
>
>     commit.verbose=-2
>     commit.verbose=-1
>     commit.verbose=0
>     commit.verbose=1
>     commit.verbose=2
>     commit.verbose=3
>
> The -2 case is interesting; I'm pretty sure the current implementation
> of this patch will misbehave since the only negative value it's
> expecting 'config_verbose' to be is -1.