{"thread":{"id":"45068","subject":"[PATCH 1/2] t7800: simplify basic usage test","startedAt":"2017-02-07T09:17:13Z","lastAt":"2017-02-07T12:02:51Z","messageCount":3,"participants":["David Aguilar","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"310973","messageId":"20170207091700.20156-1-davvid@gmail.com","threadId":"45068","inReplyTo":null,"subject":"[PATCH 1/2] t7800: simplify basic usage test","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2017-02-07T09:16:59Z","receivedAt":"2017-02-07T09:17:13Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Use \"test_line_count\" instead of \"wc -l\", use \"git -C\" instead of a\nsubshell, and use test_expect_code when calling difftool.  Ease\ndebugging by capturing output into temporary files.\n\nSuggested-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\nThis patch applies on top of js/difftool-builtin in next\n\"difftool: fix bug when printing usage\"\n\n t/t7800-difftool.sh | 15 +++++++--------\n 1 file changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 21e2ac4ad6..97bae54d83 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -24,16 +24,15 @@ prompt_given ()\n }\n \n test_expect_success 'basic usage requires no repo' '\n-\tlines=$(git difftool -h | grep ^usage: | wc -l) &&\n-\ttest \"$lines\" -eq 1 &&\n+\ttest_expect_code 129 git difftool -h >output &&\n+\tgrep ^usage: output &&\n \t# create a ceiling directory to prevent Git from finding a repo\n \tmkdir -p not/repo &&\n-\tceiling=\"$PWD/not\" &&\n-\tlines=$(cd not/repo &&\n-\t\tGIT_CEILING_DIRECTORIES=\"$ceiling\" git difftool -h |\n-\t\tgrep ^usage: | wc -l) &&\n-\ttest \"$lines\" -eq 1 &&\n-\trmdir -p not/repo\n+\ttest_when_finished rm -r not &&\n+\ttest_expect_code 129 \\\n+\tenv GIT_CEILING_DIRECTORIES=\"$(pwd)/not\" \\\n+\tgit -C not/repo difftool -h >output &&\n+\tgrep ^usage: output\n '\n \n # Create a file on master and change it on branch\n-- \n2.12.0.rc0.228.g6c028b8e94\n\n"},{"id":"310974","messageId":"20170207091700.20156-2-davvid@gmail.com","threadId":"45068","inReplyTo":"20170207091700.20156-1-davvid@gmail.com","subject":"[PATCH 2/2] t7800: replace \"wc -l\" with test_line_count","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2017-02-07T09:17:00Z","receivedAt":"2017-02-07T09:17:16Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Make t7800 easier to debug by capturing output into temporary files and\nusing test_line_count to make assertions on those files.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n t/t7800-difftool.sh | 31 +++++++++++++++++++------------\n 1 file changed, 19 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 97bae54d83..25241f4096 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -290,8 +290,8 @@ test_expect_success 'difftool + mergetool config variables' '\n test_expect_success 'difftool.<tool>.path' '\n \ttest_config difftool.tkdiff.path echo &&\n \tgit difftool --tool=tkdiff --no-prompt branch >output &&\n-\tlines=$(grep file output | wc -l) &&\n-\ttest \"$lines\" -eq 1\n+\tgrep file output >grep-output &&\n+\ttest_line_count = 1 grep-output\n '\n \n test_expect_success 'difftool --extcmd=cat' '\n@@ -428,9 +428,12 @@ run_dir_diff_test 'difftool --dir-diff branch from subdirectory' '\n \t\tgit difftool --dir-diff $symlinks --extcmd ls branch >output &&\n \t\t# \"sub\" must only exist in \"right\"\n \t\t# \"file\" and \"file2\" must be listed in both \"left\" and \"right\"\n-\t\ttest \"1\" = $(grep sub output | wc -l) &&\n-\t\ttest \"2\" = $(grep file\"$\" output | wc -l) &&\n-\t\ttest \"2\" = $(grep file2 output | wc -l)\n+\t\tgrep sub output > sub-output &&\n+\t\ttest_line_count = 1 sub-output &&\n+\t\tgrep file\"$\" output >file-output &&\n+\t\ttest_line_count = 2 file-output &&\n+\t\tgrep file2 output >file2-output &&\n+\t\ttest_line_count = 2 file2-output\n \t)\n '\n \n@@ -440,9 +443,11 @@ run_dir_diff_test 'difftool --dir-diff v1 from subdirectory' '\n \t\tgit difftool --dir-diff $symlinks --extcmd ls v1 >output &&\n \t\t# \"sub\" and \"file\" exist in both v1 and HEAD.\n \t\t# \"file2\" is unchanged.\n-\t\ttest \"2\" = $(grep sub output | wc -l) &&\n-\t\ttest \"2\" = $(grep file output | wc -l) &&\n-\t\ttest \"0\" = $(grep file2 output | wc -l)\n+\t\tgrep sub output >sub-output &&\n+\t\ttest_line_count = 2 sub-output &&\n+\t\tgrep file output >file-output &&\n+\t\ttest_line_count = 2 file-output &&\n+\t\t! grep file2 output\n \t)\n '\n \n@@ -452,8 +457,9 @@ run_dir_diff_test 'difftool --dir-diff branch from subdirectory w/ pathspec' '\n \t\tgit difftool --dir-diff $symlinks --extcmd ls branch -- .>output &&\n \t\t# \"sub\" only exists in \"right\"\n \t\t# \"file\" and \"file2\" must not be listed\n-\t\ttest \"1\" = $(grep sub output | wc -l) &&\n-\t\ttest \"0\" = $(grep file output | wc -l)\n+\t\tgrep sub output >sub-output &&\n+\t\ttest_line_count = 1 sub-output &&\n+\t\t! grep file output\n \t)\n '\n \n@@ -463,8 +469,9 @@ run_dir_diff_test 'difftool --dir-diff v1 from subdirectory w/ pathspec' '\n \t\tgit difftool --dir-diff $symlinks --extcmd ls v1 -- .>output &&\n \t\t# \"sub\" exists in v1 and HEAD\n \t\t# \"file\" is filtered out by the pathspec\n-\t\ttest \"2\" = $(grep sub output | wc -l) &&\n-\t\ttest \"0\" = $(grep file output | wc -l)\n+\t\tgrep sub output >sub-output &&\n+\t\ttest_line_count = 2 sub-output &&\n+\t\t! grep file output\n \t)\n '\n \n-- \n2.12.0.rc0.228.g6c028b8e94\n\n"},{"id":"310981","messageId":"alpine.DEB.2.20.1702071300021.3496@virtualbox","threadId":"45068","inReplyTo":"20170207091700.20156-2-davvid@gmail.com","subject":"Re: [PATCH 2/2] t7800: replace \"wc -l\" with test_line_count","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-02-07T12:02:36Z","receivedAt":"2017-02-07T12:02:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi David,\n\nOn Tue, 7 Feb 2017, David Aguilar wrote:\n\n> Make t7800 easier to debug by capturing output into temporary files and\n> using test_line_count to make assertions on those files.\n> \n> Signed-off-by: David Aguilar <davvid@gmail.com>\n\nBoth patches look like an obvious improvement with no obvious bugs to me.\n\nIn this case, I allowed myself to forego the more thorough code review in\nfavor of merely glancing over the diffs, seeing as the changes do not\nreally need a lot of context.\n\nThank you,\nJohannes\n"}]}