Re: [PATCH v2 01/28] submodule tests: test usage behavior

From: Glen Choo <chooglen@google.com>
Date: Aug 2, 2022, 22:30 UTC
Message-ID: <kl6ltu6uth1t.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <patch-v2-01.28-daa5d3f9962-20220802T154036Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 9 quoted lines

> Test what exit code and output we emit on "git submodule -h", how we
> handle "--" when no subcommand is specified, and how the top-level
> "--recursive" option is handled.
>
> For "-h" this doesn't make sense, but let's test for it so that any
> subsequent eventual behavior change will become clear.
>
> For "--" this follows up on 68cabbfda36 (submodule: document default
> behavior, 2019-02-15) and tests that "status" doesn't don't support

s/doesn't don't/doesn't/ I assume ;)

Show 10 quoted lines

> the "--" delimiter. There's no intrinsically good reason not to
> support that. We behave this way due to edge cases in
> git-submodule.sh's implementation, but as with "-h" let's assert our
> current long-standing behavior for now.
>
> For "--recursive" the exclusion of it from the top-level appears to
> have been an omission in 15fc56a8536 (git submodule foreach: Add
> --recursive to recurse into nested submodules, 2009-08-19), there
> doesn't seem to be a reason not to support it alongside "--quiet" and
> "--cached", but let's likewise assert our existing behavior for now.

I think my original comment isn't directly addressed here:

  I'm not sure about this one. "--recursive" isn't a top level option, and
  15fc56a8536 doesn't seem to suggest that it was meant to be one (but I
  did realize that git-submodule.txt documents all of the options to all
  subcommands, which is very confusing.)

But maybe I'm mistaken and it's somewhere. Do we know whether "--recursive" was always intended to be top-level?

Show 43 quoted lines

>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  t/t7400-submodule-basic.sh | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index e7cec2e457a..b858871a953 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -14,6 +14,32 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  
>  . ./test-lib.sh
>  
> +test_expect_success 'submodule usage: -h' '
> +	git submodule -h >out 2>err &&
> +	grep "^usage: git submodule" out &&
> +	test_must_be_empty err
> +'
> +
> +test_expect_success 'submodule usage: --recursive' '
> +	test_expect_code 1 git submodule --recursive >out 2>err &&
> +	grep "^usage: git submodule" err &&
> +	test_must_be_empty out
> +'
> +
> +test_expect_success 'submodule usage: status --' '
> +	test_expect_code 1 git submodule -- &&
> +	test_expect_code 1 git submodule --end-of-options
> +'
> +
> +for opt in '--quiet' '--cached'
> +do
> +	test_expect_success "submodule usage: status $opt" '
> +		git submodule $opt &&
> +		git submodule status $opt &&
> +		git submodule $opt status
> +	'
> +done
> +
>  test_expect_success 'submodule deinit works on empty repository' '
>  	git submodule deinit --all
>  '

The changes themselves look good :)

> -- 
> 2.37.1.1233.ge8b09efaedc