Skip to content

Commit e163cff

Browse files
gitstergitster
committed
Merge branch 'dl/test-must-fail-fixes-6' into master
Dev support to limit the use of test_must_fail to only git commands. * dl/test-must-fail-fixes-6: test-lib-functions: restrict test_must_fail usage t9400: don't use test_must_fail with cvs t9834: remove use of `test_might_fail p4` t7107: don't use test_must_fail() t5324: reorder `run_with_limited_open_files test_might_fail` t3701: stop using `env` in force_color()
2 parents c28a2d0 + 6a67c75 commit e163cff

File tree

7 files changed

+86
-7
lines changed

7 files changed

+86
-7
lines changed

‎t/t0000-basic.sh‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,4 +1271,22 @@ test_expect_success 'very long name in the index handled sanely' '
12711271
test $len = 4098
12721272
'
12731273

1274+
test_expect_success 'test_must_fail on a failing git command' '
1275+
test_must_fail git notacommand
1276+
'
1277+
1278+
test_expect_success 'test_must_fail on a failing git command with env' '
1279+
test_must_fail env var1=a var2=b git notacommand
1280+
'
1281+
1282+
test_expect_success 'test_must_fail rejects a non-git command' '
1283+
! test_must_fail grep ^$ notafile 2>err &&
1284+
grep -F "test_must_fail: only '"'"'git'"'"' is allowed" err
1285+
'
1286+
1287+
test_expect_success 'test_must_fail rejects a non-git command with env' '
1288+
! test_must_fail env var1=a var2=b grep ^$ notafile 2>err &&
1289+
grep -F "test_must_fail: only '"'"'git'"'"' is allowed" err
1290+
'
1291+
12741292
test_done

‎t/t3701-add-interactive.sh‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,16 @@ diff_cmp () {
3131
# indicates a dumb terminal, so we set that variable, too.
3232

3333
force_color () {
34-
env GIT_PAGER_IN_USE=true TERM=vt100 "$@"
34+
# The first element of $@ may be a shell function, as a result POSIX
35+
# does not guarantee that "one-shot assignment" will not persist after
36+
# the function call. Thus, we prevent these variables from escaping
37+
# this function's context with this subshell.
38+
(
39+
GIT_PAGER_IN_USE=true &&
40+
TERM=vt100 &&
41+
export GIT_PAGER_IN_USE TERM &&
42+
"$@"
43+
)
3544
}
3645

3746
test_expect_success 'setup (initial)' '
@@ -604,7 +613,7 @@ test_expect_success 'detect bogus diffFilter output' '
604613
echo content >test &&
605614
test_config interactive.diffFilter "sed 1d" &&
606615
printf y >y &&
607-
test_must_fail force_color git add -p <y
616+
force_color test_must_fail git add -p <y
608617
'
609618

610619
test_expect_success 'diff.algorithm is passed to `git diff-files`' '

‎t/t5324-split-commit-graph.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ test_expect_success ULIMIT_FILE_DESCRIPTORS 'handles file descriptor exhaustion'
399399
for i in $(test_seq 64)
400400
do
401401
test_commit $i &&
402-
test_might_fail run_with_limited_open_files git commit-graph write \
402+
run_with_limited_open_files test_might_fail git commit-graph write \
403403
--split=no-merge --reachable || return 1
404404
done
405405
)

‎t/t7107-reset-pathspec-file.sh‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,12 @@ restore_checkpoint () {
2222

2323
verify_expect () {
2424
git status --porcelain -- fileA.t fileB.t fileC.t fileD.t >actual &&
25-
test_cmp expect actual
25+
if test "x$1" = 'x!'
26+
then
27+
! test_cmp expect actual
28+
else
29+
test_cmp expect actual
30+
fi
2631
}
2732

2833
test_expect_success '--pathspec-from-file from stdin' '
@@ -131,7 +136,7 @@ test_expect_success 'quotes not compatible with --pathspec-file-nul' '
131136
cat >expect <<-\EOF &&
132137
D fileA.t
133138
EOF
134-
test_must_fail verify_expect
139+
verify_expect !
135140
'
136141

137142
test_expect_success 'only touches what was listed' '

‎t/t9400-git-cvsserver-server.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ test_expect_success 'cvs server does not run with vanilla git-shell' '
603603
cd cvswork &&
604604
CVS_SERVER=$WORKDIR/remote-cvs &&
605605
export CVS_SERVER &&
606-
test_must_fail cvs log merge
606+
! cvs log merge
607607
)
608608
'
609609

‎t/t9834-git-p4-file-dir-bug.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ repository.'
1010

1111
test_expect_success 'start p4d' '
1212
start_p4d &&
13-
test_might_fail p4 configure set submit.collision.check=0
13+
{ p4 configure set submit.collision.check=0 || :; }
1414
'
1515

1616
test_expect_success 'init depot' '

‎t/test-lib-functions.sh‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -798,6 +798,37 @@ list_contains () {
798798
return 1
799799
}
800800

801+
# Returns success if the arguments indicate that a command should be
802+
# accepted by test_must_fail(). If the command is run with env, the env
803+
# and its corresponding variable settings will be stripped before we
804+
# test the command being run.
805+
test_must_fail_acceptable () {
806+
if test "$1" = "env"
807+
then
808+
shift
809+
while test $# -gt 0
810+
do
811+
case "$1" in
812+
*?=*)
813+
shift
814+
;;
815+
*)
816+
break
817+
;;
818+
esac
819+
done
820+
fi
821+
822+
case "$1" in
823+
git|__git*|test-tool|test-svn-fe|test_terminal)
824+
return 0
825+
;;
826+
*)
827+
return 1
828+
;;
829+
esac
830+
}
831+
801832
# This is not among top-level (test_expect_success | test_expect_failure)
802833
# but is a prefix that can be used in the test script, like:
803834
#
@@ -817,6 +848,17 @@ list_contains () {
817848
# Multiple signals can be specified as a comma separated list.
818849
# Currently recognized signal names are: sigpipe, success.
819850
# (Don't use 'success', use 'test_might_fail' instead.)
851+
#
852+
# Do not use this to run anything but "git" and other specific testable
853+
# commands (see test_must_fail_acceptable()). We are not in the
854+
# business of vetting system supplied commands -- in other words, this
855+
# is wrong:
856+
#
857+
# test_must_fail grep pattern output
858+
#
859+
# Instead use '!':
860+
#
861+
# ! grep pattern output
820862

821863
test_must_fail () {
822864
case "$1" in
@@ -828,6 +870,11 @@ test_must_fail () {
828870
_test_ok=
829871
;;
830872
esac
873+
if ! test_must_fail_acceptable "$@"
874+
then
875+
echo >&7 "test_must_fail: only 'git' is allowed: $*"
876+
return 1
877+
fi
831878
"$@" 2>&7
832879
exit_code=$?
833880
if test $exit_code -eq 0 && ! list_contains "$_test_ok" success

0 commit comments

Comments
 (0)