David Aguilar wrote:

> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -10,29 +10,11 @@ Testing basic diff tool invocation
[...]
> -restore_test_defaults()
> -{
> -     # Restores the test defaults used by several tests
> -     remove_config_vars
> -     unset GIT_DIFF_TOOL
> -     unset GIT_DIFFTOOL_PROMPT
> -     unset GIT_DIFFTOOL_NO_PROMPT
> -     git config diff.tool test-tool &&
> -     git config difftool.test-tool.cmd 'cat $LOCAL'
> -     git config difftool.bogus-tool.cmd false

Yay. :)

[...]
>  # Ensures that git-difftool ignores bogus --tool values
>  test_expect_success PERL 'difftool ignores bad --tool values' '
>       diff=$(git difftool --no-prompt --tool=bad-tool branch)
>       test "$?" = 1 &&
> -     test "$diff" = ""
> +     test -z "$diff"
>  '

Not about this patch: if I add more commands before that "diff",
their exit status would be ignored.  Could this be made more resilient
using test_expect_code?  Something like

        test_expect_code 1 git diff --no-prompt --tool=bad-tool branch >actual 
&&
        >expect &&
        test_cmp expect actual

[...]
>  # Specify the diff tool using $GIT_DIFF_TOOL
>  test_expect_success PERL 'GIT_DIFF_TOOL variable' '
> -     test_might_fail git config --unset diff.tool &&
> +     difftool_test_setup &&
> +     git config --unset diff.tool &&
> +
>       GIT_DIFF_TOOL=test-tool &&
>       export GIT_DIFF_TOOL &&
>  
>       diff=$(git difftool --no-prompt branch) &&
>       test "$diff" = "branch" &&
> -
> -     restore_test_defaults
> +     sane_unset GIT_DIFF_TOOL

If this test fails, GIT_DIFF_TOOL would remain set which could take
down later tests, too.  Could it be set in a subprocess (e.g., a
subshell) to avoid that?

        difftool_test_setup &&
        git config --unset diff.tool &&

        echo branch >expect &&
        GIT_DIFF_TOOL=test-tool git difftool --no-prompt branch >actual &&
        test_cmp expect actual

[...]
>  test_expect_success PERL 'GIT_DIFF_TOOL overrides' '
> -     git config diff.tool bogus-tool &&
> -     git config merge.tool bogus-tool &&
> -
> +     difftool_test_setup &&
> +     test_config diff.tool bogus-tool &&
> +     test_config merge.tool bogus-tool &&
>       GIT_DIFF_TOOL=test-tool &&
>       export GIT_DIFF_TOOL &&
>  
>       diff=$(git difftool --no-prompt branch) &&

Likewise.

[...]
>       GIT_DIFF_TOOL=bogus-tool &&
>       export GIT_DIFF_TOOL &&
>  
>       diff=$(git difftool --no-prompt --tool=test-tool branch) &&

Likewise.

[...]
>  test_expect_success PERL 'GIT_DIFFTOOL_NO_PROMPT variable' '
> +     difftool_test_setup &&
>       GIT_DIFFTOOL_NO_PROMPT=true &&
>       export GIT_DIFFTOOL_NO_PROMPT &&
>  
>       diff=$(git difftool branch) &&

Likewise.

[...]
>  test_expect_success PERL 'GIT_DIFFTOOL_PROMPT variable' '
> -     git config difftool.prompt false &&
> +     difftool_test_setup &&
> +     test_config difftool.prompt false &&
>       GIT_DIFFTOOL_PROMPT=true &&
>       export GIT_DIFFTOOL_PROMPT &&
>  
>       prompt=$(echo | git difftool branch | tail -1) &&

Likewise.  This one loses the exit status from 'git difftool',
which could be avoided by writing to temporary files:

        echo >input &&
        GIT_DIFFTOOL_PROMPT=true git difftool branch <input >output &&
        prompt=$(tail -1 <output) &&

[...]
>  test_expect_success PERL 'difftool last flag wins' '
> +     difftool_test_setup &&
>       diff=$(git difftool --prompt --no-prompt branch) &&
>       test "$diff" = "branch" &&
>  
> -     restore_test_defaults &&
> -
>       prompt=$(echo | git difftool --no-prompt --prompt branch | tail -1) &&
[...]

Likewise.

Thanks for cleaning up, and sorry I don't have anything more
substantial to offer.

Hope that helps,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to