On Sun, Mar 27, 2016 at 1:47 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Sun, Mar 27, 2016 at 3:00 AM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote:
>> On Sun, Mar 27, 2016 at 9:04 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>>> On Sat, Mar 26, 2016 at 3:48 PM, Pranit Bauva <pranit.bauva@xxxxxxxxx> 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.
Sorry for the misdirection. And you understood correctly. I do need to
introduce some changes in worktree.
> 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
> '
Having a additional setup test seems a nice way to go about.
> 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.
-2 case will fail. I should probably expect 'config_verbose' to be
negative instead.
--
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