Skip to content

Commit efafdca

Browse files
gitstergitster
committed
Merge branch 'dl/test-must-fail-fixes-5'
The effort to avoid using test_must_fail on non-git command continues. * dl/test-must-fail-fixes-5: lib-submodule-update: pass 'test_must_fail' as an argument lib-submodule-update: prepend "git" to $command lib-submodule-update: consolidate --recurse-submodules lib-submodule-update: add space after function name
2 parents 0a23331 + 5b0ac09 commit efafdca

13 files changed

+121
-59
lines changed

‎t/lib-submodule-update.sh‎

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ test_git_directory_is_unchanged () {
183183
)
184184
}
185185

186-
test_git_directory_exists() {
186+
test_git_directory_exists () {
187187
test -e ".git/modules/$1" &&
188188
if test -f sub1/.git
189189
then
@@ -303,13 +303,17 @@ test_submodule_content () {
303303
# update" is run. And even then that command doesn't delete the work tree of
304304
# a removed submodule.
305305
#
306+
# The first argument of the callback function will be the name of the submodule.
307+
#
306308
# 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.
308312
#
309313

310-
# Internal function; use test_submodule_switch() or
311-
# test_submodule_forced_switch() instead.
312-
test_submodule_switch_common() {
314+
# Internal function; use test_submodule_switch_func(), test_submodule_switch(),
315+
# or test_submodule_forced_switch() instead.
316+
test_submodule_switch_common () {
313317
command="$1"
314318
######################### Appearing submodule #########################
315319
# Switching to a commit letting a submodule appear creates empty dir ...
@@ -443,7 +447,7 @@ test_submodule_switch_common() {
443447
(
444448
cd submodule_update &&
445449
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 &&
447451
test_superproject_content origin/add_sub1 &&
448452
test_submodule_content sub1 origin/add_sub1
449453
)
@@ -456,7 +460,7 @@ test_submodule_switch_common() {
456460
cd submodule_update &&
457461
git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
458462
replace_gitfile_with_git_dir sub1 &&
459-
test_must_fail $command replace_sub1_with_directory &&
463+
$command replace_sub1_with_directory test_must_fail &&
460464
test_superproject_content origin/add_sub1 &&
461465
test_git_directory_is_unchanged sub1 &&
462466
test_submodule_content sub1 origin/add_sub1
@@ -470,7 +474,7 @@ test_submodule_switch_common() {
470474
(
471475
cd submodule_update &&
472476
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 &&
474478
test_superproject_content origin/add_sub1 &&
475479
test_submodule_content sub1 origin/add_sub1
476480
)
@@ -484,7 +488,7 @@ test_submodule_switch_common() {
484488
cd submodule_update &&
485489
git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
486490
replace_gitfile_with_git_dir sub1 &&
487-
test_must_fail $command replace_sub1_with_file &&
491+
$command replace_sub1_with_file test_must_fail &&
488492
test_superproject_content origin/add_sub1 &&
489493
test_git_directory_is_unchanged sub1 &&
490494
test_submodule_content sub1 origin/add_sub1
@@ -559,15 +563,28 @@ test_submodule_switch_common() {
559563
# conditions, set the appropriate KNOWN_FAILURE_* variable used in the tests
560564
# below to 1.
561565
#
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.
563576
#
564577
# 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...
568585
# }
569-
# test_submodule_switch "my_func"
570-
test_submodule_switch () {
586+
# test_submodule_switch_func "my_func"
587+
test_submodule_switch_func () {
571588
command="$1"
572589
test_submodule_switch_common "$command"
573590

@@ -580,17 +597,33 @@ test_submodule_switch () {
580597
cd submodule_update &&
581598
git branch -t add_sub1 origin/add_sub1 &&
582599
>sub1 &&
583-
test_must_fail $command add_sub1 &&
600+
$command add_sub1 test_must_fail &&
584601
test_superproject_content origin/no_submodule &&
585602
test_must_be_empty sub1
586603
)
587604
'
588605
}
589606

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 () {
618+
gitcmd="$1"
619+
test_submodule_switch_func "git_test_func"
620+
}
621+
590622
# Same as test_submodule_switch(), except that throwing away local changes in
591623
# the superproject is allowed.
592624
test_submodule_forced_switch () {
593-
command="$1"
625+
gitcmd="$1"
626+
command="git_test_func"
594627
KNOWN_FAILURE_FORCED_SWITCH_TESTS=1
595628
test_submodule_switch_common "$command"
596629

@@ -631,8 +664,8 @@ test_submodule_forced_switch () {
631664

632665
# Internal function; use test_submodule_switch_recursing_with_args() or
633666
# test_submodule_forced_switch_recursing_with_args() instead.
634-
test_submodule_recursing_with_args_common() {
635-
command="$1"
667+
test_submodule_recursing_with_args_common () {
668+
command="$1 --recurse-submodules"
636669

637670
######################### Appearing submodule #########################
638671
# Switching to a commit letting a submodule appear checks it out ...
@@ -840,7 +873,7 @@ test_submodule_recursing_with_args_common() {
840873
# test_submodule_switch_recursing_with_args "$GIT_COMMAND"
841874
test_submodule_switch_recursing_with_args () {
842875
cmd_args="$1"
843-
command="git $cmd_args --recurse-submodules"
876+
command="git $cmd_args"
844877
test_submodule_recursing_with_args_common "$command"
845878

846879
RESULTDS=success
@@ -957,7 +990,7 @@ test_submodule_switch_recursing_with_args () {
957990
# away local changes in the superproject is allowed.
958991
test_submodule_forced_switch_recursing_with_args () {
959992
cmd_args="$1"
960-
command="git $cmd_args --recurse-submodules"
993+
command="git $cmd_args"
961994
test_submodule_recursing_with_args_common "$command"
962995

963996
RESULT=success

‎t/t1013-read-tree-submodule.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ test_submodule_switch_recursing_with_args "read-tree -u -m"
1212

1313
test_submodule_forced_switch_recursing_with_args "read-tree -u --reset"
1414

15-
test_submodule_switch "git read-tree -u -m"
15+
test_submodule_switch "read-tree -u -m"
1616

17-
test_submodule_forced_switch "git read-tree -u --reset"
17+
test_submodule_forced_switch "read-tree -u --reset"
1818

1919
test_done

‎t/t2013-checkout-submodule.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ test_submodule_switch_recursing_with_args "checkout"
6868

6969
test_submodule_forced_switch_recursing_with_args "checkout -f"
7070

71-
test_submodule_switch "git checkout"
71+
test_submodule_switch "checkout"
7272

73-
test_submodule_forced_switch "git checkout -f"
73+
test_submodule_forced_switch "checkout -f"
7474

7575
test_done

‎t/t3426-rebase-submodule.sh‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@ git_rebase () {
1717
git status -su >actual &&
1818
ls -1pR * >>actual &&
1919
test_cmp expect actual &&
20-
git rebase "$1"
20+
may_only_be_test_must_fail "$2" &&
21+
$2 git rebase "$1"
2122
}
2223

23-
test_submodule_switch "git_rebase"
24+
test_submodule_switch_func "git_rebase"
2425

2526
git_rebase_interactive () {
2627
git status -su >expect &&
@@ -35,10 +36,11 @@ git_rebase_interactive () {
3536
test_cmp expect actual &&
3637
set_fake_editor &&
3738
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"
3941
}
4042

41-
test_submodule_switch "git_rebase_interactive"
43+
test_submodule_switch_func "git_rebase_interactive"
4244

4345
test_expect_success 'rebase interactive ignores modified submodules' '
4446
test_when_finished "rm -rf super sub" &&

‎t/t3512-cherry-pick-submodule.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ test_description='cherry-pick can handle submodules'
77

88
KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
99
KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
10-
test_submodule_switch "git cherry-pick"
10+
test_submodule_switch "cherry-pick"
1111

1212
test_expect_success 'unrelated submodule/file conflict is ignored' '
1313
test_create_repo sub &&

‎t/t3513-revert-submodule.sh‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,12 @@ git_revert () {
1515
git status -su >expect &&
1616
ls -1pR * >>expect &&
1717
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 &&
1924
git revert HEAD &&
2025
rm -rf * &&
2126
tar xf "$TRASH_DIRECTORY/tmp.tar" &&
@@ -26,6 +31,6 @@ git_revert () {
2631
}
2732

2833
KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
29-
test_submodule_switch "git_revert"
34+
test_submodule_switch_func "git_revert"
3035

3136
test_done

‎t/t3906-stash-submodule.sh‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@ test_description='stash can handle submodules'
88
git_stash () {
99
git status -su >expect &&
1010
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 &&
1217
git stash &&
1318
git status -su >actual &&
1419
ls -1pR * >>actual &&
@@ -19,7 +24,7 @@ git_stash () {
1924
KNOWN_FAILURE_STASH_DOES_IGNORE_SUBMODULE_CHANGES=1
2025
KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT=1
2126
KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
22-
test_submodule_switch "git_stash"
27+
test_submodule_switch_func "git_stash"
2328

2429
setup_basic () {
2530
test_when_finished "rm -rf main sub" &&

‎t/t4137-apply-submodule.sh‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,19 @@ test_description='git apply handling submodules'
66
. "$TEST_DIRECTORY"/lib-submodule-update.sh
77

88
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
1012
}
1113

12-
test_submodule_switch "apply_index"
14+
test_submodule_switch_func "apply_index"
1315

1416
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
1620
}
1721

18-
test_submodule_switch "apply_3way"
22+
test_submodule_switch_func "apply_3way"
1923

2024
test_done

‎t/t4255-am-submodule.sh‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,21 @@ test_description='git am handling submodules'
66
. "$TEST_DIRECTORY"/lib-submodule-update.sh
77

88
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
1012
}
1113

12-
test_submodule_switch "am"
14+
test_submodule_switch_func "am"
1315

1416
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
1620
}
1721

1822
KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
19-
test_submodule_switch "am_3way"
23+
test_submodule_switch_func "am_3way"
2024

2125
test_expect_success 'setup diff.submodule' '
2226
test_commit one &&

‎t/t5572-pull-submodule.sh‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,34 +13,38 @@ reset_branch_to_HEAD () {
1313

1414
git_pull () {
1515
reset_branch_to_HEAD "$1" &&
16-
git pull
16+
may_only_be_test_must_fail "$2" &&
17+
$2 git pull
1718
}
1819

1920
# pulls without conflicts
20-
test_submodule_switch "git_pull"
21+
test_submodule_switch_func "git_pull"
2122

2223
git_pull_ff () {
2324
reset_branch_to_HEAD "$1" &&
24-
git pull --ff
25+
may_only_be_test_must_fail "$2" &&
26+
$2 git pull --ff
2527
}
2628

27-
test_submodule_switch "git_pull_ff"
29+
test_submodule_switch_func "git_pull_ff"
2830

2931
git_pull_ff_only () {
3032
reset_branch_to_HEAD "$1" &&
31-
git pull --ff-only
33+
may_only_be_test_must_fail "$2" &&
34+
$2 git pull --ff-only
3235
}
3336

34-
test_submodule_switch "git_pull_ff_only"
37+
test_submodule_switch_func "git_pull_ff_only"
3538

3639
git_pull_noff () {
3740
reset_branch_to_HEAD "$1" &&
38-
git pull --no-ff
41+
may_only_be_test_must_fail "$2" &&
42+
$2 git pull --no-ff
3943
}
4044

4145
KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
4246
KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
43-
test_submodule_switch "git_pull_noff"
47+
test_submodule_switch_func "git_pull_noff"
4448

4549
test_expect_success 'pull --recurse-submodule setup' '
4650
test_create_repo child &&

0 commit comments

Comments
 (0)