Re: [PATCH] t3200: clarify description of --set-upstream test

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 14, 2018, 17:43 UTC
Message-ID: <xmqqin6l461s.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180614140641.13752-1-kaartic.sivaraam@gmail.com>

Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:

Show 16 quoted lines

> Support for the --set-upstream option was removed in 52668846ea
> (builtin/branch: stop supporting the "--set-upstream" option,
> 2017-08-17). The change did not completely remove the command
> due to an issue noted in the commit's log message.
>
> So, a test was added to ensure that a command which uses the
> '--set-upstream' option fails and doesn't silently act as an alias
> for the '--set-upstream-to' option due to option parsing features.
>
> To avoid confusion, clarify that the option is an unsupported one
> in the corresponding test description.
>
> Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
> ---
>  t/t3200-branch.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

The above description is much clearer than what the test title after the patch gives its readers.

It is technically correct to call --set-upstream "unsupported", but the reason why we want to see it fail is not because it is unsupported, but because we actively interfere with the usual "unique prefix" logic parse-options API gives its users and make it not to trigger the longer-and-unique --set-upstream-to logic.

Show 10 quoted lines

> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 6c0b7ea4a..d14de82ba 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -884,7 +884,7 @@ test_expect_success 'test --unset-upstream on a particular branch' '
>  	test_must_fail git config branch.my14.merge
>  '
>  
> -test_expect_success '--set-upstream fails' '
> +test_expect_success 'unsupported option --set-upstream fails' '

In other words, I am wondering if s/unsupported/disabled/ makes it even more clear what is going on here.

>      test_must_fail git branch --set-upstream origin/master
>  '