* [PATCH v2 1/4] difftool: silence uninitialized variable warning
@ 2013-02-20 5:35 David Aguilar
2013-02-20 5:35 ` [PATCH v2 2/4] t7800: update copyright notice David Aguilar
0 siblings, 1 reply; 5+ messages in thread
From: David Aguilar @ 2013-02-20 5:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
Git::config() returns `undef` when given keys that do not exist.
Check that the $guitool value is defined to prevent a noisy
"Use of uninitialized variable $guitool in length" warning.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Unchanged from last time but included in the series for convenience.
git-difftool.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 0a90de4..12231fb 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -336,7 +336,7 @@ sub main
}
if ($opts{gui}) {
my $guitool = Git::config('diff.guitool');
- if (length($guitool) > 0) {
+ if (defined($guitool) && length($guitool) > 0) {
$ENV{GIT_DIFF_TOOL} = $guitool;
}
}
--
1.8.2.rc0.20.gf548dd7
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 2/4] t7800: update copyright notice
2013-02-20 5:35 [PATCH v2 1/4] difftool: silence uninitialized variable warning David Aguilar
@ 2013-02-20 5:35 ` David Aguilar
2013-02-20 5:35 ` [PATCH v2 3/4] t7800: modernize tests David Aguilar
0 siblings, 1 reply; 5+ messages in thread
From: David Aguilar @ 2013-02-20 5:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
Signed-off-by: David Aguilar <davvid@gmail.com>
---
t/t7800-difftool.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index eb1d3f8..5b5939b 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -1,6 +1,6 @@
#!/bin/sh
#
-# Copyright (c) 2009, 2010 David Aguilar
+# Copyright (c) 2009, 2010, 2012, 2013 David Aguilar
#
test_description='git-difftool
--
1.8.2.rc0.20.gf548dd7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 3/4] t7800: modernize tests
2013-02-20 5:35 ` [PATCH v2 2/4] t7800: update copyright notice David Aguilar
@ 2013-02-20 5:35 ` David Aguilar
2013-02-20 5:35 ` [PATCH v2 4/4] t7800: "defaults" is no longer a builtin tool name David Aguilar
2013-02-20 7:05 ` [PATCH v2 3/4] t7800: modernize tests Junio C Hamano
0 siblings, 2 replies; 5+ messages in thread
From: David Aguilar @ 2013-02-20 5:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
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 <davvid@gmail.com>
---
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.<tool>.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.<tool>.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 &&
+ difftool_test_setup &&
+ : >expect &&
+ git difftool --no-prompt --gui --no-gui >actual &&
+ test_cmp expect actual &&
- git config merge.tool bogus-tool &&
- git config diff.tool bogus-tool &&
- git config diff.guitool test-tool &&
- diff=$(git difftool --no-prompt --no-gui --gui branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ test_config merge.tool bogus-tool &&
+ test_config diff.tool bogus-tool &&
+ test_config diff.guitool test-tool &&
+ echo branch >expect &&
+ git difftool --no-prompt --no-gui --gui branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --gui works without configured diff.guitool' '
- git config diff.tool test-tool &&
-
- diff=$(git difftool --no-prompt --gui branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ echo branch >expect &&
+ git difftool --no-prompt --gui branch >actual &&
+ 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 &&
- GIT_DIFF_TOOL=test-tool &&
- export GIT_DIFF_TOOL &&
-
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ 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 the $GIT_*_TOOL variables and ensure
# that $GIT_DIFF_TOOL always wins unless --tool is specified
test_expect_success PERL 'GIT_DIFF_TOOL overrides' '
- git config diff.tool bogus-tool &&
- git config merge.tool bogus-tool &&
-
- GIT_DIFF_TOOL=test-tool &&
- export GIT_DIFF_TOOL &&
-
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "branch" &&
+ difftool_test_setup &&
+ test_config diff.tool bogus-tool &&
+ test_config merge.tool bogus-tool &&
- GIT_DIFF_TOOL=bogus-tool &&
- export GIT_DIFF_TOOL &&
+ echo branch >expect &&
+ GIT_DIFF_TOOL=test-tool git difftool --no-prompt branch >actual &&
+ test_cmp expect actual &&
- diff=$(git difftool --no-prompt --tool=test-tool branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ test_config diff.tool bogus-tool &&
+ test_config merge.tool bogus-tool &&
+ GIT_DIFF_TOOL=bogus-tool \
+ git difftool --no-prompt --tool=test-tool branch >actual &&
+ test_cmp expect actual
'
# Test that we don't have to pass --no-prompt to difftool
# when $GIT_DIFFTOOL_NO_PROMPT is true
test_expect_success PERL 'GIT_DIFFTOOL_NO_PROMPT variable' '
- GIT_DIFFTOOL_NO_PROMPT=true &&
- export GIT_DIFFTOOL_NO_PROMPT &&
-
- diff=$(git difftool branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ echo branch >expect &&
+ GIT_DIFFTOOL_NO_PROMPT=true git difftool branch >actual &&
+ test_cmp expect actual
'
# git-difftool supports the difftool.prompt variable.
# Test that GIT_DIFFTOOL_PROMPT can override difftool.prompt = false
test_expect_success PERL 'GIT_DIFFTOOL_PROMPT variable' '
- git config difftool.prompt false &&
- GIT_DIFFTOOL_PROMPT=true &&
- export GIT_DIFFTOOL_PROMPT &&
-
- prompt=$(echo | git difftool branch | tail -1) &&
- prompt_given "$prompt" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ test_config difftool.prompt false &&
+ echo >input &&
+ GIT_DIFFTOOL_PROMPT=true git difftool branch <input >output &&
+ prompt=$(tail -1 <output) &&
+ prompt_given "$prompt"
'
# Test that we don't have to pass --no-prompt when difftool.prompt is false
test_expect_success PERL 'difftool.prompt config variable is false' '
- git config difftool.prompt false &&
-
- diff=$(git difftool branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ test_config difftool.prompt false &&
+ echo branch >expect &&
+ git difftool branch >actual &&
+ test_cmp expect actual
'
# Test that we don't have to pass --no-prompt when mergetool.prompt is false
test_expect_success PERL 'difftool merge.prompt = false' '
+ difftool_test_setup &&
test_might_fail git config --unset difftool.prompt &&
- git config mergetool.prompt false &&
-
- diff=$(git difftool branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ test_config mergetool.prompt false &&
+ echo branch >expect &&
+ git difftool branch >actual &&
+ test_cmp expect actual
'
# Test that the -y flag can override difftool.prompt = true
test_expect_success PERL 'difftool.prompt can overridden with -y' '
- git config difftool.prompt true &&
-
- diff=$(git difftool -y branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ test_config difftool.prompt true &&
+ echo branch >expect &&
+ git difftool -y branch >actual &&
+ test_cmp expect actual
'
# Test that the --prompt flag can override difftool.prompt = false
test_expect_success PERL 'difftool.prompt can overridden with --prompt' '
- git config difftool.prompt false &&
-
- prompt=$(echo | git difftool --prompt branch | tail -1) &&
- prompt_given "$prompt" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ test_config difftool.prompt false &&
+ echo >input &&
+ git difftool --prompt branch <input >output &&
+ prompt=$(tail -1 <output) &&
+ prompt_given "$prompt"
'
# Test that the last flag passed on the command-line wins
test_expect_success PERL 'difftool last flag wins' '
- diff=$(git difftool --prompt --no-prompt branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults &&
-
- prompt=$(echo | git difftool --no-prompt --prompt branch | tail -1) &&
- prompt_given "$prompt" &&
-
- restore_test_defaults
+ difftool_test_setup &&
+ echo branch >expect &&
+ git difftool --prompt --no-prompt branch >actual &&
+ test_cmp expect actual &&
+ echo >input &&
+ git difftool --no-prompt --prompt branch <input >output &&
+ prompt=$(tail -1 <output) &&
+ prompt_given "$prompt"
'
# git-difftool falls back to git-mergetool config variables
# so test that behavior here
test_expect_success PERL 'difftool + mergetool config variables' '
- remove_config_vars &&
- git config merge.tool test-tool &&
- git config mergetool.test-tool.cmd "cat \$LOCAL" &&
-
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "branch" &&
+ test_config merge.tool test-tool &&
+ test_config mergetool.test-tool.cmd "cat \$LOCAL" &&
+ echo branch >expect &&
+ git difftool --no-prompt branch >actual &&
+ test_cmp expect actual &&
# set merge.tool to something bogus, diff.tool to test-tool
- git config merge.tool bogus-tool &&
- git config diff.tool test-tool &&
-
- diff=$(git difftool --no-prompt branch) &&
- test "$diff" = "branch" &&
-
- restore_test_defaults
+ test_config merge.tool bogus-tool &&
+ test_config diff.tool test-tool &&
+ git difftool --no-prompt branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool.<tool>.path' '
- git config difftool.tkdiff.path echo &&
- diff=$(git difftool --tool=tkdiff --no-prompt branch) &&
- git config --unset difftool.tkdiff.path &&
- lines=$(echo "$diff" | grep file | wc -l) &&
- test "$lines" -eq 1 &&
-
- restore_test_defaults
+ test_config difftool.tkdiff.path echo &&
+ git difftool --tool=tkdiff --no-prompt branch >output &&
+ lines=$(grep file output | wc -l) &&
+ test "$lines" -eq 1
'
test_expect_success PERL 'difftool --extcmd=cat' '
- diff=$(git difftool --no-prompt --extcmd=cat branch) &&
- test "$diff" = branch"$LF"master
+ echo branch >expect &&
+ echo master >>expect &&
+ git difftool --no-prompt --extcmd=cat branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --extcmd cat' '
- diff=$(git difftool --no-prompt --extcmd cat branch) &&
- test "$diff" = branch"$LF"master
+ echo branch >expect &&
+ echo master >>expect &&
+ git difftool --no-prompt --extcmd=cat branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool -x cat' '
- diff=$(git difftool --no-prompt -x cat branch) &&
- test "$diff" = branch"$LF"master
+ echo branch >expect &&
+ echo master >>expect &&
+ git difftool --no-prompt -x cat branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --extcmd echo arg1' '
- diff=$(git difftool --no-prompt --extcmd sh\ -c\ \"echo\ \$1\" branch) &&
- test "$diff" = file
+ echo file >expect &&
+ git difftool --no-prompt \
+ --extcmd sh\ -c\ \"echo\ \$1\" branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --extcmd cat arg1' '
- diff=$(git difftool --no-prompt --extcmd sh\ -c\ \"cat\ \$1\" branch) &&
- test "$diff" = master
+ echo master >expect &&
+ git difftool --no-prompt \
+ --extcmd sh\ -c\ \"cat\ \$1\" branch >actual &&
+ test_cmp expect actual
'
test_expect_success PERL 'difftool --extcmd cat arg2' '
- diff=$(git difftool --no-prompt --extcmd sh\ -c\ \"cat\ \$2\" branch) &&
- test "$diff" = branch
+ echo branch >expect &&
+ git difftool --no-prompt \
+ --extcmd sh\ -c\ \"cat\ \$2\" branch >actual &&
+ test_cmp expect actual
'
# Create a second file on master and a different version on branch
@@ -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 <input >output &&
+ cat output | stdin_contains m2 &&
+ cat output | stdin_contains br2 &&
+ cat output | stdin_doesnot_contain master &&
+ cat output | stdin_doesnot_contain branch
'
test_expect_success PERL 'say no to the second file' '
- diff=$( (echo; echo n) | git difftool -x cat branch ) &&
-
- echo "$diff" | stdin_contains master &&
- echo "$diff" | stdin_contains branch &&
- echo "$diff" | stdin_doesnot_contain m2 &&
- echo "$diff" | stdin_doesnot_contain br2
+ (echo && echo n) >input &&
+ git difftool -x cat branch <input >output &&
+ cat output | stdin_contains master &&
+ cat output | stdin_contains branch &&
+ cat output | stdin_doesnot_contain m2 &&
+ cat output | stdin_doesnot_contain br2
'
test_expect_success PERL 'difftool --tool-help' '
- tool_help=$(git difftool --tool-help) &&
- echo "$tool_help" | stdin_contains tool
+ git difftool --tool-help >output &&
+ cat output | stdin_contains tool
'
test_expect_success PERL 'setup change in subdirectory' '
@@ -359,29 +329,29 @@ test_expect_success PERL 'setup change in subdirectory' '
'
test_expect_success PERL 'difftool -d' '
- diff=$(git difftool -d --extcmd ls branch) &&
- echo "$diff" | stdin_contains sub &&
- echo "$diff" | stdin_contains file
+ git difftool -d --extcmd ls branch >output &&
+ cat output | stdin_contains sub &&
+ cat output | stdin_contains file
'
test_expect_success PERL 'difftool --dir-diff' '
- diff=$(git difftool --dir-diff --extcmd ls branch) &&
- echo "$diff" | stdin_contains sub &&
- echo "$diff" | stdin_contains file
+ git difftool --dir-diff --extcmd ls branch >output &&
+ cat output | stdin_contains sub &&
+ cat output | stdin_contains file
'
test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
- diff=$(git difftool --dir-diff --prompt --extcmd ls branch) &&
- echo "$diff" | stdin_contains sub &&
- echo "$diff" | stdin_contains file
+ git difftool --dir-diff --prompt --extcmd ls branch >output &&
+ cat output | stdin_contains sub &&
+ cat output | stdin_contains file
'
test_expect_success PERL 'difftool --dir-diff from subdirectory' '
(
cd sub &&
- diff=$(git difftool --dir-diff --extcmd ls branch) &&
- echo "$diff" | stdin_contains sub &&
- echo "$diff" | stdin_contains file
+ git difftool --dir-diff --extcmd ls branch >output &&
+ cat output | stdin_contains sub &&
+ cat output | stdin_contains file
)
'
--
1.8.2.rc0.20.gf548dd7
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 4/4] t7800: "defaults" is no longer a builtin tool name
2013-02-20 5:35 ` [PATCH v2 3/4] t7800: modernize tests David Aguilar
@ 2013-02-20 5:35 ` David Aguilar
2013-02-20 7:05 ` [PATCH v2 3/4] t7800: modernize tests Junio C Hamano
1 sibling, 0 replies; 5+ messages in thread
From: David Aguilar @ 2013-02-20 5:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
073678b8e6324a155fa99f40eee0637941a70a34 reworked the
mergetools/ directory so that every file corresponds to a
difftool-supported tool. When this happened the "defaults"
file went away as it was no longer needed by mergetool--lib.
t7800 tests that configured commands can override builtins,
but this test was not adjusted when the "defaults" file was
removed because the test continued to pass.
Adjust the test to use the everlasting "vimdiff" tool name
instead of "defaults" so that it correctly tests against a tool
that is known by mergetool--lib.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Rebased for the new parent patch.
t/t7800-difftool.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index b577c01..18df2c8 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -60,9 +60,9 @@ test_expect_success PERL 'custom commands' '
'
test_expect_success PERL 'custom tool commands override built-ins' '
- test_config difftool.defaults.cmd "cat \$REMOTE" &&
+ test_config difftool.vimdiff.cmd "cat \$REMOTE" &&
echo master >expect &&
- git difftool --tool defaults --no-prompt branch >actual &&
+ git difftool --tool vimdiff --no-prompt branch >actual &&
test_cmp expect actual
'
--
1.8.2.rc0.20.gf548dd7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 3/4] t7800: modernize tests
2013-02-20 5:35 ` [PATCH v2 3/4] t7800: modernize tests David Aguilar
2013-02-20 5:35 ` [PATCH v2 4/4] t7800: "defaults" is no longer a builtin tool name David Aguilar
@ 2013-02-20 7:05 ` Junio C Hamano
1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2013-02-20 7:05 UTC (permalink / raw)
To: David Aguilar; +Cc: git, Jonathan Nieder
David Aguilar <davvid@gmail.com> 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 <davvid@gmail.com>
> ---
> 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 <input >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 <output
be more straight-forward way to say these?
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-02-20 7:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-20 5:35 [PATCH v2 1/4] difftool: silence uninitialized variable warning David Aguilar
2013-02-20 5:35 ` [PATCH v2 2/4] t7800: update copyright notice David Aguilar
2013-02-20 5:35 ` [PATCH v2 3/4] t7800: modernize tests David Aguilar
2013-02-20 5:35 ` [PATCH v2 4/4] t7800: "defaults" is no longer a builtin tool name David Aguilar
2013-02-20 7:05 ` [PATCH v2 3/4] t7800: modernize tests Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).