Re: [PATCH v4 4/4] t7800: run both builtin and scripted difftool, for now

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> This is uglier than a simple
>
> 	touch "$GIT_EXEC_PATH/use-builtin-difftool"
>
> of course. But oh well.

That is totally irrelevant.  

The more important point is that we do the same set of tests so
instead of running just one, we run BOTH.  If we did a bit more
work, we could even say "even when there is no Perl installed, we
run tests with only the new built-in one".  This does not go that
far yet, but that should not be too hard, I would think.  See below.

> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index e94910c563..273ab55723 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -23,6 +23,20 @@ prompt_given ()
>  	test "$prompt" = "Launch 'test-tool' [Y/n]? branch"
>  }
>  
> +for use_builtin_difftool in false true
> +do

Instead of the above, I'd suggest to make the loop like so:

	for use_builtin_difftool in $TESTED_VARIANTS
	do

and before the loop begins, set TESTED_VARIANTS to either "false
true" or "true" by checking the PERL prerequisite.

> +test_expect_success 'verify we are running the correct difftool' '
> +	if test true = '$use_builtin_difftool'
> +	then
> +		test_must_fail ok=129 git difftool -h >help &&
> +		grep "g, --gui" help
> +	else
> +		git difftool -h >help &&
> +		grep "g|--gui" help
> +	fi
> +'
> +
>  # NEEDSWORK: lose all the PERL prereqs once legacy-difftool is retired.

And then we can lose this comment.  As all existing tests have PERL
prereq that can go away with the above suggested change, we'd need
to touch quite a many lines with "s/_success PERL /_success /".

It may be a good time to indent these existing tests to make it
clear that they are inside the new "for use_builtin_difftool" loop.


> +test true != $use_builtin_difftool || break
> +
> +test_expect_success 'tear down for re-run' '
> +	rm -rf * .[a-z]* &&
> +	git init
> +'
> +
> +# run as builtin difftool now
> +GIT_CONFIG_PARAMETERS="'difftool.usebuiltin=true'"
> +export GIT_CONFIG_PARAMETERS

... and indentation would extend to these newly added lines, of
course.

> +done
> +
>  test_done