Re: [PATCH v14 3/6] t0040-parse-options: improve test coverage

> On Wed, Apr 13, 2016 at 4:59 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> On Wed, Apr 13, 2016 at 10:56 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> On Tue, Apr 12, 2016 at 7:02 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>>>> +test_expect_success '--no-quiet sets quiet to 0' '
>>>> +       test-parse-options --no-quiet >output 2>output.err &&
>>>
>>> Meh, as implemented, this isn't a very interesting test, is it?
>>> 'quiet' started at 0, so all this shows is that --no-quiet didn't
>>> disturb the 0. To really test that it resets it to 0, you'd want:
>>>
>>>     test-parse-options --quiet --no-quiet >... 2>... &&
>>>
>>>> +       test_must_be_empty output.err &&
>>>> +       test_cmp expect output
>>>> +'
>>>>  test_done
>>
>> This is to test whether the 0 of quiet remains 0 if --no-quiet is
>> included. This test "defines" the current behavior. Then when I change
>> OPT_COUNTUP(), this test will ensure that this behavior is not
>> interrupted as promised by the commit message of that patch[1]. I
>> guess this also describe why I choose to include these tests between
>> 2/5 and 3/5 rather than 3/5 and 4/5. And also see the extended
>> discussion[2] for this. If I do a re-roll then I include `--quiet`
>> before `--no-quiet`
>
> Each of these patches should have a single conceptual purpose. It
> seems, from the above explanation, that you're mixing and mismatching
> bits of such changes between patches.
>
> * The two new tests for --no-verbose and --no-quiet should be together
> and check that they correctly reverse --verbose and --quiet,
> respectively.
>
> * The test you describe above which ensures that --no-quiet leaves
> 'quiet' at 0 should be bundled with the change that might break that
> behavior, namely, the OPT__COUNTUP() change.