Re: [Outreachy PATCH] t1402: test --no-allow-onelevel option precedence

[Junio C Hamano <gitster@pobox.com>]

Fien Dora <fiendora5@gmail.com> writes:

> Git allows later command-line options to override earlier ones.
> Added tests to verify that --no-allow-onelevel overrides
> --allow-onelevel and vice versa.

There are many other commands that take --[no-]foo options and a
later --no-foo overrides an earlier --foo (and vice versa), and most
of them do use the parse_options() API.  It probably is not worth
our brain cycles to add tests to cover all these other commands to
cover all these options that can be negated in a way similar to this
patch does for the ones based on parse_options().

What makes check-ref-format somewhat special is that it does not use
the parse_options() API to handle command line options, and instead
uses a handcrafted loop to parse them.  Covering the behaviour of
the command with tests may therefore make a bit more sense than all
the others.

> This ensures that the expected behavior is preserved if the
> argument-parsing code changes in the future.

So I would say that we need two changes to the log message:

 * The rationale should highlight why check-ref-format is special
   and deserves to have this pretty common "no- prefix negates"
   behaviour tested (i.e., it is a handcrafted parse loop); and

 * We have immediate plan to rewrite the option parsing logic (i.e.,
   not a vague "if we change it in the fiture").

to sell this change better.

Thanks.

>
> Signed-off-by: Fien Dora <fiendora5@gmail.com>
> ---
>  t/t1402-check-ref-format.sh | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
> index 9dd64662b2..6036df0210 100755
> --- a/t/t1402-check-ref-format.sh
> +++ b/t/t1402-check-ref-format.sh
> @@ -74,6 +74,8 @@ invalid_ref 'heads/foo*/bar*' --refspec-pattern
>  ref='foo'
>  invalid_ref "$ref"
>  valid_ref "$ref" --allow-onelevel
> +invalid_ref "$ref" '--allow-onelevel --no-allow-onelevel'
> +valid_ref "$ref" '--no-allow-onelevel --allow-onelevel'
>  invalid_ref "$ref" --refspec-pattern
>  valid_ref "$ref" '--refspec-pattern --allow-onelevel'
>  invalid_ref "$ref" --normalize