[PATCH 1/7] contrib/subtree: Clean and refactor test code

2015-11-12 Thread David Greene
From: Techlive Zheng 

Mostly prepare for the later tests refactoring.  This moves some
common code to helper functions and generally cleans things up to be
more presentable.

Signed-off-by: Techlive Zheng 
Signed-off-by: David A. Greene 
---
 contrib/subtree/t/Makefile |  31 ---
 contrib/subtree/t/t7900-subtree.sh | 103 -
 2 files changed, 79 insertions(+), 55 deletions(-)

diff --git a/contrib/subtree/t/Makefile b/contrib/subtree/t/Makefile
index c864810..276898e 100644
--- a/contrib/subtree/t/Makefile
+++ b/contrib/subtree/t/Makefile
@@ -13,11 +13,23 @@ TAR ?= $(TAR)
 RM ?= rm -f
 PROVE ?= prove
 DEFAULT_TEST_TARGET ?= test
+TEST_LINT ?= test-lint
+
+ifdef TEST_OUTPUT_DIRECTORY
+TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results
+else
+TEST_RESULTS_DIRECTORY = ../../../t/test-results
+endif
 
 # Shell quote;
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
+TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY))
 
-T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
+T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
+TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh))
+TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
+THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
 
 all: $(DEFAULT_TEST_TARGET)
 
@@ -26,20 +38,22 @@ test: pre-clean $(TEST_LINT)
 
 prove: pre-clean $(TEST_LINT)
@echo "*** prove ***"; GIT_CONFIG=.git/config $(PROVE) --exec 
'$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
-   $(MAKE) clean
+   $(MAKE) clean-except-prove-cache
 
 $(T):
@echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ 
$(GIT_TEST_OPTS)
 
 pre-clean:
