On Sun, Dec 20, 2020 at 9:05 PM Nipunn Koorapati via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> test_config fails to unset the configuration variable when
> using --add, as it tries to run git config --unset-all --add
>
> Tell test_config to invoke test_unconfig with the arg $2 when
> the arg $1 is --add
>
> Signed-off-by: Nipunn Koorapati <nipunn@xxxxxxxxxxx>
> ---
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -381,6 +381,7 @@ test_unconfig () {
> config_dir=$1
> shift
> fi
> + echo git ${config_dir:+-C "$config_dir"} config --unset-all "$@"
Stray debugging gunk?
> @@ -400,7 +401,13 @@ test_config () {
> - test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} '$1'" &&
> +
> + first_arg=$1
> + if test "$1" = --add; then
> + first_arg=$2
> + fi
> +
> + test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} '$first_arg'" &&
Several comments...
Style on this project is to place `then` on its own line (as seen a
few lines above this change):
if test "$1" = --add
then
...
This logic would be easier to understand if the variable was named
`varname` or `cfgvar` (or something), which better conveys intention,
rather than `first_arg`.
It feels odd to single out `--add` when there are other similar
options, such as `--replace-all`, `--fixed-value`, or even `--type`
which people might try using in the future.
This new option parsing is somewhat brittle. If a caller uses
`test_config --add -C <dir> ...`, it won't work as expected. Perhaps
that's not likely to happen, but it would be easy enough to fix by
unifying and generalizing option parsing a bit. Doing so would also
make it easy for the other options mentioned above to be added later
if ever needed. For instance:
options=
while test $# != 0
do
case "$1" in
-C)
config_dir=$2
shift
;;
--add)
options="$options $1"
;;
*)
break
;;
esac
shift
done
Finally, as this is a one-off case, it might be simpler just to drop
this patch altogether and open-code the cleanup in the test itself in
patch [2/3] rather than bothering with test_config() in that
particular case. For example:
test_when_finished "test_unconfig -C two remote.one.push" &&
git config -C two --add remote.one.push : &&
test_must_fail git -C two push one &&
git config -C two --add remote.one.push ^refs/heads/master &&
git -C two push one
