David Aguilar <davvid@xxxxxxxxx> 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@xxxxxxxxx> > --- > 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? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html