{"thread":{"id":"49189","subject":"[PATCH 1/2] t0020-crlf: check the right file","startedAt":"2018-08-22T12:44:47Z","lastAt":"2018-08-22T16:30:03Z","messageCount":4,"participants":["SZEDER Gábor","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"356244","messageId":"20180822124437.11242-1-szeder.dev@gmail.com","threadId":"49189","inReplyTo":null,"subject":"[PATCH 1/2] t0020-crlf: check the right file","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-08-22T12:44:36Z","receivedAt":"2018-08-22T12:44:47Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"In the test 'checkout with autocrlf=input' in 't0020-crlf.sh', one of\nthe 'has_cr' checks looks at the non-existing file 'two' instead of\n'dir/two'.  The test still succeeds, without actually checking what it\nwas supposed to, because this check is expected to fail anyway.\n\nAs a minimal fix, fix the name of the file to be checked.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t0020-crlf.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\nindex 5f056982a5..854da0ae16 100755\n--- a/t/t0020-crlf.sh\n+++ b/t/t0020-crlf.sh\n@@ -160,7 +160,7 @@ test_expect_success 'checkout with autocrlf=input' '\n \tgit config core.autocrlf input &&\n \tgit read-tree --reset -u HEAD &&\n \ttest_must_fail has_cr one &&\n-\ttest_must_fail has_cr two &&\n+\ttest_must_fail has_cr dir/two &&\n \tgit update-index -- one dir/two &&\n \ttest \"$one\" = $(git hash-object --stdin <one) &&\n \ttest \"$two\" = $(git hash-object --stdin <dir/two) &&\n-- \n2.19.0.rc0.136.gd2dd172e64\n\n"},{"id":"356245","messageId":"20180822124437.11242-2-szeder.dev@gmail.com","threadId":"49189","inReplyTo":"20180822124437.11242-1-szeder.dev@gmail.com","subject":"[PATCH 2/2] t4051-diff-function-context: read the right file","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-08-22T12:44:37Z","receivedAt":"2018-08-22T12:44:53Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The test ' context does not include preceding empty lines' in the\nblock of tests 'change with long common tail and no context' in\n't4051-diff-function-context.sh' tries to read the file\n'long_common_tail.diff.diff', but that file doesn't exist as its name\ncontains one more '.diff' suffixes than necessary.\n\nDespite this error the test still succeeded without checking what it's\nsupposed to, because this erroneous read is done on the line:\n\n  test \"$(first_context_line <long_common_tail.diff.diff)\" != \" \"\n\nwhich means that:\n\n  - the command substitution hides the error, so it won't fail the\n    test, and\n\n  - the result of the command substitution is the empty string, which\n    is, of course, not equal to a single space character, so the\n    condition is fulfilled, and the test succeeds.\n\nAs a minimal fix, fix the name of the file to be read.\n\nIn the future we might want to reorganize this test script (1) to use\n'test_cmp' instead of 'test's and command substitutions to catch\nfailing commands and to provide helpful error messages, and (2) to\nspecify what the expected result actually _is_ instead of what it\nisn't.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t4051-diff-function-context.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh\nindex 2d76a971c4..4838a1df8b 100755\n--- a/t/t4051-diff-function-context.sh\n+++ b/t/t4051-diff-function-context.sh\n@@ -174,7 +174,7 @@ test_expect_success ' context does not include other functions' '\n '\n \n test_expect_success ' context does not include preceding empty lines' '\n-\ttest \"$(first_context_line <long_common_tail.diff.diff)\" != \" \"\n+\ttest \"$(first_context_line <long_common_tail.diff)\" != \" \"\n '\n \n check_diff changed_hello_appended 'changed function plus appended function'\n-- \n2.19.0.rc0.136.gd2dd172e64\n\n"},{"id":"356275","messageId":"xmqqo9du5qlf.fsf@gitster-ct.c.googlers.com","threadId":"49189","inReplyTo":"20180822124437.11242-1-szeder.dev@gmail.com","subject":"Re: [PATCH 1/2] t0020-crlf: check the right file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-22T16:11:08Z","receivedAt":"2018-08-22T16:11:13Z","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 the test 'checkout with autocrlf=input' in 't0020-crlf.sh', one of\n> the 'has_cr' checks looks at the non-existing file 'two' instead of\n> 'dir/two'.  The test still succeeds, without actually checking what it\n> was supposed to, because this check is expected to fail anyway.\n>\n> As a minimal fix, fix the name of the file to be checked.\n>\n> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n> ---\n\nThis originates from fd777141 (\"t0020: fix ignored exit code inside\nloops\", 2015-03-25) where a loop\n\n    for f in one dir/two\n    do\n            do things on \"$f\" || break\n    done\n\nwas unrolled to correctly break out of the &&-chain.  The filenames\non the update-index line correctly copied one and dir/two, but the\nhas_cr line somehow lost dir/ prefix.\n\nThanks.  Will queue.\n\n>  t/t0020-crlf.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\n> index 5f056982a5..854da0ae16 100755\n> --- a/t/t0020-crlf.sh\n> +++ b/t/t0020-crlf.sh\n> @@ -160,7 +160,7 @@ test_expect_success 'checkout with autocrlf=input' '\n>  \tgit config core.autocrlf input &&\n>  \tgit read-tree --reset -u HEAD &&\n>  \ttest_must_fail has_cr one &&\n> -\ttest_must_fail has_cr two &&\n> +\ttest_must_fail has_cr dir/two &&\n>  \tgit update-index -- one dir/two &&\n>  \ttest \"$one\" = $(git hash-object --stdin <one) &&\n>  \ttest \"$two\" = $(git hash-object --stdin <dir/two) &&\n"},{"id":"356279","messageId":"20180822163000.GB11904@sigill.intra.peff.net","threadId":"49189","inReplyTo":"xmqqo9du5qlf.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/2] t0020-crlf: check the right file","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-22T16:30:00Z","receivedAt":"2018-08-22T16:30:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 22, 2018 at 09:11:08AM -0700, Junio C Hamano wrote:\n\n> This originates from fd777141 (\"t0020: fix ignored exit code inside\n> loops\", 2015-03-25) where a loop\n> \n>     for f in one dir/two\n>     do\n>             do things on \"$f\" || break\n>     done\n> \n> was unrolled to correctly break out of the &&-chain.  The filenames\n> on the update-index line correctly copied one and dir/two, but the\n> has_cr line somehow lost dir/ prefix.\n> \n> Thanks.  Will queue.\n\nWhoops, my error. Thanks for catching it.\n\n> > diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\n> > index 5f056982a5..854da0ae16 100755\n> > --- a/t/t0020-crlf.sh\n> > +++ b/t/t0020-crlf.sh\n> > @@ -160,7 +160,7 @@ test_expect_success 'checkout with autocrlf=input' '\n> >  \tgit config core.autocrlf input &&\n> >  \tgit read-tree --reset -u HEAD &&\n> >  \ttest_must_fail has_cr one &&\n> > -\ttest_must_fail has_cr two &&\n> > +\ttest_must_fail has_cr dir/two &&\n\nI think I'm also responsible for these mis-uses of test_must_fail, which\nprobably ought to be \"!\".\n\n-Peff\n"}]}