{"thread":{"id":"54879","subject":"[PATCH] t7800-difftool: don't accidentally match tmp dirs","startedAt":"2020-12-24T09:25:25Z","lastAt":"2021-01-09T21:41:17Z","messageCount":7,"participants":["SZEDER Gábor","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"412993","messageId":"20201224092431.13354-1-szeder.dev@gmail.com","threadId":"54879","inReplyTo":null,"subject":"[PATCH] t7800-difftool: don't accidentally match tmp dirs","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-12-24T09:24:31Z","receivedAt":"2020-12-24T09:25:25Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"In a bunch of test cases in 't7800-difftool.sh' we 'grep' for specific\nfilenames in 'git difftool's output, and those test cases are prone to\noccasional failures because those filenames might be part of the name\nof difftool's temporary directory as well, e.g.:\n\n  +git difftool --dir-diff --no-symlinks --extcmd ls v1\n  +grep sub output\n  +test_line_count = 2 sub-output\n  test_line_count: line count for sub-output != 2\n  /tmp/git-difftool.Ssubfq/left/:\n  sub\n  /tmp/git-difftool.Ssubfq/right/:\n  sub\n  error: last command exited with $?=1\n  not ok 50 - difftool --dir-diff v1 from subdirectory --no-symlinks\n\nFix this by tightening those test cases: filter out difftool's\ntemporary directories from its output, and use here docs to list and\ntest_cmp to check all files expected to present in those directories\nexplicitly.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t7800-difftool.sh | 112 ++++++++++++++++++++++++++++++--------------\n 1 file changed, 78 insertions(+), 34 deletions(-)\n\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex a578b35761..fe02fe1688 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -439,73 +439,104 @@ run_dir_diff_test () {\n }\n \n run_dir_diff_test 'difftool -d' '\n+\tcat >expect <<-\\EOF &&\n+\tfile\n+\tfile2\n+\n+\tfile\n+\tfile2\n+\tsub\n+\tEOF\n \tgit difftool -d $symlinks --extcmd ls branch >output &&\n-\tgrep sub output &&\n-\tgrep file output\n+\tgrep -v ^/ output >actual &&\n+\ttest_cmp expect actual\n '\n \n run_dir_diff_test 'difftool --dir-diff' '\n+\tcat >expect <<-\\EOF &&\n+\tfile\n+\tfile2\n+\n+\tfile\n+\tfile2\n+\tsub\n+\tEOF\n \tgit difftool --dir-diff $symlinks --extcmd ls branch >output &&\n-\tgrep sub output &&\n-\tgrep file output\n+\tgrep -v ^/ output >actual &&\n+\ttest_cmp expect actual\n '\n \n run_dir_diff_test 'difftool --dir-diff ignores --prompt' '\n+\tcat >expect <<-\\EOF &&\n+\tfile\n+\tfile2\n+\n+\tfile\n+\tfile2\n+\tsub\n+\tEOF\n \tgit difftool --dir-diff $symlinks --prompt --extcmd ls branch >output &&\n-\tgrep sub output &&\n-\tgrep file output\n+\tgrep -v ^/ output >actual &&\n+\ttest_cmp expect actual\n '\n \n run_dir_diff_test 'difftool --dir-diff branch from subdirectory' '\n \t(\n \t\tcd sub &&\n+\t\tcat >expect <<-\\EOF &&\n+\t\tfile\n+\t\tfile2\n+\n+\t\tfile\n+\t\tfile2\n+\t\tsub\n+\t\tEOF\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\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\tgrep -v ^/ output >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n run_dir_diff_test 'difftool --dir-diff v1 from subdirectory' '\n \t(\n \t\tcd sub &&\n+\t\tcat >expect <<-\\EOF &&\n+\t\tfile\n+\t\tsub\n+\n+\t\tfile\n+\t\tsub\n+\t\tEOF\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\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\tgrep -v ^/ output >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n run_dir_diff_test 'difftool --dir-diff branch from subdirectory w/ pathspec' '\n \t(\n \t\tcd sub &&\n+\t\tcat >expect <<-\\EOF &&\n+\n+\t\tsub\n+\t\tEOF\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\tgrep sub output >sub-output &&\n-\t\ttest_line_count = 1 sub-output &&\n-\t\t! grep file output\n+\t\tgrep -v ^/ output >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n run_dir_diff_test 'difftool --dir-diff v1 from subdirectory w/ pathspec' '\n \t(\n \t\tcd sub &&\n+\t\tcat >expect <<-\\EOF &&\n+\t\tsub\n+\n+\t\tsub\n+\t\tEOF\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\tgrep sub output >sub-output &&\n-\t\ttest_line_count = 2 sub-output &&\n-\t\t! grep file output\n+\t\tgrep -v ^/ output >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n@@ -516,18 +547,31 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory with GIT_DIR set' '\n \t\tGIT_WORK_TREE=$(pwd) &&\n \t\texport GIT_WORK_TREE &&\n \t\tcd sub &&\n+\t\tcat >expect <<-\\EOF &&\n+\n+\t\tsub\n+\t\tEOF\n \t\tgit difftool --dir-diff $symlinks --extcmd ls \\\n \t\t\tbranch -- sub >output &&\n-\t\tgrep sub output &&\n-\t\t! grep file output\n+\t\tgrep -v ^/ output >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n run_dir_diff_test 'difftool --dir-diff when worktree file is missing' '\n \ttest_when_finished git reset --hard &&\n \trm file2 &&\n+\tcat >expect <<-\\EOF &&\n+\tfile\n+\tfile2\n+\n+\tfile\n+\tfile2\n+\tsub\n+\tEOF\n \tgit difftool --dir-diff $symlinks --extcmd ls branch master >output &&\n-\tgrep file2 output\n+\tgrep -v ^/ output >actual &&\n+\ttest_cmp expect actual\n '\n \n run_dir_diff_test 'difftool --dir-diff with unmerged files' '\n-- \n2.30.0.rc2.444.gff896a3b01\n\n"},{"id":"413645","messageId":"xmqq35zdnu1w.fsf@gitster.c.googlers.com","threadId":"54879","inReplyTo":"20201224092431.13354-1-szeder.dev@gmail.com","subject":"Re: [PATCH] t7800-difftool: don't accidentally match tmp dirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-07T06:24:27Z","receivedAt":"2021-01-07T06:25:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> index a578b35761..fe02fe1688 100755\n> --- a/t/t7800-difftool.sh\n> +++ b/t/t7800-difftool.sh\n> @@ -439,73 +439,104 @@ run_dir_diff_test () {\n>  }\n>  \n>  run_dir_diff_test 'difftool -d' '\n> +\tcat >expect <<-\\EOF &&\n> +\tfile\n> +\tfile2\n> +\n> +\tfile\n> +\tfile2\n> +\tsub\n> +\tEOF\n>  \tgit difftool -d $symlinks --extcmd ls branch >output &&\n> -\tgrep sub output &&\n> -\tgrep file output\n> +\tgrep -v ^/ output >actual &&\n\nThis unfortunately would not catch full paths on certain platforms.\n\nSee https://github.com/git/git/runs/1660588243?check_suite_focus=true#step:7:4186\nfor an example X-<.\n\n> +\ttest_cmp expect actual\n>  '\n"},{"id":"413797","messageId":"20210108092036.GR8396@szeder.dev","threadId":"54879","inReplyTo":"xmqq35zdnu1w.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] t7800-difftool: don't accidentally match tmp dirs","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-01-08T09:20:36Z","receivedAt":"2021-01-08T09:21:24Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Jan 06, 2021 at 10:24:27PM -0800, Junio C Hamano wrote:\n> SZEDER Gábor <szeder.dev@gmail.com> writes:\n> \n> > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> > index a578b35761..fe02fe1688 100755\n> > --- a/t/t7800-difftool.sh\n> > +++ b/t/t7800-difftool.sh\n> > @@ -439,73 +439,104 @@ run_dir_diff_test () {\n> >  }\n> >  \n> >  run_dir_diff_test 'difftool -d' '\n> > +\tcat >expect <<-\\EOF &&\n> > +\tfile\n> > +\tfile2\n> > +\n> > +\tfile\n> > +\tfile2\n> > +\tsub\n> > +\tEOF\n> >  \tgit difftool -d $symlinks --extcmd ls branch >output &&\n> > -\tgrep sub output &&\n> > -\tgrep file output\n> > +\tgrep -v ^/ output >actual &&\n> \n> This unfortunately would not catch full paths on certain platforms.\n> \n> See https://github.com/git/git/runs/1660588243?check_suite_focus=true#step:7:4186\n> for an example X-<.\n\nHrm, one has to log in to view those CI logs?  Really?! :(\n\nAnyway, I (apparently falsely) assumed that the output these tests\nlook at come from Git itself, and therefore we can rely on difftool's\ntemporary directories being normalized UNIX-style absolute paths...\nBut it seems they don't actually come from Git but from 'ls', because\nthat's what those '--extcmd ls' options do, and I now going to assume\nthat 'ls' prints those absolute paths with drive letter prefixes and\nwhatnot on Windows.\n\nThe initial version of this patch just tightened all potentially\nproblematic 'grep' patterns, e.g. 'grep ^sub$ output && grep ^file$\noutput'.  That should work on Windows as well, shouldn't it.  Will see\nwhether I can dig it out from the reflogs.\n\n"},{"id":"413826","messageId":"nycvar.QRO.7.76.6.2101081622530.2213@tvgsbejvaqbjf.bet","threadId":"54879","inReplyTo":"20210108092036.GR8396@szeder.dev","subject":"Re: [PATCH] t7800-difftool: don't accidentally match tmp dirs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-08T15:23:32Z","receivedAt":"2021-01-08T15:25:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Fri, 8 Jan 2021, SZEDER Gábor wrote:\n\n> On Wed, Jan 06, 2021 at 10:24:27PM -0800, Junio C Hamano wrote:\n> > SZEDER Gábor <szeder.dev@gmail.com> writes:\n> >\n> > > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> > > index a578b35761..fe02fe1688 100755\n> > > --- a/t/t7800-difftool.sh\n> > > +++ b/t/t7800-difftool.sh\n> > > @@ -439,73 +439,104 @@ run_dir_diff_test () {\n> > >  }\n> > >\n> > >  run_dir_diff_test 'difftool -d' '\n> > > +\tcat >expect <<-\\EOF &&\n> > > +\tfile\n> > > +\tfile2\n> > > +\n> > > +\tfile\n> > > +\tfile2\n> > > +\tsub\n> > > +\tEOF\n> > >  \tgit difftool -d $symlinks --extcmd ls branch >output &&\n> > > -\tgrep sub output &&\n> > > -\tgrep file output\n> > > +\tgrep -v ^/ output >actual &&\n> >\n> > This unfortunately would not catch full paths on certain platforms.\n> >\n> > See https://github.com/git/git/runs/1660588243?check_suite_focus=true#step:7:4186\n> > for an example X-<.\n>\n> Hrm, one has to log in to view those CI logs?  Really?! :(\n>\n> Anyway, I (apparently falsely) assumed that the output these tests\n> look at come from Git itself, and therefore we can rely on difftool's\n> temporary directories being normalized UNIX-style absolute paths...\n> But it seems they don't actually come from Git but from 'ls', because\n> that's what those '--extcmd ls' options do, and I now going to assume\n> that 'ls' prints those absolute paths with drive letter prefixes and\n> whatnot on Windows.\n>\n> The initial version of this patch just tightened all potentially\n> problematic 'grep' patterns, e.g. 'grep ^sub$ output && grep ^file$\n> output'.  That should work on Windows as well, shouldn't it.  Will see\n> whether I can dig it out from the reflogs.\n\nThe logs ends thusly:\n\n-- snipsnap --\n ++++ diff -u expect actual\n--- expect\t2021-01-07 05:12:54.687742100 +0000\n+++ actual\t2021-01-07 05:12:55.370745600 +0000\n@@ -1,6 +1,8 @@\n+C:\\Users\\RUNNER~1\\AppData\\Local\\Temp/git-difftool.a01364/left/:\n file\n file2\n\n+C:\\Users\\RUNNER~1\\AppData\\Local\\Temp/git-difftool.a01364/right/:\n file\n file2\n sub\nerror: last command exited with $?=1\nnot ok 42 - difftool -d --no-symlinks\n"},{"id":"413888","messageId":"xmqq1revi413.fsf@gitster.c.googlers.com","threadId":"54879","inReplyTo":"20210108092036.GR8396@szeder.dev","subject":"Re: [PATCH] t7800-difftool: don't accidentally match tmp dirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-08T20:10:00Z","receivedAt":"2021-01-08T20:11:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> On Wed, Jan 06, 2021 at 10:24:27PM -0800, Junio C Hamano wrote:\n>> SZEDER Gábor <szeder.dev@gmail.com> writes:\n>> \n>> > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n>> > index a578b35761..fe02fe1688 100755\n>> > --- a/t/t7800-difftool.sh\n>> > +++ b/t/t7800-difftool.sh\n>> > @@ -439,73 +439,104 @@ run_dir_diff_test () {\n>> >  }\n>> >  \n>> >  run_dir_diff_test 'difftool -d' '\n>> > +\tcat >expect <<-\\EOF &&\n>> > +\tfile\n>> > +\tfile2\n>> > +\n>> > +\tfile\n>> > +\tfile2\n>> > +\tsub\n>> > +\tEOF\n>> >  \tgit difftool -d $symlinks --extcmd ls branch >output &&\n>> > -\tgrep sub output &&\n>> > -\tgrep file output\n>> > +\tgrep -v ^/ output >actual &&\n>> \n>> This unfortunately would not catch full paths on certain platforms.\n>> \n>> See https://github.com/git/git/runs/1660588243?check_suite_focus=true#step:7:4186\n>> for an example X-<.\n>\n> Hrm, one has to log in to view those CI logs?  Really?! :(\n\nYup, it sucks.  I am curious (but not strongly interested enough to\ndemand) to learn the reason why from GitHub folks.\n\n> Anyway, I (apparently falsely) assumed that the output these tests\n> look at come from Git itself, and therefore we can rely on difftool's\n> temporary directories being normalized UNIX-style absolute paths...\n> But it seems they don't actually come from Git but from 'ls', because\n> that's what those '--extcmd ls' options do, and I now going to assume\n> that 'ls' prints those absolute paths with drive letter prefixes and\n> whatnot on Windows.\n>\n> The initial version of this patch just tightened all potentially\n> problematic 'grep' patterns, e.g. 'grep ^sub$ output && grep ^file$\n> output'.  That should work on Windows as well, shouldn't it.  Will see\n> whether I can dig it out from the reflogs.\n\nI only see 'file', 'file2', 'b', 'c', etc. used in the tests; do we\never use any path that ends with a colon?\n\nI wonder if would it be more robust to use something like\n\n\tsed -e 's|^.*/\\([a-z]*\\)/:$|/directory-path-\\1/:|'\n\nin place of \"grep -v /\" to redact the temporary directory names\ndifftool creates.  It would give you /directory-path-left/: or\n/directory-path-right/: instead of removing these lines and it would\nclarify what these two series of file names are, which may be an\nadded bonus.\n"},{"id":"413917","messageId":"20210109170513.31084-1-szeder.dev@gmail.com","threadId":"54879","inReplyTo":"20210108092036.GR8396@szeder.dev","subject":"[PATCH v2] t7800-difftool: don't accidentally match tmp dirs","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-01-09T17:05:13Z","receivedAt":"2021-01-09T17:06:05Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"In a bunch of test cases in 't7800-difftool.sh' we 'grep' for specific\nfilenames in 'git difftool's output, and those test cases are prone to\noccasional failures because those filenames might be part of the name\nof difftool's temporary directory as well, e.g.:\n\n  +git difftool --dir-diff --no-symlinks --extcmd ls v1\n  +grep sub output\n  +test_line_count = 2 sub-output\n  test_line_count: line count for sub-output != 2\n  /tmp/git-difftool.Ssubfq/left/:\n  sub\n  /tmp/git-difftool.Ssubfq/right/:\n  sub\n  error: last command exited with $?=1\n  not ok 50 - difftool --dir-diff v1 from subdirectory --no-symlinks\n\nFix this by tightening the 'grep' patterns looking for those\ninteresting filenames to match only lines where a filename stands on\nits own.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t7800-difftool.sh | 38 +++++++++++++++++++-------------------\n 1 file changed, 19 insertions(+), 19 deletions(-)\n\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex a578b35761..32291fb67b 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -440,20 +440,20 @@ run_dir_diff_test () {\n \n run_dir_diff_test 'difftool -d' '\n \tgit difftool -d $symlinks --extcmd ls branch >output &&\n-\tgrep sub output &&\n-\tgrep file output\n+\tgrep \"^sub$\" output &&\n+\tgrep \"^file$\" output\n '\n \n run_dir_diff_test 'difftool --dir-diff' '\n \tgit difftool --dir-diff $symlinks --extcmd ls branch >output &&\n-\tgrep sub output &&\n-\tgrep file output\n+\tgrep \"^sub$\" output &&\n+\tgrep \"^file$\" output\n '\n \n run_dir_diff_test 'difftool --dir-diff ignores --prompt' '\n \tgit difftool --dir-diff $symlinks --prompt --extcmd ls branch >output &&\n-\tgrep sub output &&\n-\tgrep file output\n+\tgrep \"^sub$\" output &&\n+\tgrep \"^file$\" output\n '\n \n run_dir_diff_test 'difftool --dir-diff branch from subdirectory' '\n@@ -462,11 +462,11 @@ 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\tgrep sub output >sub-output &&\n+\t\tgrep \"^sub$\" output >sub-output &&\n \t\ttest_line_count = 1 sub-output &&\n-\t\tgrep file\"$\" output >file-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\tgrep \"^file2$\" output >file2-output &&\n \t\ttest_line_count = 2 file2-output\n \t)\n '\n@@ -477,11 +477,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\tgrep sub output >sub-output &&\n+\t\tgrep \"^sub$\" output >sub-output &&\n \t\ttest_line_count = 2 sub-output &&\n-\t\tgrep file output >file-output &&\n+\t\tgrep \"^file$\" output >file-output &&\n \t\ttest_line_count = 2 file-output &&\n-\t\t! grep file2 output\n+\t\t! grep \"^file2$\" output\n \t)\n '\n \n@@ -491,9 +491,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\tgrep sub output >sub-output &&\n+\t\tgrep \"^sub$\" output >sub-output &&\n \t\ttest_line_count = 1 sub-output &&\n-\t\t! grep file output\n+\t\t! grep \"^file$\" output\n \t)\n '\n \n@@ -503,9 +503,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\tgrep sub output >sub-output &&\n+\t\tgrep \"^sub$\" output >sub-output &&\n \t\ttest_line_count = 2 sub-output &&\n-\t\t! grep file output\n+\t\t! grep \"^file$\" output\n \t)\n '\n \n@@ -518,8 +518,8 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory with GIT_DIR set' '\n \t\tcd sub &&\n \t\tgit difftool --dir-diff $symlinks --extcmd ls \\\n \t\t\tbranch -- sub >output &&\n-\t\tgrep sub output &&\n-\t\t! grep file output\n+\t\tgrep \"^sub$\" output &&\n+\t\t! grep \"^file$\" output\n \t)\n '\n \n@@ -527,7 +527,7 @@ run_dir_diff_test 'difftool --dir-diff when worktree file is missing' '\n \ttest_when_finished git reset --hard &&\n \trm file2 &&\n \tgit difftool --dir-diff $symlinks --extcmd ls branch master >output &&\n-\tgrep file2 output\n+\tgrep \"^file2$\" output\n '\n \n run_dir_diff_test 'difftool --dir-diff with unmerged files' '\n-- \n2.30.0.337.g235c185d39\n\n"},{"id":"413926","messageId":"xmqqft39g56u.fsf@gitster.c.googlers.com","threadId":"54879","inReplyTo":"20210109170513.31084-1-szeder.dev@gmail.com","subject":"Re: [PATCH v2] t7800-difftool: don't accidentally match tmp dirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-09T21:40:09Z","receivedAt":"2021-01-09T21:41:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> In a bunch of test cases in 't7800-difftool.sh' we 'grep' for specific\n> filenames in 'git difftool's output, and those test cases are prone to\n> occasional failures because those filenames might be part of the name\n> of difftool's temporary directory as well, e.g.:\n>\n>   +git difftool --dir-diff --no-symlinks --extcmd ls v1\n>   +grep sub output\n>   +test_line_count = 2 sub-output\n>   test_line_count: line count for sub-output != 2\n>   /tmp/git-difftool.Ssubfq/left/:\n>   sub\n>   /tmp/git-difftool.Ssubfq/right/:\n>   sub\n>   error: last command exited with $?=1\n>   not ok 50 - difftool --dir-diff v1 from subdirectory --no-symlinks\n>\n> Fix this by tightening the 'grep' patterns looking for those\n> interesting filenames to match only lines where a filename stands on\n> its own.\n\nOK.  I thought that your previous \"we know not just we want to see\nsub and file anywhere in the output, but we want them to appear in\nthis order\" attempt was nicer, especially when dealing with the\ndirectory diff output.  That was why I suggested to \"normalize\" the\nlines that report the directory path with a sed script, instead of\nremoving them with \"grep -v\", and change the condition to tell if a\nline is a directory from \"^/\" to \"/:$\".\n\nBut the approach taken with this round is much simpler.  When we\nonly care about 'sub' and 'file' in the output, for example, we used\nto just grep for these strings, allowing substring matches.  Now we\nmake sure the lines we consider matches contain these tokens we want\nto see and nothing else.  It focuses on only fixing the bug, without\nattempting to \"improve\" the tests while at it.  And it is certainly\nsimpler to explain the change between the version before this patch\nand the version with this patch.\n\nThanks.\n\n\n\n> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n> ---\n>  t/t7800-difftool.sh | 38 +++++++++++++++++++-------------------\n>  1 file changed, 19 insertions(+), 19 deletions(-)\n>\n> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> index a578b35761..32291fb67b 100755\n> --- a/t/t7800-difftool.sh\n> +++ b/t/t7800-difftool.sh\n> @@ -440,20 +440,20 @@ run_dir_diff_test () {\n>  \n>  run_dir_diff_test 'difftool -d' '\n>  \tgit difftool -d $symlinks --extcmd ls branch >output &&\n> -\tgrep sub output &&\n> -\tgrep file output\n> +\tgrep \"^sub$\" output &&\n> +\tgrep \"^file$\" output\n>  '\n>  \n>  run_dir_diff_test 'difftool --dir-diff' '\n>  \tgit difftool --dir-diff $symlinks --extcmd ls branch >output &&\n> -\tgrep sub output &&\n> -\tgrep file output\n> +\tgrep \"^sub$\" output &&\n> +\tgrep \"^file$\" output\n>  '\n>  \n>  run_dir_diff_test 'difftool --dir-diff ignores --prompt' '\n>  \tgit difftool --dir-diff $symlinks --prompt --extcmd ls branch >output &&\n> -\tgrep sub output &&\n> -\tgrep file output\n> +\tgrep \"^sub$\" output &&\n> +\tgrep \"^file$\" output\n>  '\n>  \n>  run_dir_diff_test 'difftool --dir-diff branch from subdirectory' '\n> @@ -462,11 +462,11 @@ 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\tgrep sub output >sub-output &&\n> +\t\tgrep \"^sub$\" output >sub-output &&\n>  \t\ttest_line_count = 1 sub-output &&\n> -\t\tgrep file\"$\" output >file-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\tgrep \"^file2$\" output >file2-output &&\n>  \t\ttest_line_count = 2 file2-output\n>  \t)\n>  '\n> @@ -477,11 +477,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\tgrep sub output >sub-output &&\n> +\t\tgrep \"^sub$\" output >sub-output &&\n>  \t\ttest_line_count = 2 sub-output &&\n> -\t\tgrep file output >file-output &&\n> +\t\tgrep \"^file$\" output >file-output &&\n>  \t\ttest_line_count = 2 file-output &&\n> -\t\t! grep file2 output\n> +\t\t! grep \"^file2$\" output\n>  \t)\n>  '\n>  \n> @@ -491,9 +491,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\tgrep sub output >sub-output &&\n> +\t\tgrep \"^sub$\" output >sub-output &&\n>  \t\ttest_line_count = 1 sub-output &&\n> -\t\t! grep file output\n> +\t\t! grep \"^file$\" output\n>  \t)\n>  '\n>  \n> @@ -503,9 +503,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\tgrep sub output >sub-output &&\n> +\t\tgrep \"^sub$\" output >sub-output &&\n>  \t\ttest_line_count = 2 sub-output &&\n> -\t\t! grep file output\n> +\t\t! grep \"^file$\" output\n>  \t)\n>  '\n>  \n> @@ -518,8 +518,8 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory with GIT_DIR set' '\n>  \t\tcd sub &&\n>  \t\tgit difftool --dir-diff $symlinks --extcmd ls \\\n>  \t\t\tbranch -- sub >output &&\n> -\t\tgrep sub output &&\n> -\t\t! grep file output\n> +\t\tgrep \"^sub$\" output &&\n> +\t\t! grep \"^file$\" output\n>  \t)\n>  '\n>  \n> @@ -527,7 +527,7 @@ run_dir_diff_test 'difftool --dir-diff when worktree file is missing' '\n>  \ttest_when_finished git reset --hard &&\n>  \trm file2 &&\n>  \tgit difftool --dir-diff $symlinks --extcmd ls branch master >output &&\n> -\tgrep file2 output\n> +\tgrep \"^file2$\" output\n>  '\n>  \n>  run_dir_diff_test 'difftool --dir-diff with unmerged files' '\n"}]}