On Thu, Jan 4, 2024 at 5:23 PM Tamino Bauknecht <dev@xxxxxx> wrote:
> This commit introduces the new boolean configuration option fetch.all
> which allows to fetch all available remotes by default. The config
> option can be overridden by explicitly specifying a remote.
> The behavior for --all is unchanged and calling git-fetch with --all and
> a remote will still result in an error.
>
> The option was also added to the config documentation and new tests
> cover the expected behavior.
>
> Signed-off-by: Tamino Bauknecht <dev@xxxxxx>
> ---
> I fixed the formatting in the test cases and replaced the "cp" with an
> explicit "expect".
Thanks, looks better. More below...
> diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
> @@ -24,6 +24,15 @@ setup_repository () {
> +setup_test_clone () {
> + test_dir="$1"
> + git clone one "$test_dir"
> + for r in one two three
> + do
> + git -C "$test_dir" remote add "$r" "../$r" || return 1
> + done
> +}
I wasn't paying attention to this function in the previous round, but
it ought to be made more robust. If the `clone` operation fails, we
want to know about it right away rather than finding out about it "by
accident" when the subsequent `remote add` operation fails. In other
words, you should maintain an unbroken &&-chain, not just in test
bodies, but also in functions which are called from within test
bodies. So, this should really be:
setup_test_clone () {
test_dir="$1" &&
git clone one "$test_dir" &&
for r in one two three
do
git -C "$test_dir" remote add "$r" "../$r" || return 1
done
}
> @@ -209,4 +218,90 @@ test_expect_success 'git fetch --multiple --jobs=0 picks a default' '
> +for fetch_all in true false
> +do
> + test_expect_success "git fetch --all (works with fetch.all = $fetch_all)" '
> + test_dir="test_fetch_all_$fetch_all" &&
> + setup_test_clone "$test_dir" &&
> + (
> + cd "$test_dir" &&
> + git config fetch.all $fetch_all &&
> + git fetch --all &&
> + cat >expect <<-\EOF &&
> + ...
> + EOF
> + git branch -r >actual &&
> + test_cmp expect actual
> + )
> + '
> +done
> +
> +test_expect_success 'git fetch (fetch all remotes with fetch.all = true)' '
> + setup_test_clone test9 &&
> + (
> + cd test9 &&
> + git config fetch.all true &&
> + git fetch --all &&
> + git branch -r >actual &&
> + cat >expect <<-\EOF &&
> + ...
> + EOF
> + test_cmp expect actual
> + )
> +'
I'm probably overlooking something, but isn't this testing the exact
same thing as was tested in the "true" case of the loop just above?
Or maybe there is a bug in this test and you meant `git fetch` rather
than `git fetch --all`?
