"Victoria Dye via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> Finally, add tests to 't3007-ls-files-recurse-submodules.sh' to verify the
> corrected config is loaded. Use 'ls-files' to test this because, unlike some
> other '--recurse-submodules' commands, 'ls-files' parses the config of the
> submodule in the same process as the superproject (via 'show_submodule()' ->
> 'repo_read_index()' -> 'prepare_repo_settings()'). As a result,
> 'the_repository' points to the config of the superproject but the
> commondir/gitdir in the config sequence will be that of the submodule,
> providing the exact scenario needed to verify this patch.
>
> The first test ('--recurse-submodules parses submodule repo config') checks
> that the submodule's *repo* config is read when running 'ls-files' on the
> superproject; this confirms already-working behavior, serving as a reference
> for how worktree config parsing should behave. The second test
> ('--recurse-submodules parses submodule worktree config') tests the same
> scenario as the previous but instead using the *worktree* config,
> demonstrating the corrected behavior. The 'test_config' helper is extended
> for this case so that it properly applies the '--worktree' option to the
> configure/unconfigure operations it performs.
Thanks! This version is really clear.
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 6e19ebc922a..b3864e22e9a 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -542,8 +542,17 @@ test_config () {
> config_dir=$1
> shift
> fi
> - test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} '$1'" &&
> - git ${config_dir:+-C "$config_dir"} config "$@"
> +
> + # If --worktree is provided, use it to configure/unconfigure
> + is_worktree=
> + if test "$1" = --worktree
> + then
> + is_worktree=1
> + shift
> + fi
> +
> + test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} ${is_worktree:+--worktree} '$1'" &&
> + git ${config_dir:+-C "$config_dir"} config ${is_worktree:+--worktree} "$@"
> }
>
> test_config_global () {
The test_config() change looks good to me. In the future, I think we
should extend this so that we can get rid of test_config_global() (which
is basically the same thing) and have only have one way of doing this.
