test: errors preparing for a test are not special

This script uses the following idiom to start each test in a known good state: test_expect_success 'some commands use a pager' ' rm -f paginated.out || cleanup_fail && test_terminal git log && test -e paginated.out ' where "cleanup_fail" is a function that prints an error message and errors out. That is bogus on three levels: - Cleanup commands like "rm -f" and "test_unconfig" are designed not to fail, so this logic would never trip. - If they were to malfunction anyway, it is not useful to set apart cleanup commands as a special kind of failure with a special error message. Whichever command fails, the next step is to investigate which command that was, for example by running tests with "prove -e 'sh -x'", and fix it. - Relying on left-associativity of mixed &&/|| lists makes the code somewhat cryptic. The fix is simple: drop the "|| cleanup_fail" in each test and the definition of the "cleanup_fail" function so no new callers can arise. Reported-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Nieder committed Dec 14, 2011 at 02:22 UTC 0d16451943ec4f17ea1503a12549531bb278788a
1 file changed +17 -56
t/t7006-pager.sh
+17 -56
@@ -6,11 +6,6 @@ test_description='Test automatic use of a pager.'
6 . "$TEST_DIRECTORY"/lib-pager.sh
7 . "$TEST_DIRECTORY"/lib-terminal.sh
8
9 -cleanup_fail() {
10 - echo >&2 cleanup failed
11 - (exit 1)
12 -}
13 -
9 test_expect_success 'setup' '
10 sane_unset GIT_PAGER GIT_PAGER_IN_USE &&
11 test_unconfig core.pager &&
@@ -22,9 +17,7 @@ test_expect_success 'setup' '
17 '
18
19 test_expect_success TTY 'some commands use a pager' '
25 - rm -f paginated.out ||
26 - cleanup_fail &&
27 -
20 + rm -f paginated.out &&
21 test_terminal git log &&
22 test -e paginated.out
23 '
@@ -45,49 +38,37 @@ test_expect_failure TTY 'pager runs from subdir' '
38 '
39
40 test_expect_success TTY 'some commands do not use a pager' '
48 - rm -f paginated.out ||
49 - cleanup_fail &&
50 -
41 + rm -f paginated.out &&
42 test_terminal git rev-list HEAD &&
43 ! test -e paginated.out
44 '
45
46 test_expect_success 'no pager when stdout is a pipe' '
56 - rm -f paginated.out ||
57 - cleanup_fail &&
58 -
47 + rm -f paginated.out &&
48 git log | cat &&
49 ! test -e paginated.out
50 '
51
52 test_expect_success 'no pager when stdout is a regular file' '
64 - rm -f paginated.out ||
65 - cleanup_fail &&
66 -
53 + rm -f paginated.out &&
54 git log >file &&
55 ! test -e paginated.out
56 '
57
58 test_expect_success TTY 'git --paginate rev-list uses a pager' '
72 - rm -f paginated.out ||
73 - cleanup_fail &&
74 -
59 + rm -f paginated.out &&
60 test_terminal git --paginate rev-list HEAD &&
61 test -e paginated.out
62 '
63
64 test_expect_success 'no pager even with --paginate when stdout is a pipe' '
80 - rm -f file paginated.out ||
81 - cleanup_fail &&
82 -
65 + rm -f file paginated.out &&
66 git --paginate log | cat &&
67 ! test -e paginated.out
68 '
69
70 test_expect_success TTY 'no pager with --no-pager' '
88 - rm -f paginated.out ||
89 - cleanup_fail &&
90 -
71 + rm -f paginated.out &&
72 test_terminal git --no-pager log &&
73 ! test -e paginated.out
74 '
@@ -136,9 +117,7 @@ colorful() {
117 }
118
119 test_expect_success 'tests can detect color' '
139 - rm -f colorful.log colorless.log ||
140 - cleanup_fail &&
141 -
120 + rm -f colorful.log colorless.log &&
121 git log --no-color >colorless.log &&
122 git log --color >colorful.log &&
123 ! colorful colorless.log &&
@@ -147,18 +126,14 @@ test_expect_success 'tests can detect color' '
126
127 test_expect_success 'no color when stdout is a regular file' '
128 rm -f colorless.log &&
150 - test_config color.ui auto ||
151 - cleanup_fail &&
152 -
129 + test_config color.ui auto &&
130 git log >colorless.log &&
131 ! colorful colorless.log
132 '
133
134 test_expect_success TTY 'color when writing to a pager' '
135 rm -f paginated.out &&
159 - test_config color.ui auto ||
160 - cleanup_fail &&
161 -
136 + test_config color.ui auto &&
137 (
138 TERM=vt100 &&
139 export TERM &&
@@ -181,9 +156,7 @@ test_expect_success TTY 'colors are suppressed by color.pager' '
156
157 test_expect_success 'color when writing to a file intended for a pager' '
158 rm -f colorful.log &&
184 - test_config color.ui auto ||
185 - cleanup_fail &&
186 -
159 + test_config color.ui auto &&
160 (
161 TERM=vt100 &&
162 GIT_PAGER_IN_USE=true &&
@@ -242,9 +215,7 @@ test_default_pager() {
215 $test_expectation SIMPLEPAGER,TTY "$cmd - default pager is used by default" "
216 sane_unset PAGER GIT_PAGER &&
217 test_unconfig core.pager &&
245 - rm -f default_pager_used ||
246 - cleanup_fail &&
247 -
218 + rm -f default_pager_used &&
219 cat >\$less <<-\EOF &&
220 #!/bin/sh
221 wc >default_pager_used
@@ -265,9 +236,7 @@ test_PAGER_overrides() {
236 $test_expectation TTY "$cmd - PAGER overrides default pager" "
237 sane_unset GIT_PAGER &&
238 test_unconfig core.pager &&
268 - rm -f PAGER_used ||
269 - cleanup_fail &&
270 -
239 + rm -f PAGER_used &&
240 PAGER='wc >PAGER_used' &&
241 export PAGER &&
242 $full_command &&
@@ -292,9 +261,7 @@ test_core_pager() {
261
262 $test_expectation TTY "$cmd - repository-local core.pager setting $used_if_wanted" "
263 sane_unset GIT_PAGER &&
295 - rm -f core.pager_used ||
296 - cleanup_fail &&
297 -
264 + rm -f core.pager_used &&
265 PAGER=wc &&
266 export PAGER &&
267 test_config core.pager 'wc >core.pager_used' &&
@@ -321,9 +288,7 @@ test_pager_subdir_helper() {
288 $test_expectation TTY "$cmd - core.pager $used_if_wanted from subdirectory" "
289 sane_unset GIT_PAGER &&
290 rm -f core.pager_used &&
324 - rm -fr sub ||
325 - cleanup_fail &&
326 -
291 + rm -fr sub &&
292 PAGER=wc &&
293 stampname=\$(pwd)/core.pager_used &&
294 export PAGER stampname &&
@@ -341,9 +306,7 @@ test_GIT_PAGER_overrides() {
306 parse_args "$@"
307
308 $test_expectation TTY "$cmd - GIT_PAGER overrides core.pager" "
344 - rm -f GIT_PAGER_used ||
345 - cleanup_fail &&
346 -
309 + rm -f GIT_PAGER_used &&
310 test_config core.pager wc &&
311 GIT_PAGER='wc >GIT_PAGER_used' &&
312 export GIT_PAGER &&
@@ -356,9 +319,7 @@ test_doesnt_paginate() {
319 parse_args "$@"
320
321 $test_expectation TTY "no pager for '$cmd'" "
359 - rm -f GIT_PAGER_used ||
360 - cleanup_fail &&
361 -
322 + rm -f GIT_PAGER_used &&
323 GIT_PAGER='wc >GIT_PAGER_used' &&
324 export GIT_PAGER &&
325 $full_command &&