From: Brian Gernhardt <hidden> Date: 2016-06-15 22:49:22
Instead of using `cd dir && (...) && cd..` use `(cd dir && ...)`
This ensures that the test doesn't get caught in the subdirectory if
there is an error in the subshell.
Signed-off-by: Brian Gernhardt <redacted>
---
t/t7610-mergetool.sh | 49 ++++++++++++++++++++++++++-----------------------
1 files changed, 26 insertions(+), 23 deletions(-)
@@ -68,22 +68,24 @@ test_expect_success 'mergetool crlf' '' test_expect_success'mergetool in subdir''-gitcheckout-btest3branch1-cdsubdir&&(-test_must_failgitmergemaster>/dev/null2>&1&&-(yes""|gitmergetoolfile3>/dev/null2>&1)&&-test"$(catfile3)"="master new sub")&&-cd..+gitcheckout-btest3branch1&&+(+cdsubdir&&+test_must_failgitmergemaster>/dev/null2>&1&&+(yes""|gitmergetoolfile3>/dev/null2>&1)&&+test"$(catfile3)"="master new sub"+)' test_expect_success'mergetool on file in parent dir''-cdsubdir&&(-(yes""|gitmergetool../file1>/dev/null2>&1)&&-(yes""|gitmergetool../file2>/dev/null2>&1)&&-test"$(cat../file1)"="master updated"&&-test"$(cat../file2)"="master new"&&-gitcommit-m"branch1 resolved with mergetool - subdir")&&-cd..+(+cdsubdir&&+(yes""|gitmergetool../file1>/dev/null2>&1)&&+(yes""|gitmergetool../file2>/dev/null2>&1)&&+test"$(cat../file1)"="master updated"&&+test"$(cat../file2)"="master new"&&+gitcommit-m"branch1 resolved with mergetool - subdir"+)' test_expect_success'mergetool skips autoresolved''
@@ -96,16 +98,17 @@ test_expect_success 'mergetool skips autoresolved' '' test_expect_success'mergetool merges all from subdir''-cdsubdir&&(-gitconfigrerere.enabledfalse&&-test_must_failgitmergemaster&&-gitmergetool--no-prompt&&-test"$(cat../file1)"="master updated"&&-test"$(cat../file2)"="master new"&&-test"$(catfile3)"="master new sub"&&-gitadd../file1../file2file3&&-gitcommit-m"branch2 resolved by mergetool from subdir")&&-cd..+(+cdsubdir&&+gitconfigrerere.enabledfalse&&+test_must_failgitmergemaster&&+gitmergetool--no-prompt&&+test"$(cat../file1)"="master updated"&&+test"$(cat../file2)"="master new"&&+test"$(catfile3)"="master new sub"&&+gitadd../file1../file2file3&&+gitcommit-m"branch2 resolved by mergetool from subdir"+)' test_done
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:49:22
Brian Gernhardt wrote:
Instead of using `cd dir && (...) && cd..` use `(cd dir && ...)`
Thank you.
quoted hunk
+++ b/t/t7610-mergetool.sh
@@ -68,22 +68,24 @@ test_expect_success 'mergetool crlf' '' test_expect_success'mergetool in subdir''-gitcheckout-btest3branch1-cdsubdir&&(-test_must_failgitmergemaster>/dev/null2>&1&&-(yes""|gitmergetoolfile3>/dev/null2>&1)&&-test"$(catfile3)"="master new sub")&&-cd..+gitcheckout-btest3branch1&&+(+cdsubdir&&+test_must_failgitmergemaster>/dev/null2>&1&&+(yes""|gitmergetoolfile3>/dev/null2>&1)&&+test"$(catfile3)"="master new sub"+)
While we're looking at this test script, some more nitpicks for
squashing in. :)
-- 8< --
Subject: t7610 (mergetool): more nitpicks
- use tabs to indent
- do not redirect output away unnecessarily
- avoid a subshell for 'yes "" | git mergetool file3'
- use test_tick for reproducible, increasing timestamps
- use test_cmp instead of 'test $foo = bar'; the former is much
nicer to debug with --verbose since it produces a diff.
Signed-off-by: Jonathan Nieder <redacted>
---
t/t7610-mergetool.sh | 186 ++++++++++++++++++++++++++++---------------------
1 files changed, 106 insertions(+), 80 deletions(-)
@@ -3,112 +3,138 @@# Copyright (c) 2008 Charles Bailey#-test_description='gitmergetool+test_description='Testingbasicmergetoolinvocation-Testingbasicmergetoolinvocation'+Allthemergetooltestsworkbycheckingoutatemporarybranchbased+offbranch1andthenmerginginmasterandcheckingtheresultsof+runningmergetool.+' ../test-lib.sh-# All the mergetool test work by checking out a temporary branch based-# off 'branch1' and then merging in master and checking the results of-# running mergetool- test_expect_success'setup''-gitconfigrerere.enabledtrue&&-echomaster>file1&&-mkdirsubdir&&-echomastersub>subdir/file3&&-gitaddfile1subdir/file3&&-gitcommit-m"added file1"&&+gitconfigrerere.enabledtrue&&+echomaster>file1&&+mkdirsubdir&&+echomastersub>subdir/file3&&+gitaddfile1subdir/file3&&+test_tick&&+gitcommit-m"added file1"&&-gitcheckout-bbranch1master&&-echobranch1change>file1&&-echobranch1newfile>file2&&-echobranch1sub>subdir/file3&&-gitaddfile1file2subdir/file3&&-gitcommit-m"branch1 changes"&&+gitcheckout-bbranch1master&&+echobranch1change>file1&&+echobranch1newfile>file2&&+echobranch1sub>subdir/file3&&+gitaddfile1file2subdir/file3&&+test_tick&&+gitcommit-m"branch1 changes"&&-gitcheckoutmaster&&-echomasterupdated>file1&&-echomasternew>file2&&-echomasternewsub>subdir/file3&&-gitaddfile1file2subdir/file3&&-gitcommit-m"master updates"&&+gitcheckoutmaster&&+echomasterupdated>file1&&+echomasternew>file2&&+echomasternewsub>subdir/file3&&+gitaddfile1file2subdir/file3&&+test_tick&&+gitcommit-m"master updates"&&-gitconfigmerge.toolmytool&&-gitconfigmergetool.mytool.cmd"cat \"\$REMOTE\" >\"\$MERGED\""&&-gitconfigmergetool.mytool.trustExitCodetrue+gitconfigmerge.toolmytool&&+gitconfigmergetool.mytool.cmd"cat \"\$REMOTE\" >\"\$MERGED\""&&+gitconfigmergetool.mytool.trustExitCodetrue' test_expect_success'custom mergetool''-gitcheckout-btest1branch1&&-test_must_failgitmergemaster>/dev/null2>&1&&-(yes""|gitmergetoolfile1>/dev/null2>&1)&&-(yes""|gitmergetoolfile2>/dev/null2>&1)&&-(yes""|gitmergetoolsubdir/file3>/dev/null2>&1)&&-test"$(catfile1)"="master updated"&&-test"$(catfile2)"="master new"&&-test"$(catsubdir/file3)"="master new sub"&&-gitcommit-m"branch1 resolved with mergetool"+echomasterupdated>file1.expected&&+echomasternew>file2.expected&&+echomasternewsub>sub.expected&&++gitcheckout-btest1branch1&&+test_must_failgitmergemaster&&+yes""|gitmergetoolfile1&&+yes""|gitmergetoolfile2&&+yes""|gitmergetoolsubdir/file3&&++test_cmpfile1.expectedfile1&&+test_cmpfile2.expectedfile2&&+test_cmpsub.expectedsubdir/file3&&++gitcommit-m"branch1 resolved with mergetool"' test_expect_success'mergetool crlf''-gitconfigcore.autocrlftrue&&-gitcheckout-btest2branch1-test_must_failgitmergemaster>/dev/null2>&1&&-(yes""|gitmergetoolfile1>/dev/null2>&1)&&-(yes""|gitmergetoolfile2>/dev/null2>&1)&&-(yes""|gitmergetoolsubdir/file3>/dev/null2>&1)&&-test"$(printfx|catfile1-)"="$(printf"master updated\r\nx")"&&-test"$(printfx|catfile2-)"="$(printf"master new\r\nx")"&&-test"$(printfx|catsubdir/file3-)"="$(printf"master new sub\r\nx")"&&-gitcommit-m"branch1 resolved with mergetool - autocrlf"&&-gitconfigcore.autocrlffalse&&-gitreset--hard+gitconfigcore.autocrlftrue&&+test_when_finished"git config --unset core.autocrlf"&&+echomasterupdated|append_cr>file1.expected&&+echomasternew|append_cr>file2.expected&&+echomasternewsub|append_cr>sub.expected&&++gitcheckout-btest2branch1&&+test_must_failgitmergemaster&&+test_when_finished"git reset --hard"&&+yes""|gitmergetoolfile1&&+yes""|gitmergetoolfile2&&+yes""|gitmergetoolsubdir/file3&&++test_cmpfile1.expectedfile1&&+test_cmpfile2.expectedfile2&&+test_cmpsub.expectedsubdir/file3&&++gitcommit-m"branch1 resolved with mergetool - autocrlf"' test_expect_success'mergetool in subdir''-gitcheckout-btest3branch1&&-(-cdsubdir&&-test_must_failgitmergemaster>/dev/null2>&1&&-(yes""|gitmergetoolfile3>/dev/null2>&1)&&-test"$(catfile3)"="master new sub"-)+echomasternewsub>sub.expected&&+gitcheckout-btest3branch1&&+(+cdsubdir&&+test_must_failgitmergemaster&&+yes""|gitmergetoolfile3&&+test_cmp../sub.expectedfile3+)' test_expect_success'mergetool on file in parent dir''-(-cdsubdir&&-(yes""|gitmergetool../file1>/dev/null2>&1)&&-(yes""|gitmergetool../file2>/dev/null2>&1)&&-test"$(cat../file1)"="master updated"&&-test"$(cat../file2)"="master new"&&-gitcommit-m"branch1 resolved with mergetool - subdir"-)+echomasterupdated>file1.expected&&+echomasternew>file2.expected&&+(+cdsubdir&&+yes""|gitmergetool../file1&&+yes""|gitmergetool../file2&&+test_cmp../file1.expected../file1&&+test_cmp../file2.expected../file2&&+gitcommit-m"branch1 resolved with mergetool - subdir"+)' test_expect_success'mergetool skips autoresolved''-gitcheckout-btest4branch1&&-test_must_failgitmergemaster&&-test-n"$(gitls-files-u)"&&-output="$(gitmergetool--no-prompt)"&&-test"$output"="No files need merging"&&-gitreset--hard+echo"No files need merging">expected&&+gitcheckout-btest4branch1&&+test_must_failgitmergemaster&&+test_when_finished"git reset --hard"&&+test_must_failgitupdate-index--refresh&&+gitmergetool--no-prompt>actual&&+test_cmpexpectedactual' test_expect_success'mergetool merges all from subdir''-(-cdsubdir&&-gitconfigrerere.enabledfalse&&-test_must_failgitmergemaster&&-gitmergetool--no-prompt&&-test"$(cat../file1)"="master updated"&&-test"$(cat../file2)"="master new"&&-test"$(catfile3)"="master new sub"&&-gitadd../file1../file2file3&&-gitcommit-m"branch2 resolved by mergetool from subdir"-)+echomasterupdated>file1.expected&&+echomasternew>file2.expected&&+echomasternewsub>sub.expected&&++gitconfigrerere.enabledfalse&&+test_when_finished"git config rerere.enabled true"&&+mv.git/rr-cache.git/rr-cache-moved&&+test_when_finished"mv .git/rr-cache-moved .git/rr-cache"&&+(+cdsubdir&&+test_must_failgitmergemaster&&+test_when_finished"git reset --hard"&&+gitmergetool--no-prompt&&+test_cmp../file1.expected../file1&&+test_cmp../file2.expected../file2&&+test_cmp../sub.expectedfile3&&+gitadd../file1../file2file3&&+gitcommit-m"branch2 resolved by mergetool from subdir"+)' test_done
From: Brian Gernhardt <hidden> Date: 2016-06-15 22:49:22
On Aug 23, 2010, at 11:05 PM, Jonathan Nieder wrote:
While we're looking at this test script, some more nitpicks for
squashing in. :)
-- 8< --
Subject: t7610 (mergetool): more nitpicks
- use tabs to indent
- do not redirect output away unnecessarily
- avoid a subshell for 'yes "" | git mergetool file3'
- use test_tick for reproducible, increasing timestamps
- use test_cmp instead of 'test $foo = bar'; the former is much
nicer to debug with --verbose since it produces a diff.
Fair enough. I just took a stab at the bit that irritated me the most. Thanks for cleaning up the rest.
~~ Brian
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:49:22
- use tabs to indent
- do not redirect output away unnecessarily
- avoid a subshell for 'yes "" | git mergetool file3'
- use test_tick for reproducible, increasing timestamps
- use test_cmp instead of 'test $foo = bar'; the former is much
nicer to debug with --verbose since it produces a diff.
Helped-by: Junio C Hamano [off-list ref]
Signed-off-by: Jonathan Nieder <redacted>
---
Jonathan Nieder wrote:
As Junio noticed, the net effect is to run "reset --hard" before
the "[core] autocrlf" configuration is removed, which could be
an undesirable behavior change. So this version runs "reset --hard"
after unsetting autocrlf for good measure.
t/t7610-mergetool.sh | 187 +++++++++++++++++++++++++++++---------------------
1 files changed, 108 insertions(+), 79 deletions(-)
@@ -3,112 +3,141 @@# Copyright (c) 2008 Charles Bailey#-test_description='gitmergetool+test_description='Testingbasicmergetoolinvocation-Testingbasicmergetoolinvocation'+Allthemergetooltestsworkbycheckingoutatemporarybranchbased+offbranch1andthenmerginginmasterandcheckingtheresultsof+runningmergetool.+' ../test-lib.sh-# All the mergetool test work by checking out a temporary branch based-# off 'branch1' and then merging in master and checking the results of-# running mergetool- test_expect_success'setup''-gitconfigrerere.enabledtrue&&-echomaster>file1&&-mkdirsubdir&&-echomastersub>subdir/file3&&-gitaddfile1subdir/file3&&-gitcommit-m"added file1"&&+gitconfigrerere.enabledtrue&&+echomaster>file1&&+mkdirsubdir&&+echomastersub>subdir/file3&&+gitaddfile1subdir/file3&&+test_tick&&+gitcommit-m"added file1"&&-gitcheckout-bbranch1master&&-echobranch1change>file1&&-echobranch1newfile>file2&&-echobranch1sub>subdir/file3&&-gitaddfile1file2subdir/file3&&-gitcommit-m"branch1 changes"&&+gitcheckout-bbranch1master&&+echobranch1change>file1&&+echobranch1newfile>file2&&+echobranch1sub>subdir/file3&&+gitaddfile1file2subdir/file3&&+test_tick&&+gitcommit-m"branch1 changes"&&-gitcheckoutmaster&&-echomasterupdated>file1&&-echomasternew>file2&&-echomasternewsub>subdir/file3&&-gitaddfile1file2subdir/file3&&-gitcommit-m"master updates"&&+gitcheckoutmaster&&+echomasterupdated>file1&&+echomasternew>file2&&+echomasternewsub>subdir/file3&&+gitaddfile1file2subdir/file3&&+test_tick&&+gitcommit-m"master updates"&&-gitconfigmerge.toolmytool&&-gitconfigmergetool.mytool.cmd"cat \"\$REMOTE\" >\"\$MERGED\""&&-gitconfigmergetool.mytool.trustExitCodetrue+gitconfigmerge.toolmytool&&+gitconfigmergetool.mytool.cmd"cat \"\$REMOTE\" >\"\$MERGED\""&&+gitconfigmergetool.mytool.trustExitCodetrue' test_expect_success'custom mergetool''-gitcheckout-btest1branch1&&-test_must_failgitmergemaster>/dev/null2>&1&&-(yes""|gitmergetoolfile1>/dev/null2>&1)&&-(yes""|gitmergetoolfile2>/dev/null2>&1)&&-(yes""|gitmergetoolsubdir/file3>/dev/null2>&1)&&-test"$(catfile1)"="master updated"&&-test"$(catfile2)"="master new"&&-test"$(catsubdir/file3)"="master new sub"&&-gitcommit-m"branch1 resolved with mergetool"+echomasterupdated>file1.expected&&+echomasternew>file2.expected&&+echomasternewsub>sub.expected&&++gitcheckout-btest1branch1&&+test_must_failgitmergemaster&&+yes""|gitmergetoolfile1&&+yes""|gitmergetoolfile2&&+yes""|gitmergetoolsubdir/file3&&++test_cmpfile1.expectedfile1&&+test_cmpfile2.expectedfile2&&+test_cmpsub.expectedsubdir/file3&&++gitcommit-m"branch1 resolved with mergetool"' test_expect_success'mergetool crlf''-gitconfigcore.autocrlftrue&&-gitcheckout-btest2branch1-test_must_failgitmergemaster>/dev/null2>&1&&-(yes""|gitmergetoolfile1>/dev/null2>&1)&&-(yes""|gitmergetoolfile2>/dev/null2>&1)&&-(yes""|gitmergetoolsubdir/file3>/dev/null2>&1)&&-test"$(printfx|catfile1-)"="$(printf"master updated\r\nx")"&&-test"$(printfx|catfile2-)"="$(printf"master new\r\nx")"&&-test"$(printfx|catsubdir/file3-)"="$(printf"master new sub\r\nx")"&&-gitcommit-m"branch1 resolved with mergetool - autocrlf"&&-gitconfigcore.autocrlffalse&&-gitreset--hard+gitconfigcore.autocrlftrue&&+test_when_finished"+gitconfig--unsetcore.autocrlf&&+gitreset--hard+" &&+echomasterupdated|append_cr>file1.expected&&+echomasternew|append_cr>file2.expected&&+echomasternewsub|append_cr>sub.expected&&++gitcheckout-btest2branch1&&+test_must_failgitmergemaster&&+test_when_finished"git reset --hard"&&+yes""|gitmergetoolfile1&&+yes""|gitmergetoolfile2&&+yes""|gitmergetoolsubdir/file3&&++test_cmpfile1.expectedfile1&&+test_cmpfile2.expectedfile2&&+test_cmpsub.expectedsubdir/file3&&++gitcommit-m"branch1 resolved with mergetool - autocrlf"' test_expect_success'mergetool in subdir''-gitcheckout-btest3branch1&&-(-cdsubdir&&-test_must_failgitmergemaster>/dev/null2>&1&&-(yes""|gitmergetoolfile3>/dev/null2>&1)&&-test"$(catfile3)"="master new sub"-)+echomasternewsub>sub.expected&&+gitcheckout-btest3branch1&&+(+cdsubdir&&+test_must_failgitmergemaster&&+yes""|gitmergetoolfile3&&+test_cmp../sub.expectedfile3+)' test_expect_success'mergetool on file in parent dir''-(-cdsubdir&&-(yes""|gitmergetool../file1>/dev/null2>&1)&&-(yes""|gitmergetool../file2>/dev/null2>&1)&&-test"$(cat../file1)"="master updated"&&-test"$(cat../file2)"="master new"&&-gitcommit-m"branch1 resolved with mergetool - subdir"-)+echomasterupdated>file1.expected&&+echomasternew>file2.expected&&+(+cdsubdir&&+yes""|gitmergetool../file1&&+yes""|gitmergetool../file2&&+test_cmp../file1.expected../file1&&+test_cmp../file2.expected../file2&&+gitcommit-m"branch1 resolved with mergetool - subdir"+)' test_expect_success'mergetool skips autoresolved''-gitcheckout-btest4branch1&&-test_must_failgitmergemaster&&-test-n"$(gitls-files-u)"&&-output="$(gitmergetool--no-prompt)"&&-test"$output"="No files need merging"&&-gitreset--hard+echo"No files need merging">expected&&+gitcheckout-btest4branch1&&+test_must_failgitmergemaster&&+test_when_finished"git reset --hard"&&+test_must_failgitupdate-index--refresh&&+gitmergetool--no-prompt>actual&&+test_cmpexpectedactual' test_expect_success'mergetool merges all from subdir''-(-cdsubdir&&+echomasterupdated>file1.expected&&+echomasternew>file2.expected&&+echomasternewsub>sub.expected&&+gitconfigrerere.enabledfalse&&-test_must_failgitmergemaster&&-gitmergetool--no-prompt&&-test"$(cat../file1)"="master updated"&&-test"$(cat../file2)"="master new"&&-test"$(catfile3)"="master new sub"&&-gitadd../file1../file2file3&&-gitcommit-m"branch2 resolved by mergetool from subdir"-)+test_when_finished"git config rerere.enabled true"&&+mv.git/rr-cache.git/rr-cache-moved&&+test_when_finished"mv .git/rr-cache-moved .git/rr-cache"&&+(+cdsubdir&&+test_must_failgitmergemaster&&+test_when_finished"git reset --hard"&&+gitmergetool--no-prompt&&+test_cmp../file1.expected../file1&&+test_cmp../file2.expected../file2&&+test_cmp../sub.expectedfile3&&+gitadd../file1../file2file3&&+gitcommit-m"branch2 resolved by mergetool from subdir"+)' test_done
From: David Aguilar <hidden> Date: 2016-06-15 22:49:23
On Tue, Aug 24, 2010 at 07:25:52PM -0500, Jonathan Nieder wrote:
- use tabs to indent
[...]
Cool. I'd like to do the same to git-mergetool.sh too.
Until now I've stuck to the existing style.
My editor is setup to display tabs visually which makes it easy
for me to emulate existing code but not everyone does that.
Any objections to a patch that replaces the mixed 4-space+tab
indents with pure tabs?
It would make the git-mergetool consistent with
git-mergetool--lib, git-difftool--helper, and their tests.
I don't know if pure-tabs is the preferred style for *.sh
scripts. It's not explicitly mentioned in CodingGuidelines'
shell script section. Updating all of the *.sh scripts, though,
seems like code churn so I wouldn't recommend that unless we
were going to be changing the scripts anyways.
--
David
From: Charles Bailey <hidden> Date: 2016-06-15 22:49:23
On Wed, Aug 25, 2010 at 12:40:38AM -0700, David Aguilar wrote:
On Tue, Aug 24, 2010 at 07:25:52PM -0500, Jonathan Nieder wrote:
quoted
- use tabs to indent
[...]
Cool. I'd like to do the same to git-mergetool.sh too.
Until now I've stuck to the existing style.
My editor is setup to display tabs visually which makes it easy
for me to emulate existing code but not everyone does that.
Any objections to a patch that replaces the mixed 4-space+tab
indents with pure tabs?
Just the same objection as the last time:
http://thread.gmane.org/gmane.comp.version-control.git/115069/focus=115192
We've already tidyied up mergetool to be consistent, I don't see what
touching 90% of the lines achieves other than getting yourself
'blamed' for everything in mergetool.
Charles.
From: David Aguilar <hidden> Date: 2016-06-15 22:49:23
On Aug 25, 2010, at 12:56 AM, Charles Bailey [off-list ref]
wrote:
On Wed, Aug 25, 2010 at 12:40:38AM -0700, David Aguilar wrote:
quoted
On Tue, Aug 24, 2010 at 07:25:52PM -0500, Jonathan Nieder wrote:
quoted
- use tabs to indent
[...]
Cool. I'd like to do the same to git-mergetool.sh too.
Until now I've stuck to the existing style.
My editor is setup to display tabs visually which makes it easy
for me to emulate existing code but not everyone does that.
Any objections to a patch that replaces the mixed 4-space+tab
indents with pure tabs?
Just the same objection as the last time:
http://thread.gmane.org/gmane.comp.version-control.git/115069/focus=115192
We've already tidyied up mergetool to be consistent, I don't see what
touching 90% of the lines achieves other than getting yourself
'blamed' for everything in mergetool.
Charles.
Too true.
Instead of that I should probably get around to factoring out the tool-
specific parts like we discussed in the past.
Thanks Charles,
--
David