Re: [PATCH v2 3/4] t7800: modernize tests
David Aguilar writes:
> Eliminate a lot of redundant work by using test_config().
> Catch more return codes by more use of temporary files
> and test_cmp.
>
> The original tests relied upon restore_test_defaults()
> from the previous test to provide the next test with a sane
> environment. Make the tests do their own setup so that they
> are not dependent on the success of the previous test.
> The end result is shorter tests and better test isolation.
>
> Signed-off-by: David Aguilar
> ---
> We no longer export variables into the environment per Jonathan's
> suggestion. This covers all of the review notes.
>
> t/t7800-difftool.sh | 360
>
> 1 file changed, 165 insertions(+), 195 deletions(-)
>
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index 5b5939b..b577c01 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -10,29 +10,11 @@ Testing basic diff tool invocation
>
> . ./test-lib.sh
>
> +difftool_test_setup()
> {
> + test_config diff.tool test-tool &&
> + test_config difftool.test-tool.cmd 'cat $LOCAL' &&
> + test_config difftool.bogus-tool.cmd false
> }
Cute.
Are we sure that $LOCAL is free of $IFS, or is it safer to say 'cat
"$LOCAL"' or something?
> @@ -324,26 +294,26 @@ test_expect_success PERL 'setup with 2 files different'
> '
> '
>
> test_expect_success PERL 'say no to the first file' '
> - diff=$( (echo n; echo) | git difftool -x cat branch ) &&
> -
> - echo "$diff" | stdin_contains m2 &&
> - echo "$diff" | stdin_contains br2 &&
> - echo "$diff" | stdin_doesnot_contain master &&
> - echo "$diff" | stdin_doesnot_contain branch
> + (echo n && echo) >input &&
> + git difftool -x cat branch output &&
> + cat output | stdin_contains m2 &&
> + cat output | stdin_contains br2 &&
> + cat output | stdin_doesnot_contain master &&
> + cat output | stdin_doesnot_contain branch
Do you need these pipes? In other words, wouldn't
stdin_contains whatever http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] t7800: modernize tests
Eliminate a lot of redundant work by using test_config().
Catch more return codes by more use of temporary files
and test_cmp.
The original tests relied upon restore_test_defaults()
from the previous test to provide the next test with a sane
environment. Make the tests do their own setup so that they
are not dependent on the success of the previous test.
The end result is shorter tests and better test isolation.
Signed-off-by: David Aguilar
---
We no longer export variables into the environment per Jonathan's
suggestion. This covers all of the review notes.
t/t7800-difftool.sh | 360
1 file changed, 165 insertions(+), 195 deletions(-)
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 5b5939b..b577c01 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -10,29 +10,11 @@ Testing basic diff tool invocation
. ./test-lib.sh
-remove_config_vars()
+difftool_test_setup()
{
- # Unset all config variables used by git-difftool
- git config --unset diff.tool
- git config --unset diff.guitool
- git config --unset difftool.test-tool.cmd
- git config --unset difftool.prompt
- git config --unset merge.tool
- git config --unset mergetool.test-tool.cmd
- git config --unset mergetool.prompt
- return 0
-}
-
-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
+ test_config diff.tool test-tool &&
+ test_config difftool.test-tool.cmd 'cat $LOCAL' &&
+ test_config difftool.bogus-tool.cmd false
}
prompt_given()
@@ -65,249 +47,237 @@ test_expect_success PERL 'setup' '
# Configure a custom difftool..cmd and use it
test_expect_success PERL 'custom commands' '
- restore_test_defaults &&
- git config difftool.test-tool.cmd "cat \$REMOTE" &&
+ difftool_test_setup &&
+ test_config difftool.test-tool.cmd "cat \$REMOTE" &&
+ echo master >expect &&
+ git difftool --no-prompt branch >actual &&
+ test_cmp expect actual &&
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "master" &&
-
- restore_test_defaults &&
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "branch"
+ test_config difftool.test-tool.cmd "cat \$LOCAL" &&
+ echo branch >expect &&
+ git difftool --no-prompt branch >actual &&
+ test_cmp expect actual
'
-# Ensures that a custom difftool..cmd overrides built-ins
-test_expect_success PERL 'custom commands override built-ins' '
- restore_test_defaults &&
- git config difftool.defaults.cmd "cat \$REMOTE" &&
-
- diff=$(git difftool --tool defaults --no-prompt branch) &&
- test "$diff" = "master" &&
-
- git config --unset difftool.defaults.cmd
+test_expect_success PERL 'custom tool commands override built-ins' '
+ test_config difftool.defaults.cmd "cat \$REMOTE" &&
+ echo master >expect &&
+ git difftool --tool defaults --no-prompt branch >actual &&
+ test_cmp expect actual
'
-# 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" = ""
+ : >expect &&
+ test_expect_code 1 \
+ git difftool --no-prompt --tool=bad-tool branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool forwards arguments to diff' '
+ difftool_test_setup &&
>for-diff &&
git add for-diff &&
echo changes>for-diff &&
git add for-diff &&
- diff=$(git difftool --cached --no-prompt -- for-diff) &&
- test "$diff" = "" &&
+ : >expect &&
+ git difftool --cached --no-prompt -- for-diff >actual &&
+ test_cmp expect actual &&
git reset -- for-diff &&
rm for-diff
'
test_expect_success PERL 'difftool honors --gui' '
- git config merge.tool bogus-tool &&
- git config diff.tool bogus-tool &&
- git config diff.guitool test-tool &&
-
- diff=$(git difftool --no-prompt --gui branch) &&
- test "$diff" = "branch" &&
+ difftool_test_setup &&
+ test_config merge.tool bogus-tool &&
+ test_config diff.tool bogus-tool &&
+ test_config diff.guitool test-tool &&
- restore_test_defaults
+ echo branch >expect &&
+ git difftool --no-prompt --gui branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --gui last setting wins' '
- git config diff.guitool bogus-tool &&
- git difftool --no-prompt --gui --no-gui &&
+ di
