Re: [PATCH v3 1/2] t4255: test am submodule with diff.submodule

2015-01-07 Thread Eric Sunshine
On Wed, Jan 7, 2015 at 2:31 PM, Doug Kelly dougk@gmail.com wrote:
 git am will break when using diff.submodule=log; add some test cases
 to illustrate this breakage as simply as possible.  There are
 currently two ways this can fail:

 * With errors (unrecognized input), if only change
 * Silently (no submodule change), if other files change

 Test for both conditions and ensure without diff.submodule this works.

 Signed-off-by: Doug Kelly dougk@gmail.com
 Thanks-to: Eric Sunshine sunsh...@sunshineco.com
 Thanks-to: Junio C Hamano gits...@pobox.com

On this project, it's customary to say Helped-by: rather than
Thanks-to:. Also, place your sign-off last.

 ---
 Updated with Eric Sunshine's comments and changes to reduce complexity,
 and also changed to include Junio's suggestions for using test_config,
 test_unconfig, and test_might_fail (since we don't know if a previous
 am failed or not -- we always want to clean up first).

Looking much better. Thanks. A couple minor comments below...

 diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
 index 8bde7db..523accf 100755
 --- a/t/t4255-am-submodule.sh
 +++ b/t/t4255-am-submodule.sh
 @@ -18,4 +18,76 @@ am_3way () {
  KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
  test_submodule_switch am_3way

 +test_expect_success 'setup diff.submodule' '
 +   test_commit one 
 +   INITIAL=$(git rev-parse HEAD) 
 +
 +   git init submodule 
 +   (
 +   cd submodule 
 +   test_commit two 
 +   git rev-parse HEAD ../initial-submodule
 +   ) 
 +   git submodule add ./submodule 
 +   git commit -m first 
 +
 +   (
 +   cd submodule 
 +   test_commit three 
 +   git rev-parse HEAD ../first-submodule
 +   ) 
 +   git add submodule 
 +   test_tick 

You can drop this test_tick (as I did in my squash[1]).

 +   git commit -m second 
 +   SECOND=$(git rev-parse HEAD) 
 +
 +   (
 +   cd submodule 
 +   git mv two.t four.t 
 +   test_tick 

And this one (which I overlooked in [1]).

The reason I suggest dropping the test_tick invocations is that they
do not impact these tests at all, yet their presence misleads the
reader into thinking that they are somehow significant.

 +   git commit -m second submodule 
 +   git rev-parse HEAD ../second-submodule
 +   ) 
 +   test_commit four 
 +   git add submodule 
 +   git commit --amend --no-edit 
 +   THIRD=$(git rev-parse HEAD) 
 +   git submodule update --init
 +'

[1]: http://article.gmane.org/gmane.comp.version-control.git/261852
--
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


[PATCH v3 1/2] t4255: test am submodule with diff.submodule

2015-01-07 Thread Doug Kelly
git am will break when using diff.submodule=log; add some test cases
to illustrate this breakage as simply as possible.  There are
currently two ways this can fail:

* With errors (unrecognized input), if only change
* Silently (no submodule change), if other files change

Test for both conditions and ensure without diff.submodule this works.

Signed-off-by: Doug Kelly dougk@gmail.com
Thanks-to: Eric Sunshine sunsh...@sunshineco.com
Thanks-to: Junio C Hamano gits...@pobox.com
---
Updated with Eric Sunshine's comments and changes to reduce complexity,
and also changed to include Junio's suggestions for using test_config,
test_unconfig, and test_might_fail (since we don't know if a previous
am failed or not -- we always want to clean up first).

 t/t4255-am-submodule.sh | 72 +
 1 file changed, 72 insertions(+)

diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
index 8bde7db..523accf 100755
--- a/t/t4255-am-submodule.sh
+++ b/t/t4255-am-submodule.sh
@@ -18,4 +18,76 @@ am_3way () {
 KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
 test_submodule_switch am_3way
 
+test_expect_success 'setup diff.submodule' '
+   test_commit one 
+   INITIAL=$(git rev-parse HEAD) 
+
+   git init submodule 
+   (
+   cd submodule 
+   test_commit two 
+   git rev-parse HEAD ../initial-submodule
+   ) 
+   git submodule add ./submodule 
+   git commit -m first 
+
+   (
+   cd submodule 
+   test_commit three 
+   git rev-parse HEAD ../first-submodule
+   ) 
+   git add submodule 
+   test_tick 
+   git commit -m second 
+   SECOND=$(git rev-parse HEAD) 
+
+   (
+   cd submodule 
+   git mv two.t four.t 
+   test_tick 
+   git commit -m second submodule 
+   git rev-parse HEAD ../second-submodule
+   ) 
+   test_commit four 
+   git add submodule 
+   git commit --amend --no-edit 
+   THIRD=$(git rev-parse HEAD) 
+   git submodule update --init
+'
+
+run_test() {
+   START_COMMIT=$1 
+   EXPECT=$2 
+   test_might_fail git am --abort 
+   git reset --hard $START_COMMIT 
+   rm -f *.patch 
+   git format-patch -1 
+   git reset --hard $START_COMMIT^ 
+   git submodule update 
+   git am *.patch 
+   git submodule update 
+   git -C submodule rev-parse HEAD actual 
+   test_cmp $EXPECT actual
+}
+
+test_expect_success 'diff.submodule unset' '
+   test_unconfig diff.submodule 
+   run_test $SECOND first-submodule
+'
+
+test_expect_success 'diff.submodule unset with extra file' '
+   test_unconfig diff.submodule 
+   run_test $THIRD second-submodule
+'
+
+test_expect_failure 'diff.submodule=log' '
+   test_config diff.submodule log 
+   run_test $SECOND first-submodule
+'
+
+test_expect_failure 'diff.submodule=log with extra file' '
+   test_config diff.submodule log 
+   run_test $THIRD second-submodule
+'
+
 test_done
-- 
2.0.5

--
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