-   $(RM) -r test-results
+   $(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'
 
-clean:
-   $(RM) -r 'trash directory'.* test-results
+clean-except-prove-cache:
+   $(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)'
$(RM) -r valgrind/bin
+
+clean: clean-except-prove-cache
$(RM) .prove
 
-test-lint: test-lint-duplicates test-lint-executable
+test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax
 
 test-lint-duplicates:
@dups=`echo $(T) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
@@ -51,12 +65,15 @@ test-lint-executable:
test -z "$$bad" || { \
echo >&2 "non-executable tests:" $$bad; exit 1; }
 
+test-lint-shell-syntax:
+   @'$(PERL_PATH_SQ)' ../../../t/check-non-portable-shell.pl $(T) 
$(THELPERS)
+
 aggregate-results-and-cleanup: $(T)
$(MAKE) aggregate-results
$(MAKE) clean
 
 aggregate-results:
-   for f in ../../../t/test-results/t*-*.counts; do \
+   for f in '$(TEST_RESULTS_DIRECTORY_SQ)'/t*-*.counts; do \
echo "$$f"; \
done | '$(SHELL_PATH_SQ)' ../../../t/aggregate-results.sh
 
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index dfbe443..f9dda3d 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -5,7 +5,7 @@
 #
 test_description='Basic porcelain support for subtrees
 
-This test verifies the basic operation of the merge, pull, add
+This test verifies the basic operation of the add, pull, merge
 and split subcommands of git subtree.
 '
 
@@ -20,7 +20,6 @@ create()
git add "$1"
 }
 
-
 check_equal()
 {
test_debug 'echo'
@@ -38,6 +37,30 @@ undo()
git reset --hard HEAD~
 }
 
+# Make sure no patch changes more than one file.
+# The original set of commits changed only one file each.
+# A multi-file change would imply that we pruned commits
+# too aggressively.
+join_commits()
+{
+   commit=
+   all=
+   while read x y; do
+   if [ -z "$x" ]; then
+   continue
+   elif [ "$x" = "commit:" ]; then
+   if [ -n "$commit" ]; then
+   echo "$commit $all"
+   all=
+   fi
+   commit="$y"
+   else
+   all="$all $y"
+   fi
+   done
+   echo "$commit $all"
+}
+
 last_commit_message()
 {
git log --pretty=format:%s -1
@@ -123,9 +146,11 @@ test_expect_success 'add subproj to mainline' '
check_equal ''"$(last_commit_message)"'' "Add '"'sub dir/'"' from 
commit '"'"'''"$(git rev-parse sub1)"'''"'"'"
 '
 
-# this shouldn't actually do anything, since FETCH_HEAD is already a parent
-test_expect_success 'merge fetched subproj' '
-   git merge -m "merge -s -ours" -s ours FETCH_HEAD
+test_expect_success 'merge the added subproj again, should do nothing' '
+   # this shouldn not actually do anything, since FETCH_HEAD
+   # is already a parent
+   result=$(git merge -s ours -m "merge -s -ours" FETCH_HEAD) &&
+   check_equal "${result}" "Already 

Re: [PATCH 1/7] contrib/subtree: Clean and refactor test code

2015-11-10 Thread David A. Greene
David Greene  writes:

Just a ping to ask if anyone has looked at this.  Apparently send-email
uses the commit author as the From address.  These messages are actually
from me, sent on behalf of the commit authors.

I've got more coming, but want to get these in first as they clean up
the testsuite and will make adding future changes easier.

  -David

> From: Techlive Zheng 
>
> Mostly prepare for the later tests refactoring.  This moves some
> common code to helper functions and generally cleans things up to be
> more presentable.
>
> Signed-off-by: Techlive Zheng 
> Signed-off-by: David A. Greene 
> ---
>  contrib/subtree/t/Makefile |  31 ---
>  contrib/subtree/t/t7900-subtree.sh | 103 
> -
>  2 files changed, 79 insertions(+), 55 deletions(-)
>
> diff --git a/contrib/subtree/t/Makefile b/contrib/subtree/t/Makefile
> index c864810..276898e 100644
> --- a/contrib/subtree/t/Makefile
> +++ b/contrib/subtree/t/Makefile
> @@ -13,11 +13,23 @@ TAR ?= $(TAR)
>  RM ?= rm -f
>  PROVE ?= prove
>  DEFAULT_TEST_TARGET ?= test
> +TEST_LINT ?= test-lint
> +
> +ifdef TEST_OUTPUT_DIRECTORY
> +TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results
> +else
> +TEST_RESULTS_DIRECTORY = ../../../t/test-results
> +endif
>  
>  # Shell quote;
>  SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
> +PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
> +TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY))
>  
> -T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
> +T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
> +TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh))
> +TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
> +THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
>  
>  all: $(DEFAULT_TEST_TARGET)
>  
> @@ -26,20 +38,22 @@ test: pre-clean $(TEST_LINT)
>  
>  prove: pre-clean $(TEST_LINT)
>   @echo "*** prove ***"; GIT_CONFIG=.git/config $(PROVE) --exec 
> '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
> - $(MAKE) clean
> + $(MAKE) clean-except-prove-cache
>  
>  $(T):
>   @echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ 
> $(GIT_TEST_OPTS)
>  
>  pre-clean:
> - $(RM) -r test-results
> + $(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'
>  
> -clean:
> - $(RM) -r 'trash directory'.* test-results
> +clean-except-prove-cache:
> + $(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)'
>   $(RM) -r valgrind/bin
> +
> +clean: clean-except-prove-cache
>   $(RM) .prove
>  
> -test-lint: test-lint-duplicates test-lint-executable
> +test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax
>  
>  test-lint-duplicates:
>   @dups=`echo $(T) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
> @@ -51,12 +65,15 @@ test-lint-executable:
>   test -z "$$bad" || { \
>   echo >&2 "non-executable tests:" $$bad; exit 1; }
>  
> +test-lint-shell-syntax:
> + @'$(PERL_PATH_SQ)' ../../../t/check-non-portable-shell.pl $(T) 
> $(THELPERS)
> +
>  aggregate-results-and-cleanup: $(T)
>   $(MAKE) aggregate-results
>   $(MAKE) clean
>  
>  aggregate-results:
> - for f in ../../../t/test-results/t*-*.counts; do \
> + for f in '$(TEST_RESULTS_DIRECTORY_SQ)'/t*-*.counts; do \
>   echo "$$f"; \
>   done | '$(SHELL_PATH_SQ)' ../../../t/aggregate-results.sh
>  
> diff --git a/contrib/subtree/t/t7900-subtree.sh 
> b/contrib/subtree/t/t7900-subtree.sh
> index dfbe443..f9dda3d 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -5,7 +5,7 @@
>  #
>  test_description='Basic porcelain support for subtrees
>  
> -This test verifies the basic operation of the merge, pull, add
> +This test verifies the basic operation of the add, pull, merge
>  and split subcommands of git subtree.
>  '
>  
> @@ -20,7 +20,6 @@ create()
>   git add "$1"
>  }
>  
> -
>  check_equal()
>  {
>   test_debug 'echo'
> @@ -38,6 +37,30 @@ undo()
>   git reset --hard HEAD~
>  }
>  
> +# Make sure no patch changes more than one file.
> +# The original set of commits changed only one file each.
> +# A multi-file change would imply that we pruned commits
> +# too aggressively.
> +join_commits()
> +{
> + commit=
> + all=
> + while read x y; do
> + if [ -z "$x" ]; then
> + continue
> + elif [ "$x" = "commit:" ]; then
> + if [ -n "$commit" ]; then
> + echo "$commit $all"
> + all=
> + fi
> + commit="$y"
> + else
> + all="$all $y"
> + fi
> + done
> + echo "$commit $all"
> +}
> +
>  last_commit_message()
>  {
>   git log --pretty=format:%s -1
> @@ -123,9 +146,11 @@ test_expect_success 'add subproj to 

[PATCH 1/7] contrib/subtree: Clean and refactor test code

2015-11-05 Thread David Greene
From: Techlive Zheng 

Mostly prepare for the later tests refactoring.  This moves some
common code to helper functions and generally cleans things up to be
more presentable.

Signed-off-by: Techlive Zheng 
Signed-off-by: David A. Greene 
---
 contrib/subtree/t/Makefile |  31 ---
 contrib/subtree/t/t7900-subtree.sh | 103 -
 2 files changed, 79 insertions(+), 55 deletions(-)

diff --git a/contrib/subtree/t/Makefile b/contrib/subtree/t/Makefile
index c864810..276898e 100644
--- a/contrib/subtree/t/Makefile
+++ b/contrib/subtree/t/Makefile
@@ -13,11 +13,23 @@ TAR ?= $(TAR)
 RM ?= rm -f
 PROVE ?= prove
 DEFAULT_TEST_TARGET ?= test
+TEST_LINT ?= test-lint
+
+ifdef TEST_OUTPUT_DIRECTORY
+TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results
+else
+TEST_RESULTS_DIRECTORY = ../../../t/test-results
+endif
 
 # Shell quote;
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
+TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY))
 
