lib-submodule-update: pass 'test_must_fail' as an argument
When we run a test helper function in test_submodule_switch_common(), we sometimes specify a whole helper function as the $command. When we do this, in some test cases, we just mark the whole function with `test_must_fail`. However, it's possible that the helper function might fail earlier or later than expected due to an introduced bug. If this happens, then the test case will still report as passing but it should really be marked as failing since it didn't actually display the intended behaviour. Instead of invoking `test_must_fail $command`, pass the string "test_must_fail" as the second argument in case where the git command is expected to fail. When $command is a helper function, the parent function calling test_submodule_switch_common() is test_submodule_switch_func(). For all test_submodule_switch_func() invocations, increase the granularity of the argument test helper function by prefixing the git invocation which is meant to fail with the second argument like this: $2 git checkout "$1" In the other cases, test_submodule_switch() and test_submodule_forced_switch(), instead of passing in the git command directly, wrap it using the git_test_func() and pass the git arguments using the global variable $gitcmd. Unfortunately, since closures aren't a thing in shell scripts, the global variable is necessary. Another unfortunate result is that the "git_test_func" will used as the test case name when $command is printed but it's worth it for the cleaner code. Finally, as an added bonus, `test_must_fail` will now only run on git commands. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Denton Liu committed
Jun 24, 2020 at 04:50 UTC
5b0ac09fb17656553d85c7e3fbe0583ba3023e5e
8 files changed
+84
-26
t/lib-submodule-update.sh
+42
-13
@@ -303,8 +303,12 @@ test_submodule_content () {
303
# update" is run. And even then that command doesn't delete the work tree of
304
# a removed submodule.
305
#
306
+# The first argument of the callback function will be the name of the submodule.
307
+#
308
# Removing a submodule containing a .git directory must fail even when forced
307
-# to protect the history!
309
+# to protect the history! If we are testing this case, the second argument of
310
+# the callback function will be 'test_must_fail', else it will be the empty
311
+# string.
312
#
313
314
# Internal function; use test_submodule_switch_func(), test_submodule_switch(),
@@ -443,7 +447,7 @@ test_submodule_switch_common () {
447
(
448
cd submodule_update &&
449
git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
446
- test_must_fail $command replace_sub1_with_directory &&
450
+ $command replace_sub1_with_directory test_must_fail &&
451
test_superproject_content origin/add_sub1 &&
452
test_submodule_content sub1 origin/add_sub1
453
)
@@ -456,7 +460,7 @@ test_submodule_switch_common () {
460
cd submodule_update &&
461
git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
462
replace_gitfile_with_git_dir sub1 &&
459
- test_must_fail $command replace_sub1_with_directory &&
463
+ $command replace_sub1_with_directory test_must_fail &&
464
test_superproject_content origin/add_sub1 &&
465
test_git_directory_is_unchanged sub1 &&
466
test_submodule_content sub1 origin/add_sub1
@@ -470,7 +474,7 @@ test_submodule_switch_common () {
474
(
475
cd submodule_update &&
476
git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
473
- test_must_fail $command replace_sub1_with_file &&
477
+ $command replace_sub1_with_file test_must_fail &&
478
test_superproject_content origin/add_sub1 &&
479
test_submodule_content sub1 origin/add_sub1
480
)
@@ -484,7 +488,7 @@ test_submodule_switch_common () {
488
cd submodule_update &&
489
git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
490
replace_gitfile_with_git_dir sub1 &&
487
- test_must_fail $command replace_sub1_with_file &&
491
+ $command replace_sub1_with_file test_must_fail &&
492
test_superproject_content origin/add_sub1 &&
493
test_git_directory_is_unchanged sub1 &&
494
test_submodule_content sub1 origin/add_sub1
@@ -559,12 +563,25 @@ test_submodule_switch_common () {
563
# conditions, set the appropriate KNOWN_FAILURE_* variable used in the tests
564
# below to 1.
565
#
562
-# Use as follows:
566
+# The first argument of the callback function will be the name of the submodule.
567
+#
568
+# Removing a submodule containing a .git directory must fail even when forced
569
+# to protect the history! If we are testing this case, the second argument of
570
+# the callback function will be 'test_must_fail', else it will be the empty
571
+# string.
572
+#
573
+# The following example uses `git some-command` as an example command to be
574
+# tested. It updates the worktree and index to match a target, but not any
575
+# submodule directories.
576
#
577
# my_func () {
565
-# target=$1
566
-# # Do something here that updates the worktree and index to match target,
567
-# # but not any submodule directories.
578
+# ...prepare for `git some-command` to be run...
579
+# $2 git some-command "$1" &&
580
+# if test -n "$2"
581
+# then
582
+# return
583
+# fi &&
584
+# ...check the state after git some-command is run...
585
# }
586
# test_submodule_switch_func "my_func"
587
test_submodule_switch_func () {
@@ -580,23 +597,35 @@ test_submodule_switch_func () {
597
cd submodule_update &&
598
git branch -t add_sub1 origin/add_sub1 &&
599
>sub1 &&
583
- test_must_fail $command add_sub1 &&
600
+ $command add_sub1 test_must_fail &&
601
test_superproject_content origin/no_submodule &&
602
test_must_be_empty sub1
603
)
604
'
605
}
606
607
+# Ensures that the that the arg either contains "test_must_fail" or is empty.
608
+may_only_be_test_must_fail () {
609
+ test -z "$1" || test "$1" = test_must_fail || die
610
+}
611
+
612
+git_test_func () {
613
+ may_only_be_test_must_fail "$2" &&
614
+ $2 git $gitcmd "$1"
615
+}
616
+
617
test_submodule_switch () {
591
- test_submodule_switch_func "git $1"
618
+ gitcmd="$1"
619
+ test_submodule_switch_func "git_test_func"
620
}
621
622
# Same as test_submodule_switch(), except that throwing away local changes in
623
# the superproject is allowed.
624
test_submodule_forced_switch () {
597
- command="$1"
625
+ gitcmd="$1"
626
+ command="git_test_func"
627
KNOWN_FAILURE_FORCED_SWITCH_TESTS=1
599
- test_submodule_switch_common "git $command"
628
+ test_submodule_switch_common "$command"
629
630
# When forced, a file in the superproject does not prevent creating a
631
# submodule of the same name.
t/t3426-rebase-submodule.sh
+4
-2
@@ -17,7 +17,8 @@ git_rebase () {
17
git status -su >actual &&
18
ls -1pR * >>actual &&
19
test_cmp expect actual &&
20
- git rebase "$1"
20
+ may_only_be_test_must_fail "$2" &&
21
+ $2 git rebase "$1"
22
}
23
24
test_submodule_switch_func "git_rebase"
@@ -35,7 +36,8 @@ git_rebase_interactive () {
36
test_cmp expect actual &&
37
set_fake_editor &&
38
echo "fake-editor.sh" >.git/info/exclude &&
38
- git rebase -i "$1"
39
+ may_only_be_test_must_fail "$2" &&
40
+ $2 git rebase -i "$1"
41
}
42
43
test_submodule_switch_func "git_rebase_interactive"
t/t3513-revert-submodule.sh
+6
-1
@@ -15,7 +15,12 @@ git_revert () {
15
git status -su >expect &&
16
ls -1pR * >>expect &&
17
tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
18
- git checkout "$1" &&
18
+ may_only_be_test_must_fail "$2" &&
19
+ $2 git checkout "$1" &&
20
+ if test -n "$2"
21
+ then
22
+ return
23
+ fi &&
24
git revert HEAD &&
25
rm -rf * &&
26
tar xf "$TRASH_DIRECTORY/tmp.tar" &&
t/t3906-stash-submodule.sh
+6
-1
@@ -8,7 +8,12 @@ test_description='stash can handle submodules'
8
git_stash () {
9
git status -su >expect &&
10
ls -1pR * >>expect &&
11
- git read-tree -u -m "$1" &&
11
+ may_only_be_test_must_fail "$2" &&
12
+ $2 git read-tree -u -m "$1" &&
13
+ if test -n "$2"
14
+ then
15
+ return
16
+ fi &&
17
git stash &&
18
git status -su >actual &&
19
ls -1pR * >>actual &&
t/t4137-apply-submodule.sh
+6
-2
@@ -6,13 +6,17 @@ test_description='git apply handling submodules'
6
. "$TEST_DIRECTORY"/lib-submodule-update.sh
7
8
apply_index () {
9
- git diff --ignore-submodules=dirty "..$1" | git apply --index -
9
+ git diff --ignore-submodules=dirty "..$1" >diff &&
10
+ may_only_be_test_must_fail "$2" &&
11
+ $2 git apply --index diff
12
}
13
14
test_submodule_switch_func "apply_index"
15
16
apply_3way () {
15
- git diff --ignore-submodules=dirty "..$1" | git apply --3way -
17
+ git diff --ignore-submodules=dirty "..$1" >diff &&
18
+ may_only_be_test_must_fail "$2" &&
19
+ $2 git apply --3way diff
20
}
21
22
test_submodule_switch_func "apply_3way"
t/t4255-am-submodule.sh
+6
-2
@@ -6,13 +6,17 @@ test_description='git am handling submodules'
6
. "$TEST_DIRECTORY"/lib-submodule-update.sh
7
8
am () {
9
- git format-patch --stdout --ignore-submodules=dirty "..$1" | git am -
9
+ git format-patch --stdout --ignore-submodules=dirty "..$1" >patch &&
10
+ may_only_be_test_must_fail "$2" &&
11
+ $2 git am patch
12
}
13
14
test_submodule_switch_func "am"
15
16
am_3way () {
15
- git format-patch --stdout --ignore-submodules=dirty "..$1" | git am --3way -
17
+ git format-patch --stdout --ignore-submodules=dirty "..$1" >patch &&
18
+ may_only_be_test_must_fail "$2" &&
19
+ $2 git am --3way patch
20
}
21
22
KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
t/t5572-pull-submodule.sh
+8
-4
@@ -13,7 +13,8 @@ reset_branch_to_HEAD () {
13
14
git_pull () {
15
reset_branch_to_HEAD "$1" &&
16
- git pull
16
+ may_only_be_test_must_fail "$2" &&
17
+ $2 git pull
18
}
19
20
# pulls without conflicts
@@ -21,21 +22,24 @@ test_submodule_switch_func "git_pull"
22
23
git_pull_ff () {
24
reset_branch_to_HEAD "$1" &&
24
- git pull --ff
25
+ may_only_be_test_must_fail "$2" &&
26
+ $2 git pull --ff
27
}
28
29
test_submodule_switch_func "git_pull_ff"
30
31
git_pull_ff_only () {
32
reset_branch_to_HEAD "$1" &&
31
- git pull --ff-only
33
+ may_only_be_test_must_fail "$2" &&
34
+ $2 git pull --ff-only
35
}
36
37
test_submodule_switch_func "git_pull_ff_only"
38
39
git_pull_noff () {
40
reset_branch_to_HEAD "$1" &&
38
- git pull --no-ff
41
+ may_only_be_test_must_fail "$2" &&
42
+ $2 git pull --no-ff
43
}
44
45
KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
t/t6041-bisect-submodule.sh
+6
-1
@@ -10,7 +10,12 @@ git_bisect () {
10
ls -1pR * >>expect &&
11
tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
12
GOOD=$(git rev-parse --verify HEAD) &&
13
- git checkout "$1" &&
13
+ may_only_be_test_must_fail "$2" &&
14
+ $2 git checkout "$1" &&
15
+ if test -n "$2"
16
+ then
17
+ return
18
+ fi &&
19
echo "foo" >bar &&
20
git add bar &&
21
git commit -m "bisect bad" &&