From 5b5e45949bb310fa8972f3e55b9cb05ce8047f37 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 21 May 2011 14:11:33 -0500 Subject: t4018 (funcname patterns): make .gitattributes state easier to track Most, but not all, tests in this script rely on attributes declaring that files with a .java extension should use the "java" driver: *.java diff=java Split out a "set up" test to put such a .gitattributes in place after the tests that do not want it have run, to make it more likely that individual tests other than this setup test can be safely modified, rearranged, or skipped. Presumably this setup code will learn to request other drivers for other extensions in the same place when the test suite learns to exercise other diff drivers. Similarly, make sure that early test assertions that do not use these default attributes set up .gitattributes appropriately for themselves, so tests that run before can be modified with less risk of breaking something. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index 3646930623..24eb1a35d6 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -36,11 +36,12 @@ builtin_patterns="bibtex cpp csharp fortran html java objc pascal perl php pytho for p in $builtin_patterns do test_expect_success "builtin $p pattern compiles" ' - echo "*.java diff=$p" > .gitattributes && + echo "*.java diff=$p" >.gitattributes && ! { git diff --no-index Beer.java Beer-correct.java 2>&1 | grep "fatal" > /dev/null; } ' test_expect_success "builtin $p wordRegex pattern compiles" ' + echo "*.java diff=$p" >.gitattributes && ! { git diff --no-index --word-diff \ Beer.java Beer-correct.java 2>&1 | grep "fatal" > /dev/null; } @@ -53,8 +54,11 @@ test_expect_success 'default behaviour' ' grep "^@@.*@@ public class Beer" ' +test_expect_success 'set up .gitattributes declaring drivers to test' ' + echo "*.java diff=java" >.gitattributes +' + test_expect_success 'preset java pattern' ' - echo "*.java diff=java" >.gitattributes && git diff --no-index Beer.java Beer-correct.java | grep "^@@.*@@ public static void main(" ' -- cgit v1.2.1 From f792a0b88ec24dd20c29282b4e022c7b48abd59b Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 21 May 2011 14:22:28 -0500 Subject: t4018 (funcname patterns): make configuration easier to track Introduce a "test_config" function to set a configuration variable for use by a single test (automatically unsetting it when the assertion finishes). If this function is used consistently, the configuration used in a test_expect_success block can be read at the beginning of that block instead of requiring reading all the tests that come before. So it becomes a little easier to add new tests or rearrange existing ones without fear of breaking configuration. In particular, the test of alternation in xfuncname patterns also checks that xfuncname takes precedence over funcname variable as a sort of side-effect, since the latter leaks in from previous tests. In the new syntax, the test has to say explicitly what variables it is using, making the test clearer and a future regression in coverage from carelessly editing the script less likely. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 't') diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index 24eb1a35d6..ce0a0e32e1 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -32,6 +32,11 @@ EOF sed 's/beer\\/beer,\\/' < Beer.java > Beer-correct.java +test_config () { + git config "$1" "$2" && + test_when_finished "git config --unset $1" +} + builtin_patterns="bibtex cpp csharp fortran html java objc pascal perl php python ruby tex" for p in $builtin_patterns do @@ -63,29 +68,29 @@ test_expect_success 'preset java pattern' ' grep "^@@.*@@ public static void main(" ' -git config diff.java.funcname '!static -!String -[^ ].*s.*' - test_expect_success 'custom pattern' ' + test_config diff.java.funcname "!static +!String +[^ ].*s.*" && git diff --no-index Beer.java Beer-correct.java | grep "^@@.*@@ int special;$" ' test_expect_success 'last regexp must not be negated' ' - git config diff.java.funcname "!static" && + test_config diff.java.funcname "!static" && git diff --no-index Beer.java Beer-correct.java 2>&1 | grep "fatal: Last expression must not be negated:" ' test_expect_success 'pattern which matches to end of line' ' - git config diff.java.funcname "Beer$" && + test_config diff.java.funcname "Beer$" && git diff --no-index Beer.java Beer-correct.java | grep "^@@.*@@ Beer" ' test_expect_success 'alternation in pattern' ' - git config diff.java.xfuncname "^[ ]*((public|static).*)$" && + test_config diff.java.funcname "Beer$" && + test_config diff.java.xfuncname "^[ ]*((public|static).*)$" && git diff --no-index Beer.java Beer-correct.java | grep "^@@.*@@ public static void main(" ' -- cgit v1.2.1 From d64d6cdc2071d1eb7f6a45118edc42627e6fc692 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 21 May 2011 14:25:14 -0500 Subject: t4018 (funcname patterns): minor cleanups Introduce a test_expect_funcname function to make a diff and apply a regexp anchored on the left to the function name it writes, avoiding some repetition. Omit the space after >, <<, and < operators for consistency with other scripts. Quote the < Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 49 ++++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 24 deletions(-) (limited to 't') diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index ce0a0e32e1..ad74c605a4 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -9,8 +9,7 @@ test_description='Test custom diff function name patterns' LF=' ' - -cat > Beer.java << EOF +cat >Beer.java <<\EOF public class Beer { int special; @@ -29,34 +28,40 @@ public class Beer } } EOF - -sed 's/beer\\/beer,\\/' < Beer.java > Beer-correct.java +sed 's/beer\\/beer,\\/' Beer-correct.java test_config () { git config "$1" "$2" && test_when_finished "git config --unset $1" } -builtin_patterns="bibtex cpp csharp fortran html java objc pascal perl php python ruby tex" -for p in $builtin_patterns +test_expect_funcname () { + test_expect_code 1 git diff --no-index \ + Beer.java Beer-correct.java >diff && + grep "^@@.*@@ $1" diff +} + +for p in bibtex cpp csharp fortran html java objc pascal perl php python ruby tex do test_expect_success "builtin $p pattern compiles" ' echo "*.java diff=$p" >.gitattributes && - ! { git diff --no-index Beer.java Beer-correct.java 2>&1 | - grep "fatal" > /dev/null; } + test_expect_code 1 git diff --no-index \ + Beer.java Beer-correct.java 2>msg && + ! grep fatal msg && + ! grep error msg ' test_expect_success "builtin $p wordRegex pattern compiles" ' echo "*.java diff=$p" >.gitattributes && - ! { git diff --no-index --word-diff \ - Beer.java Beer-correct.java 2>&1 | - grep "fatal" > /dev/null; } + test_expect_code 1 git diff --no-index --word-diff \ + Beer.java Beer-correct.java 2>msg && + ! grep fatal msg && + ! grep error msg ' done test_expect_success 'default behaviour' ' rm -f .gitattributes && - git diff --no-index Beer.java Beer-correct.java | - grep "^@@.*@@ public class Beer" + test_expect_funcname "public class Beer\$" ' test_expect_success 'set up .gitattributes declaring drivers to test' ' @@ -64,35 +69,31 @@ test_expect_success 'set up .gitattributes declaring drivers to test' ' ' test_expect_success 'preset java pattern' ' - git diff --no-index Beer.java Beer-correct.java | - grep "^@@.*@@ public static void main(" + test_expect_funcname "public static void main(" ' test_expect_success 'custom pattern' ' test_config diff.java.funcname "!static !String [^ ].*s.*" && - git diff --no-index Beer.java Beer-correct.java | - grep "^@@.*@@ int special;$" + test_expect_funcname "int special;\$" ' test_expect_success 'last regexp must not be negated' ' test_config diff.java.funcname "!static" && - git diff --no-index Beer.java Beer-correct.java 2>&1 | - grep "fatal: Last expression must not be negated:" + test_expect_code 128 git diff --no-index Beer.java Beer-correct.java 2>msg && + grep ": Last expression must not be negated:" msg ' test_expect_success 'pattern which matches to end of line' ' - test_config diff.java.funcname "Beer$" && - git diff --no-index Beer.java Beer-correct.java | - grep "^@@.*@@ Beer" + test_config diff.java.funcname "Beer\$" && + test_expect_funcname "Beer\$" ' test_expect_success 'alternation in pattern' ' test_config diff.java.funcname "Beer$" && test_config diff.java.xfuncname "^[ ]*((public|static).*)$" && - git diff --no-index Beer.java Beer-correct.java | - grep "^@@.*@@ public static void main(" + test_expect_funcname "public static void main(" ' test_done -- cgit v1.2.1 From f12c66b9bb851aa7350d40370e6adf78535c5930 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 21 May 2011 14:29:01 -0500 Subject: userdiff/perl: anchor "sub" and "package" patterns on the left MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The userdiff funcname mechanism has no concept of nested scopes --- instead, "git diff" and "git grep --show-function" simply label the diff header with the most recent matching line. Unfortunately that means text following a subroutine in a POD section: =head1 DESCRIPTION You might use this facility like so: sub example { foo; } Now, having said that, let's say more about the facility. Blah blah blah ... etc etc. gets the subroutine name instead of the POD header in its diff/grep funcname header, making it harder to get oriented when reading a diff without enough context. The fix is simple: anchor the funcname syntax to the left margin so nested subroutines and packages like this won't get picked up. (The builtin C++ funcname pattern already does the same thing.) This means the userdiff driver will misparse the idiom { my $static; sub foo { ... use $static ... } } but I think that's worth it; we can revisit this later if the userdiff mechanism learns to keep track of the beginning and end of nested scopes. Reported-by: Ævar Arnfjörð Bjarmason Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 59 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 3 deletions(-) (limited to 't') diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index ad74c605a4..f071a8fdd1 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -29,6 +29,47 @@ public class Beer } EOF sed 's/beer\\/beer,\\/' Beer-correct.java +cat >Beer.perl <<\EOF +package Beer; + +use strict; +use warnings; +use parent qw(Exporter); +our @EXPORT_OK = qw(round); + +sub round { + my ($n) = @_; + print "$n bottles of beer on the wall "; + print "$n bottles of beer\n"; + print "Take one down, pass it around, "; + $n = $n - 1; + print "$n bottles of beer on the wall.\n"; +} + +__END__ + +=head1 NAME + +Beer - subroutine to output fragment of a drinking song + +=head1 SYNOPSIS + + use Beer qw(round); + + sub song { + for (my $i = 99; $i > 0; $i--) { + round $i; + } + } + + song; + +=cut +EOF +sed -e ' + s/beer\\/beer,\\/ + s/song;/song();/ +' Beer-correct.perl test_config () { git config "$1" "$2" && @@ -36,8 +77,9 @@ test_config () { } test_expect_funcname () { - test_expect_code 1 git diff --no-index \ - Beer.java Beer-correct.java >diff && + lang=${2-java} + test_expect_code 1 git diff --no-index -U1 \ + "Beer.$lang" "Beer-correct.$lang" >diff && grep "^@@.*@@ $1" diff } @@ -65,13 +107,24 @@ test_expect_success 'default behaviour' ' ' test_expect_success 'set up .gitattributes declaring drivers to test' ' - echo "*.java diff=java" >.gitattributes + cat >.gitattributes <<-\EOF + *.java diff=java + *.perl diff=perl + EOF ' test_expect_success 'preset java pattern' ' test_expect_funcname "public static void main(" ' +test_expect_success 'preset perl pattern' ' + test_expect_funcname "sub round {\$" perl +' + +test_expect_success 'perl pattern is not distracted by sub within POD' ' + test_expect_funcname "=head" perl +' + test_expect_success 'custom pattern' ' test_config diff.java.funcname "!static !String -- cgit v1.2.1 From 12f0967a8a1e3c11c678de181f77d1c7883b37cf Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 21 May 2011 14:35:51 -0500 Subject: userdiff/perl: match full line of POD headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The builtin perl userdiff driver is not greedy enough about catching POD header lines. Capture the whole line, so instead of just declaring that we are in some "@@ =head1" section, diff/grep output can explain that the enclosing section is about "@@ =head1 OPTIONS". Reported-by: Ævar Arnfjörð Bjarmason Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 4 ++++ 1 file changed, 4 insertions(+) (limited to 't') diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index f071a8fdd1..8a5714912d 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -125,6 +125,10 @@ test_expect_success 'perl pattern is not distracted by sub within POD' ' test_expect_funcname "=head" perl ' +test_expect_success 'perl pattern gets full line of POD header' ' + test_expect_funcname "=head1 SYNOPSIS\$" perl +' + test_expect_success 'custom pattern' ' test_config diff.java.funcname "!static !String -- cgit v1.2.1 From ea2ca4497bdb716977a3e2526780635cb6bac513 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 21 May 2011 14:38:26 -0500 Subject: userdiff/perl: catch sub with brace on second line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accept sub foo { } as an alternative to a more common style that introduces perl functions with a brace on the first line (and likewise for BEGIN/END blocks). The new regex is a little hairy to avoid matching # forward declaration sub foo; while continuing to match "sub foo($;@) {" and sub foo { # This routine is interesting; # in fact, the lines below explain how... While at it, pay attention to Perl 5.14's "package foo {" syntax as an alternative to the traditional "package foo;". Requested-by: Ævar Arnfjörð Bjarmason Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index 8a5714912d..b2fd1a99da 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -35,7 +35,11 @@ package Beer; use strict; use warnings; use parent qw(Exporter); -our @EXPORT_OK = qw(round); +our @EXPORT_OK = qw(round finalround); + +sub other; # forward declaration + +# hello sub round { my ($n) = @_; @@ -46,6 +50,12 @@ sub round { print "$n bottles of beer on the wall.\n"; } +sub finalround +{ + print "Go to the store, buy some more\n"; + print "99 bottles of beer on the wall.\n"); +} + __END__ =head1 NAME @@ -54,12 +64,13 @@ Beer - subroutine to output fragment of a drinking song =head1 SYNOPSIS - use Beer qw(round); + use Beer qw(round finalround); sub song { for (my $i = 99; $i > 0; $i--) { round $i; } + finalround; } song; @@ -67,7 +78,9 @@ Beer - subroutine to output fragment of a drinking song =cut EOF sed -e ' + s/hello/goodbye/ s/beer\\/beer,\\/ + s/more\\/more,\\/ s/song;/song();/ ' Beer-correct.perl @@ -121,6 +134,10 @@ test_expect_success 'preset perl pattern' ' test_expect_funcname "sub round {\$" perl ' +test_expect_success 'perl pattern accepts K&R style brace placement, too' ' + test_expect_funcname "sub finalround\$" perl +' + test_expect_success 'perl pattern is not distracted by sub within POD' ' test_expect_funcname "=head" perl ' @@ -129,6 +146,10 @@ test_expect_success 'perl pattern gets full line of POD header' ' test_expect_funcname "=head1 SYNOPSIS\$" perl ' +test_expect_success 'perl pattern is not distracted by forward declaration' ' + test_expect_funcname "package Beer;\$" perl +' + test_expect_success 'custom pattern' ' test_config diff.java.funcname "!static !String -- cgit v1.2.1 From f5b7ce1b9041d19658b79d8b956d7e79ec6006bd Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 21 May 2011 14:40:32 -0500 Subject: tests: make test_expect_code quieter on success A command exiting with the expected status is not particularly notable. While the indication of progress might be useful when tracking down where in a test a failure has happened, the same applies to most other test helpers, which are quiet about success, so this single helper's output stands out in an unpleasant way. An alternative method for showing progress information might to invent a --progress option that runs tests with "set -x", or until that is available, to run tests using commands like prove -v -j2 --shuffle --exec='sh -x' t2202-add-addremove.sh Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/test-lib.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 't') diff --git a/t/test-lib.sh b/t/test-lib.sh index 8a274fbe79..a174f66961 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -731,12 +731,11 @@ test_expect_code () { exit_code=$? if test $exit_code = $want_code then - echo >&2 "test_expect_code: command exited with $exit_code: $*" return 0 - else - echo >&2 "test_expect_code: command exited with $exit_code, we wanted $want_code $*" - return 1 fi + + echo >&2 "test_expect_code: command exited with $exit_code, we wanted $want_code $*" + return 1 } # test_cmp is a helper function to compare actual and expected output. -- cgit v1.2.1 From f143d9c695cd4c3e86069c536fa0dff04fc93e93 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sun, 22 May 2011 12:29:32 -0500 Subject: userdiff/perl: tighten BEGIN/END block pattern to reject here-doc delimiters A naive method of treating BEGIN/END blocks with a brace on the second line as diff/grep funcname context involves also matching unrelated lines that consist of all-caps letters: sub foo { print <<'EOF' text goes here ... EOF ... rest of foo ... } That's not so great, because it means that "git diff" and "git grep --show-function" would write "=EOF" or "@@ EOF" as context instead of a more useful reminder like "@@ sub foo {". To avoid this, tighten the pattern to only match the special block names that perl accepts (namely BEGIN, END, INIT, CHECK, UNITCHECK, AUTOLOAD, and DESTROY). The list is taken from perl's toke.c. Suggested-by: Jakub Narebski Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index b2fd1a99da..b68c56b68c 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -29,7 +29,7 @@ public class Beer } EOF sed 's/beer\\/beer,\\/' Beer-correct.java -cat >Beer.perl <<\EOF +cat >Beer.perl <<\EOT package Beer; use strict; @@ -56,6 +56,15 @@ sub finalround print "99 bottles of beer on the wall.\n"); } +sub withheredocument { + print <<"EOF" +decoy here-doc +EOF + # some lines of context + # to pad it out + print "hello\n"; +} + __END__ =head1 NAME @@ -76,7 +85,7 @@ Beer - subroutine to output fragment of a drinking song song; =cut -EOF +EOT sed -e ' s/hello/goodbye/ s/beer\\/beer,\\/ @@ -138,6 +147,10 @@ test_expect_success 'perl pattern accepts K&R style brace placement, too' ' test_expect_funcname "sub finalround\$" perl ' +test_expect_success 'but is not distracted by end of <