Re: [PATCHv2 4/7] t7008: demonstrate behavior of grep with textconv
Michael J Gruber writes: >>> +test_expect_failure 'grep does not honor textconv' ' >>> + echo "a:binaryQfile" >expect && >>> + git grep Qfile >actual && >> >> This should pass --textconv to "git grep". > > But "git grep" does not know that option yet, so the test would fail for > the wrong reason. > > The point ist that I expect "git grep" to apply textconv filters by > default, which it does not. (I know I might be the only one with this > expectation.) > > Or do we want to document the absence of that option? First, whether you write expect_failure or expec_success, please label the test to say what is expected to happen in the ideal world. The test in question says "grep does not honor textconv", but if you want it to honor textconv in the ideal world, it should be "grep honors textconv (when it should)". Now, from the point of view of testing "git grep honors textconv" missing support at the command line parser level and a buggy implementation of the command line parser that accepts but does not trigger the feature are the same thing. The command would not honor textconv either way. Marking the above as "failure" without explicitly asking for the feature with "--textconv" means we want it to use textconv by default, but that is *not* what the test title says is testing. In your patch, what the body of the text is really expecting is "grep uses textconv by default". If that is what it tests, then passing --textconv from the command line as I suggested would be wrong, but I was going by the title of the patch. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 4/7] t7008: demonstrate behavior of grep with textconv
Junio C Hamano venit, vidit, dixit 23.04.2013 17:16: > Michael J Gruber writes: > >> Currently, "git grep" does not honor any textconv filters. Demonstrate >> this in the tests. >> >> Signed-off-by: Michael J Gruber >> --- >> t/t7008-grep-binary.sh | 23 +++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh >> index 26f8319..126fe4c 100755 >> --- a/t/t7008-grep-binary.sh >> +++ b/t/t7008-grep-binary.sh >> @@ -145,4 +145,27 @@ test_expect_success 'grep respects not-binary diff >> attribute' ' >> test_cmp expect actual >> ' >> >> +cat >nul_to_q_textconv <<'EOF' >> +#!/bin/sh >> +"$PERL_PATH" -pe 'y/\000/Q/' < "$1" >> +EOF >> +chmod +x nul_to_q_textconv >> + >> +test_expect_success 'setup textconv filters' ' >> +echo a diff=foo >.gitattributes && >> +git config diff.foo.textconv "\"$(pwd)\""/nul_to_q_textconv >> +' >> + >> +test_expect_failure 'grep does not honor textconv' ' >> +echo "a:binaryQfile" >expect && >> +git grep Qfile >actual && > > This should pass --textconv to "git grep". But "git grep" does not know that option yet, so the test would fail for the wrong reason. The point ist that I expect "git grep" to apply textconv filters by default, which it does not. (I know I might be the only one with this expectation.) Or do we want to document the absence of that option? >> +test_cmp expect actual >> +' >> + >> +test_expect_failure 'grep blob does not honor textconv' ' >> +echo "HEAD:a:binaryQfile" >expect && >> +git grep Qfile HEAD:a >actual && > > Likewise. > >> +test_cmp expect actual >> +' >> + >> test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 4/7] t7008: demonstrate behavior of grep with textconv
Michael J Gruber writes: > Currently, "git grep" does not honor any textconv filters. Demonstrate > this in the tests. > > Signed-off-by: Michael J Gruber > --- > t/t7008-grep-binary.sh | 23 +++ > 1 file changed, 23 insertions(+) > > diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh > index 26f8319..126fe4c 100755 > --- a/t/t7008-grep-binary.sh > +++ b/t/t7008-grep-binary.sh > @@ -145,4 +145,27 @@ test_expect_success 'grep respects not-binary diff > attribute' ' > test_cmp expect actual > ' > > +cat >nul_to_q_textconv <<'EOF' > +#!/bin/sh > +"$PERL_PATH" -pe 'y/\000/Q/' < "$1" > +EOF > +chmod +x nul_to_q_textconv > + > +test_expect_success 'setup textconv filters' ' > + echo a diff=foo >.gitattributes && > + git config diff.foo.textconv "\"$(pwd)\""/nul_to_q_textconv > +' > + > +test_expect_failure 'grep does not honor textconv' ' > + echo "a:binaryQfile" >expect && > + git grep Qfile >actual && This should pass --textconv to "git grep". > + test_cmp expect actual > +' > + > +test_expect_failure 'grep blob does not honor textconv' ' > + echo "HEAD:a:binaryQfile" >expect && > + git grep Qfile HEAD:a >actual && Likewise. > + test_cmp expect actual > +' > + > test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 4/7] t7008: demonstrate behavior of grep with textconv
Currently, "git grep" does not honor any textconv filters. Demonstrate this in the tests. Signed-off-by: Michael J Gruber --- t/t7008-grep-binary.sh | 23 +++ 1 file changed, 23 insertions(+) diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh index 26f8319..126fe4c 100755 --- a/t/t7008-grep-binary.sh +++ b/t/t7008-grep-binary.sh @@ -145,4 +145,27 @@ test_expect_success 'grep respects not-binary diff attribute' ' test_cmp expect actual ' +cat >nul_to_q_textconv <<'EOF' +#!/bin/sh +"$PERL_PATH" -pe 'y/\000/Q/' < "$1" +EOF +chmod +x nul_to_q_textconv + +test_expect_success 'setup textconv filters' ' + echo a diff=foo >.gitattributes && + git config diff.foo.textconv "\"$(pwd)\""/nul_to_q_textconv +' + +test_expect_failure 'grep does not honor textconv' ' + echo "a:binaryQfile" >expect && + git grep Qfile >actual && + test_cmp expect actual +' + +test_expect_failure 'grep blob does not honor textconv' ' + echo "HEAD:a:binaryQfile" >expect && + git grep Qfile HEAD:a >actual && + test_cmp expect actual +' + test_done -- 1.8.2.1.799.g1ac2534 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
