git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 4/4] lib-submodule-update: pass OVERWRITING_FAIL

From
Denton Liu <liu.denton@gmail.com>
Date
Apr 29, 2020, 12:22 UTC
Message-ID
<22eacd20a8213c3281974a6c103fd2d00c95efaa.1588162842.git.liu.denton@gmail.com>
In-Reply-To
<cover.1588162842.git.liu.denton@gmail.com>

We are using `test_must_fail $command`. However, $command is not necessarily a git command; it could be a test helper function.

In an effort to stop using test_must_fail with non-git commands, instead of invoking `test_must_fail $command`, run `OVERWRITING_FAIL=test_must_fail $command` instead. Increase the granularity of the test helper functions by specifically choosing the individual git invocation which is designed to fail.

While we're at it, some helper functions have git commands piping into another git command. Break these pipes up into two separate invocations with a file buffer so that the return code of the first command is not lost.

This patch can be better viewed with `--ignore-all-space`.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/lib-submodule-update.sh   | 25 +++++++++++++++++--------
 t/t3426-rebase-submodule.sh |  4 ++--
 t/t3513-revert-submodule.sh | 20 ++++++++++++--------
 t/t3906-stash-submodule.sh  | 15 +++++++++------
 t/t4137-apply-submodule.sh  |  6 ++++--
 t/t4255-am-submodule.sh     |  6 ++++--
 t/t5572-pull-submodule.sh   |  8 ++++----
 t/t6041-bisect-submodule.sh | 33 ++++++++++++++++++---------------
 8 files changed, 70 insertions(+), 47 deletions(-)
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index cd80d77707..c8d1b6f528 100644
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -304,12 +304,15 @@ test_submodule_content () {
 # a removed submodule.
 #
 # Removing a submodule containing a .git directory must fail even when forced
-# to protect the history!
+# to protect the history! If we are testing this case,
+# OVERWRITING_FAIL=test_must_fail, otherwise OVERWRITING_FAIL will be the empty
+# string.
 #
 
 # Internal function; use test_submodule_switch_func(), test_submodule_switch_func(),
 # or test_submodule_forced_switch() instead.
 test_submodule_switch_common () {
+	OVERWRITING_FAIL=
 	command="$1"
 	######################### Appearing submodule #########################
 	# Switching to a commit letting a submodule appear creates empty dir ...
@@ -443,7 +446,7 @@ test_submodule_switch_common () {
 		(
 			cd submodule_update &&
 			git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
-			test_must_fail $command replace_sub1_with_directory &&
+			OVERWRITING_FAIL=test_must_fail $command replace_sub1_with_directory &&
 			test_superproject_content origin/add_sub1 &&
 			test_submodule_content sub1 origin/add_sub1
 		)
@@ -456,7 +459,7 @@ test_submodule_switch_common () {
 			cd submodule_update &&
 			git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
 			replace_gitfile_with_git_dir sub1 &&
-			test_must_fail $command replace_sub1_with_directory &&
+			OVERWRITING_FAIL=test_must_fail $command replace_sub1_with_directory &&
 			test_superproject_content origin/add_sub1 &&
 			test_git_directory_is_unchanged sub1 &&
 			test_submodule_content sub1 origin/add_sub1
@@ -470,7 +473,7 @@ test_submodule_switch_common () {
 		(
 			cd submodule_update &&
 			git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
-			test_must_fail $command replace_sub1_with_file &&
+			OVERWRITING_FAIL=test_must_fail $command replace_sub1_with_file &&
 			test_superproject_content origin/add_sub1 &&
 			test_submodule_content sub1 origin/add_sub1
 		)
@@ -484,7 +487,7 @@ test_submodule_switch_common () {
 			cd submodule_update &&
 			git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
 			replace_gitfile_with_git_dir sub1 &&
-			test_must_fail $command replace_sub1_with_file &&
+			OVERWRITING_FAIL=test_must_fail $command replace_sub1_with_file &&
 			test_superproject_content origin/add_sub1 &&
 			test_git_directory_is_unchanged sub1 &&
 			test_submodule_content sub1 origin/add_sub1
@@ -559,6 +562,11 @@ test_submodule_switch_common () {
 # conditions, set the appropriate KNOWN_FAILURE_* variable used in the tests
 # below to 1.
 #
+# Removing a submodule containing a .git directory must fail even when forced
+# to protect the history! If we are testing this case,
+# OVERWRITING_FAIL=test_must_fail, otherwise OVERWRITING_FAIL will be the empty
+# string.
+#
 # Use as follows:
 #
 # my_func () {
@@ -568,6 +576,7 @@ test_submodule_switch_common () {
 # }
 # test_submodule_switch_func "my_func"
 test_submodule_switch_func () {
+	OVERWRITING_FAIL=
 	command="$1"
 	test_submodule_switch_common "$command"
 
@@ -580,7 +589,7 @@ test_submodule_switch_func () {
 			cd submodule_update &&
 			git branch -t add_sub1 origin/add_sub1 &&
 			>sub1 &&
-			test_must_fail $command add_sub1 &&
+			OVERWRITING_FAIL=test_must_fail $command add_sub1 &&
 			test_superproject_content origin/no_submodule &&
 			test_must_be_empty sub1
 		)
@@ -588,7 +597,7 @@ test_submodule_switch_func () {
 }
 
 test_submodule_switch () {
-	test_submodule_switch_func "git $1"
+	test_submodule_switch_func "eval \$OVERWRITING_FAIL git $1"
 }
 
 # Same as test_submodule_switch(), except that throwing away local changes in
@@ -596,7 +605,7 @@ test_submodule_switch () {
 test_submodule_forced_switch () {
 	command="$1"
 	KNOWN_FAILURE_FORCED_SWITCH_TESTS=1
-	test_submodule_switch_common "git $command"
+	test_submodule_switch_common "eval \$OVERWRITING_FAIL git $command"
 
 	# When forced, a file in the superproject does not prevent creating a
 	# submodule of the same name.
diff --git a/t/t3426-rebase-submodule.sh b/t/t3426-rebase-submodule.sh
index 788605ccc0..c6a7f584ed 100755
--- a/t/t3426-rebase-submodule.sh
+++ b/t/t3426-rebase-submodule.sh
@@ -17,7 +17,7 @@ git_rebase () {
 	git status -su >actual &&
 	ls -1pR * >>actual &&
 	test_cmp expect actual &&
-	git rebase "$1"
+	$OVERWRITING_FAIL git rebase "$1"
 }
 
 test_submodule_switch_func "git_rebase"
@@ -35,7 +35,7 @@ git_rebase_interactive () {
 	test_cmp expect actual &&
 	set_fake_editor &&
 	echo "fake-editor.sh" >.git/info/exclude &&
-	git rebase -i "$1"
+	$OVERWRITING_FAIL git rebase -i "$1"
 }
 
 test_submodule_switch_func "git_rebase_interactive"
diff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh
index 95a7f64471..6c899db7e1 100755
--- a/t/t3513-revert-submodule.sh
+++ b/t/t3513-revert-submodule.sh
@@ -15,14 +15,18 @@ git_revert () {
 	git status -su >expect &&
 	ls -1pR * >>expect &&
 	tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
-	git checkout "$1" &&
-	git revert HEAD &&
-	rm -rf * &&
-	tar xf "$TRASH_DIRECTORY/tmp.tar" &&
-	git status -su >actual &&
-	ls -1pR * >>actual &&
-	test_cmp expect actual &&
-	git revert HEAD
+	$OVERWRITING_FAIL git checkout "$1" &&
+	if test -z "$OVERWRITING_FAIL"
+	then
+		git checkout "$1" &&
+		git revert HEAD &&
+		rm -rf * &&
+		tar xf "$TRASH_DIRECTORY/tmp.tar" &&
+		git status -su >actual &&
+		ls -1pR * >>actual &&
+		test_cmp expect actual &&
+		git revert HEAD
+	fi
 }
 
 KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
diff --git a/t/t3906-stash-submodule.sh b/t/t3906-stash-submodule.sh
index 6a7e801ca0..860940072d 100755
--- a/t/t3906-stash-submodule.sh
+++ b/t/t3906-stash-submodule.sh
@@ -8,12 +8,15 @@ test_description='stash can handle submodules'
 git_stash () {
 	git status -su >expect &&
 	ls -1pR * >>expect &&
-	git read-tree -u -m "$1" &&
-	git stash &&
-	git status -su >actual &&
-	ls -1pR * >>actual &&
-	test_cmp expect actual &&
-	git stash apply
+	$OVERWRITING_FAIL git read-tree -u -m "$1" &&
+	if test -z "$OVERWRITING_FAIL"
+	then
+		git stash &&
+		git status -su >actual &&
+		ls -1pR * >>actual &&
+		test_cmp expect actual &&
+		git stash apply
+	fi
 }
 
 KNOWN_FAILURE_STASH_DOES_IGNORE_SUBMODULE_CHANGES=1
diff --git a/t/t4137-apply-submodule.sh b/t/t4137-apply-submodule.sh
index b645e303a0..dd08d9e1a4 100755
--- a/t/t4137-apply-submodule.sh
+++ b/t/t4137-apply-submodule.sh
@@ -6,13 +6,15 @@ test_description='git apply handling submodules'
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 
 apply_index () {
-	git diff --ignore-submodules=dirty "..$1" | git apply --index -
+	git diff --ignore-submodules=dirty "..$1" >diff &&
+	$OVERWRITING_FAIL git apply --index - <diff
 }
 
 test_submodule_switch_func "apply_index"
 
 apply_3way () {
-	git diff --ignore-submodules=dirty "..$1" | git apply --3way -
+	git diff --ignore-submodules=dirty "..$1" >diff &&
+	$OVERWRITING_FAIL git apply --3way - <diff
 }
 
 test_submodule_switch_func "apply_3way"
diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
index 1b179d5f45..b0fe78a956 100755
--- a/t/t4255-am-submodule.sh
+++ b/t/t4255-am-submodule.sh
@@ -6,13 +6,15 @@ test_description='git am handling submodules'
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 
 am () {
-	git format-patch --stdout --ignore-submodules=dirty "..$1" | git am -
+	git format-patch --stdout --ignore-submodules=dirty "..$1" >patch &&
+	$OVERWRITING_FAIL git am - <patch
 }
 
 test_submodule_switch_func "am"
 
 am_3way () {
-	git format-patch --stdout --ignore-submodules=dirty "..$1" | git am --3way -
+	git format-patch --stdout --ignore-submodules=dirty "..$1" >patch &&
+	$OVERWRITING_FAIL git am --3way - <patch
 }
 
 KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
index f911bf631e..4dd9913b6b 100755
--- a/t/t5572-pull-submodule.sh
+++ b/t/t5572-pull-submodule.sh
@@ -13,7 +13,7 @@ reset_branch_to_HEAD () {
 
 git_pull () {
 	reset_branch_to_HEAD "$1" &&
-	git pull
+	$OVERWRITING_FAIL git pull
 }
 
 # pulls without conflicts
@@ -21,21 +21,21 @@ test_submodule_switch_func "git_pull"
 
 git_pull_ff () {
 	reset_branch_to_HEAD "$1" &&
-	git pull --ff
+	$OVERWRITING_FAIL git pull --ff
 }
 
 test_submodule_switch_func "git_pull_ff"
 
 git_pull_ff_only () {
 	reset_branch_to_HEAD "$1" &&
-	git pull --ff-only
+	$OVERWRITING_FAIL git pull --ff-only
 }
 
 test_submodule_switch_func "git_pull_ff_only"
 
 git_pull_noff () {
 	reset_branch_to_HEAD "$1" &&
-	git pull --no-ff
+	$OVERWRITING_FAIL git pull --no-ff
 }
 
 KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
diff --git a/t/t6041-bisect-submodule.sh b/t/t6041-bisect-submodule.sh
index 0e0cdf638d..714d393899 100755
--- a/t/t6041-bisect-submodule.sh
+++ b/t/t6041-bisect-submodule.sh
@@ -10,21 +10,24 @@ git_bisect () {
 	ls -1pR * >>expect &&
 	tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
 	GOOD=$(git rev-parse --verify HEAD) &&
-	git checkout "$1" &&
-	echo "foo" >bar &&
-	git add bar &&
-	git commit -m "bisect bad" &&
-	BAD=$(git rev-parse --verify HEAD) &&
-	git reset --hard HEAD^^ &&
-	git submodule update &&
-	git bisect start &&
-	git bisect good $GOOD &&
-	rm -rf * &&
-	tar xf "$TRASH_DIRECTORY/tmp.tar" &&
-	git status -su >actual &&
-	ls -1pR * >>actual &&
-	test_cmp expect actual &&
-	git bisect bad $BAD
+	$OVERWRITING_FAIL git checkout "$1" &&
+	if test -z "$OVERWRITING_FAIL"
+	then
+		echo "foo" >bar &&
+		git add bar &&
+		git commit -m "bisect bad" &&
+		BAD=$(git rev-parse --verify HEAD) &&
+		git reset --hard HEAD^^ &&
+		git submodule update &&
+		git bisect start &&
+		git bisect good $GOOD &&
+		rm -rf * &&
+		tar xf "$TRASH_DIRECTORY/tmp.tar" &&
+		git status -su >actual &&
+		ls -1pR * >>actual &&
+		test_cmp expect actual &&
+		git bisect bad $BAD
+	fi
 }
 
 test_submodule_switch_func "git_bisect"

-- 
2.26.2.548.gbb00c8a0a9
Previous: Junio C HamanoNext: Junio C Hamano
Message 6 of 64 in “t: replace incorrect test_must_fail usage (part 5)”
  1. 0/4 t: replace incorrect test_must_fail usage (part 5)Denton Liu, Apr 29, 2020
  2. 1/4 lib-submodule-update: add space after function nameDenton Liu, Apr 29, 2020
  3. 3/4 lib-submodule-update: prepend "git" to $commandDenton Liu, Apr 29, 2020
  4. 2/4 lib-submodule-update: consolidate --recurse-submodulesDenton Liu, Apr 29, 2020
  5. Junio C HamanoApr 29, 2020
  6. 4/4 lib-submodule-update: pass OVERWRITING_FAILDenton Liu, Apr 29, 2020
  7. Junio C HamanoApr 29, 2020
  8. Denton LiuApr 30, 2020
  9. Junio C HamanoApr 30, 2020
  10. Denton LiuApr 30, 2020
  11. lib-submodule-update: pass OVERWRITING_FAILDenton Liu, Apr 30, 2020
  12. Junio C HamanoApr 30, 2020
  13. Denton LiuMay 1, 2020
  14. Junio C HamanoMay 1, 2020
  15. lib-submodule-update: pass OVERWRITING_FAILDenton Liu, May 5, 2020
  16. Taylor BlauApr 29, 2020
  17. Johannes SixtApr 29, 2020
  18. Re* [PATCH 0/4] t: replace incorrect test_must_fail usage (part 5)Junio C Hamano, Apr 29, 2020
  19. Taylor BlauApr 29, 2020
  20. Junio C HamanoApr 29, 2020
  21. Taylor BlauApr 29, 2020
  22. Junio C HamanoApr 29, 2020
  23. Taylor BlauApr 29, 2020
  24. Eric SunshineApr 29, 2020
  25. Junio C HamanoApr 29, 2020
  26. 0/4 t: replace incorrect test_must_fail usage (part 5)Denton Liu, May 21, 2020
  27. 1/4 lib-submodule-update: add space after function nameDenton Liu, May 21, 2020
  28. 2/4 lib-submodule-update: consolidate --recurse-submodulesDenton Liu, May 21, 2020
  29. 3/4 lib-submodule-update: prepend "git" to $commandDenton Liu, May 21, 2020
  30. Philip OakleyMay 21, 2020
  31. Denton LiuMay 21, 2020
  32. 4/4 lib-submodule-update: pass OVERWRITING_FAILDenton Liu, May 21, 2020
  33. Jeff KingMay 21, 2020
  34. Denton LiuMay 21, 2020
  35. Junio C HamanoMay 21, 2020
  36. Jeff KingMay 21, 2020
  37. Jeff KingMay 21, 2020
  38. Junio C HamanoMay 21, 2020
  39. Jeff KingMay 21, 2020
  40. 0/4 t: replace incorrect test_must_fail usage (part 5)Denton Liu, Jun 11, 2020
  41. 1/4 lib-submodule-update: add space after function nameDenton Liu, Jun 11, 2020
  42. 2/4 lib-submodule-update: consolidate --recurse-submodulesDenton Liu, Jun 11, 2020
  43. 3/4 lib-submodule-update: prepend "git" to $commandDenton Liu, Jun 11, 2020
  44. 4/4 lib-submodule-update: use callbacks in test_submodule_switch_common()Denton Liu, Jun 11, 2020
  45. Junio C HamanoJun 12, 2020
  46. Junio C HamanoJun 18, 2020
  47. 0/4 t: replace incorrect test_must_fail usage (part 5)Denton Liu, Jun 18, 2020
  48. 1/4 lib-submodule-update: add space after function nameDenton Liu, Jun 18, 2020
  49. 2/4 lib-submodule-update: consolidate --recurse-submodulesDenton Liu, Jun 18, 2020
  50. 3/4 lib-submodule-update: prepend "git" to $commandDenton Liu, Jun 18, 2020
  51. 4/4 lib-submodule-update: use callbacks in test_submodule_switch_common()Denton Liu, Jun 18, 2020
  52. Jeff KingJun 18, 2020
  53. 0/4 t: replace incorrect test_must_fail usage (part 5)Denton Liu, Jun 23, 2020
  54. 1/4 lib-submodule-update: add space after function nameDenton Liu, Jun 23, 2020
  55. 2/4 lib-submodule-update: consolidate --recurse-submodulesDenton Liu, Jun 23, 2020
  56. 4/4 lib-submodule-update: use callbacks in test_submodule_switch_common()Denton Liu, Jun 23, 2020
  57. 3/4 lib-submodule-update: prepend "git" to $commandDenton Liu, Jun 23, 2020
  58. Jeff KingJun 23, 2020
  59. lib-submodule-update: pass 'test_must_fail' as an argumentDenton Liu, Jun 23, 2020
  60. lib-submodule-update: pass 'test_must_fail' as an argumentDenton Liu, Jun 23, 2020
  61. Junio C HamanoJun 23, 2020
  62. Đoàn Trần Công DanhJun 24, 2020
  63. lib-submodule-update: pass 'test_must_fail' as an argumentDenton Liu, Jun 24, 2020
  64. Junio C HamanoJun 24, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.