-T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
+T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
+TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh))
+TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
+THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
 
 all: $(DEFAULT_TEST_TARGET)
 
@@ -26,20 +38,22 @@ test: pre-clean $(TEST_LINT)
 
 prove: pre-clean $(TEST_LINT)
@echo "*** prove ***"; GIT_CONFIG=.git/config $(PROVE) --exec 
'$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
-   $(MAKE) clean
+   $(MAKE) clean-except-prove-cache
 
 $(T):
@echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ 
$(GIT_TEST_OPTS)
 
 pre-clean:
-   $(RM) -r test-results
+   $(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'
 
-clean:
-   $(RM) -r 'trash directory'.* test-results
+clean-except-prove-cache:
+   $(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)'
$(RM) -r valgrind/bin
+
+clean: clean-except-prove-cache
$(RM) .prove
 
-test-lint: test-lint-duplicates test-lint-executable
+test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax
 
 test-lint-duplicates:
@dups=`echo $(T) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
@@ -51,12 +65,15 @@ test-lint-executable:
test -z "$$bad" || { \
echo >&2 "non-executable tests:" $$bad; exit 1; }
 
+test-lint-shell-syntax:
+   @'$(PERL_PATH_SQ)' ../../../t/check-non-portable-shell.pl $(T) 
$(THELPERS)
+
 aggregate-results-and-cleanup: $(T)
$(MAKE) aggregate-results
$(MAKE) clean
 
 aggregate-results:
-   for f in ../../../t/test-results/t*-*.counts; do \
+   for f in '$(TEST_RESULTS_DIRECTORY_SQ)'/t*-*.counts; do \
echo "$$f"; \
done | '$(SHELL_PATH_SQ)' ../../../t/aggregate-results.sh
 
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index dfbe443..f9dda3d 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -5,7 +5,7 @@
 #
 test_description='Basic porcelain support for subtrees
 
-This test verifies the basic operation of the merge, pull, add
+This test verifies the basic operation of the add, pull, merge
 and split subcommands of git subtree.
 '
 
@@ -20,7 +20,6 @@ create()
git add "$1"
 }
 
-
 check_equal()
 {
test_debug 'echo'
@@ -38,6 +37,30 @@ undo()
git reset --hard HEAD~
 }
 
+# Make sure no patch changes more than one file.
+# The original set of commits changed only one file each.
+# A multi-file change would imply that we pruned commits
+# too aggressively.
+join_commits()
+{
+   commit=
+   all=
+   while read x y; do
+   if [ -z "$x" ]; then
+   continue
+   elif [ "$x" = "commit:" ]; then
+   if [ -n "$commit" ]; then
+   echo "$commit $all"
+   all=
+   fi
+   commit="$y"
+   else
+   all="$all $y"
+   fi
+   done
+   echo "$commit $all"
+}
+
 last_commit_message()
 {
git log --pretty=format:%s -1
@@ -123,9 +146,11 @@ test_expect_success 'add subproj to mainline' '
check_equal ''"$(last_commit_message)"'' "Add '"'sub dir/'"' from 
commit '"'"'''"$(git rev-parse sub1)"'''"'"'"
 '
 
-# this shouldn't actually do anything, since FETCH_HEAD is already a parent
-test_expect_success 'merge fetched subproj' '
-   git merge -m "merge -s -ours" -s ours FETCH_HEAD
+test_expect_success 'merge the added subproj again, should do nothing' '
+   # this shouldn not actually do anything, since FETCH_HEAD
+   # is already a parent
+   result=$(git merge -s ours -m "merge -s -ours" FETCH_HEAD) &&
+   check_equal "${result}" "Already