Re: [PATCH v15 3/7] t0040-parse-options: improve test coverage

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 4, 2016, 08:36 UTC
Message-ID: <CAPig+cTW9=w8fMTwcF-Z6DPmXMM4qiWZA=7S1GcFEoGG8hPUtw@mail.gmail.com>
In-Reply-To: <1462046616-2582-3-git-send-email-pranit.bauva@gmail.com>

On Sat, Apr 30, 2016 at 4:03 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:

> Include tests to check for multiple levels of quiet and to check if the
> '--no-quiet' option sets it to 0.

As this patch is also adding a test of --[no-]verbose, the commit message should mention it.

More below...

Show 15 quoted lines

> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> ---
> diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
> @@ -476,4 +476,61 @@ test_expect_success '--no-list resets list' '
> +test_expect_success 'multiple quiet levels' '
> +       test-parse-options -q -q -q >output 2>output.err &&
> +       test_must_be_empty output.err &&
> +       test_cmp expect output
> +'
> +
> +test_expect_success '--no-quiet sets quiet to 0' '
> +       test-parse-options -q -q -q --no-quiet >output 2>output.err &&
> +       test_must_be_empty output.err &&
> +       test_cmp expect output
> +'

It wouldn't hurt to have two tests for --no-quiet: one which tests --no-quiet alone to ensure that 'quiet' *remains* at 0, and one which tests --no-quiet in combination with some --quiet's to ensure that 'quiet' is *reset* to 0. These tests would give you good coverage for changes by subsequent patches, such as the OPTION_COUNTUP patch which flips the initial value to -1.

Show 6 quoted lines

> +
> +test_expect_success '--no-verbose sets verbose to 0' '
> +       test-parse-options --no-verbose >output 2> output.err &&
> +       test_must_be_empty output.err &&
> +       test_cmp expect output
> +'

One would expect to see 'verbose' get the same treatment of having a test invoke --verbose multiple times. (Yes, I realize that the "long options" test does just this, but testing multiple --verbose's is not its primary purpose, so having a test which does test multiple --verbose's as its primary purpose can be beneficial and is less likely to be broken by someone in the future.)