Re: [PATCH v3 03/10] t5526: create superproject commits with test helper
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Feb 24, 2022, 23:14 UTC
- Message-ID
- <20220224231420.2878760-1-jonathantanmy@google.com>
- In-Reply-To
- <20220224100842.95827-4-chooglen@google.com>
Glen Choo <chooglen@google.com> writes:
Show 16 quoted lines
> +# For each superproject in the test setup, update its submodule, add the
> +# submodule and create a new commit with the submodule change.
> +#
> +# This requires add_submodule_commits() to be called first, otherwise
> +# the submodules will not have changed and cannot be "git add"-ed.
> +add_superproject_commits() {
> +(
> + cd submodule &&
> + (
> + cd subdir/deepsubmodule &&
> + git fetch &&
> + git checkout -q FETCH_HEAD
> + ) &&
> + git add subdir/deepsubmodule &&
> + git commit -m "new deep submodule"
> + ) &&The indentation looks off. Also, no need for "-q".
Show 21 quoted lines
> @@ -378,19 +387,7 @@ test_expect_success "Recursion picks up all submodules when necessary" ' > ' > > test_expect_success "'--recurse-submodules=on-demand' doesn't recurse when no new commits are fetched in the superproject (and ignores config)" ' > - add_upstream_commit && > - ( > - cd submodule && > - ( > - cd subdir/deepsubmodule && > - git fetch && > - git checkout -q FETCH_HEAD > - ) && > - git add subdir/deepsubmodule && > - git commit -m "new deepsubmodule" && > - new_head=$(git rev-parse --short HEAD) && > - check_sub $new_head > - ) && > + add_submodule_commits && > ( > cd downstream && > git config fetch.recurseSubmodules true &&
Hmm...I'm surprised that this still passes even when code is deleted but the replacement is not added. What's happening here, I guess, is that we're checking that nothing has happened. The test probably should be rewritten but that's outside the scope of this patch set. So for now, just add the add_superproject_commits call.
Show 12 quoted lines
> @@ -402,10 +399,7 @@ test_expect_success "'--recurse-submodules=on-demand' doesn't recurse when no ne > ' > > test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necessary (and ignores config)" ' > - git add submodule && > - git commit -m "new submodule" && > - new_head=$(git rev-parse --short HEAD) && > - check_super $new_head && > + add_superproject_commits && > ( > cd downstream && > git config fetch.recurseSubmodules false &&
add_superproject_commits without add_submodule_commits?
The rest looks good and overall this looks like a good idea to simplify the test.