Permalink
Browse files

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>
  • Loading branch information...
jrn authored and gitster committed Dec 14, 2011
1 parent 92058e4 commit 0d16451943ec4f17ea1503a12549531bb278788a
Showing with 17 additions and 56 deletions.
  1. +17 −56 t/t7006-pager.sh
View
@@ -6,11 +6,6 @@ test_description='Test automatic use of a pager.'
. "$TEST_DIRECTORY"/lib-pager.sh
. "$TEST_DIRECTORY"/lib-terminal.sh
-cleanup_fail() {
- echo >&2 cleanup failed
- (exit 1)
-}
-
test_expect_success 'setup' '
sane_unset GIT_PAGER GIT_PAGER_IN_USE &&
test_unconfig core.pager &&
@@ -22,9 +17,7 @@ test_expect_success 'setup' '
'
test_expect_success TTY 'some commands use a pager' '
- rm -f paginated.out ||
- cleanup_fail &&
-
+ rm -f paginated.out &&
test_terminal git log &&
test -e paginated.out
'
@@ -45,49 +38,37 @@ test_expect_failure TTY 'pager runs from subdir' '
'
test_expect_success TTY 'some commands do not use a pager' '
- rm -f paginated.out ||
- cleanup_fail &&
-
+ rm -f paginated.out &&
test_terminal git rev-list HEAD &&
! test -e paginated.out
'
test_expect_success 'no pager when stdout is a pipe' '
- rm -f paginated.out ||
- cleanup_fail &&
-
+ rm -f paginated.out &&
git log | cat &&
! test -e paginated.out
'
test_expect_success 'no pager when stdout is a regular file' '
- rm -f paginated.out ||
- cleanup_fail &&
-
+ rm -f paginated.out &&
git log >file &&
! test -e paginated.out
'
test_expect_success TTY 'git --paginate rev-list uses a pager' '
- rm -f paginated.out ||
- cleanup_fail &&
-
+ rm -f paginated.out &&
test_terminal git --paginate rev-list HEAD &&
test -e paginated.out
'
test_expect_success 'no pager even with --paginate when stdout is a pipe' '
- rm -f file paginated.out ||
- cleanup_fail &&
-
+ rm -f file paginated.out &&
git --paginate log | cat &&
! test -e paginated.out
'
test_expect_success TTY 'no pager with --no-pager' '
- rm -f paginated.out ||
- cleanup_fail &&
-
+ rm -f paginated.out &&
test_terminal git --no-pager log &&
! test -e paginated.out
'
@@ -136,9 +117,7 @@ colorful() {
}
test_expect_success 'tests can detect color' '
- rm -f colorful.log colorless.log ||
- cleanup_fail &&
-
+ rm -f colorful.log colorless.log &&
git log --no-color >colorless.log &&
git log --color >colorful.log &&
! colorful colorless.log &&
@@ -147,18 +126,14 @@ test_expect_success 'tests can detect color' '
test_expect_success 'no color when stdout is a regular file' '
rm -f colorless.log &&
- test_config color.ui auto ||
- cleanup_fail &&
-
+ test_config color.ui auto &&
git log >colorless.log &&
! colorful colorless.log
'
test_expect_success TTY 'color when writing to a pager' '
rm -f paginated.out &&
- test_config color.ui auto ||
- cleanup_fail &&
-
+ test_config color.ui auto &&
(
TERM=vt100 &&
export TERM &&
@@ -181,9 +156,7 @@ test_expect_success TTY 'colors are suppressed by color.pager' '
test_expect_success 'color when writing to a file intended for a pager' '
rm -f colorful.log &&
- test_config color.ui auto ||
- cleanup_fail &&
-
+ test_config color.ui auto &&
(
TERM=vt100 &&
GIT_PAGER_IN_USE=true &&
@@ -242,9 +215,7 @@ test_default_pager() {
$test_expectation SIMPLEPAGER,TTY "$cmd - default pager is used by default" "
sane_unset PAGER GIT_PAGER &&
test_unconfig core.pager &&
- rm -f default_pager_used ||
- cleanup_fail &&
-
+ rm -f default_pager_used &&
cat >\$less <<-\EOF &&
#!/bin/sh
wc >default_pager_used
@@ -265,9 +236,7 @@ test_PAGER_overrides() {
$test_expectation TTY "$cmd - PAGER overrides default pager" "
sane_unset GIT_PAGER &&
test_unconfig core.pager &&
- rm -f PAGER_used ||
- cleanup_fail &&
-
+ rm -f PAGER_used &&
PAGER='wc >PAGER_used' &&
export PAGER &&
$full_command &&
@@ -292,9 +261,7 @@ test_core_pager() {
$test_expectation TTY "$cmd - repository-local core.pager setting $used_if_wanted" "
sane_unset GIT_PAGER &&
- rm -f core.pager_used ||
- cleanup_fail &&
-
+ rm -f core.pager_used &&
PAGER=wc &&
export PAGER &&
test_config core.pager 'wc >core.pager_used' &&
@@ -321,9 +288,7 @@ test_pager_subdir_helper() {
$test_expectation TTY "$cmd - core.pager $used_if_wanted from subdirectory" "
sane_unset GIT_PAGER &&
rm -f core.pager_used &&
- rm -fr sub ||
- cleanup_fail &&
-
+ rm -fr sub &&
PAGER=wc &&
stampname=\$(pwd)/core.pager_used &&
export PAGER stampname &&
@@ -341,9 +306,7 @@ test_GIT_PAGER_overrides() {
parse_args "$@"
$test_expectation TTY "$cmd - GIT_PAGER overrides core.pager" "
- rm -f GIT_PAGER_used ||
- cleanup_fail &&
-
+ rm -f GIT_PAGER_used &&
test_config core.pager wc &&
GIT_PAGER='wc >GIT_PAGER_used' &&
export GIT_PAGER &&
@@ -356,9 +319,7 @@ test_doesnt_paginate() {
parse_args "$@"
$test_expectation TTY "no pager for '$cmd'" "
- rm -f GIT_PAGER_used ||
- cleanup_fail &&
-
+ rm -f GIT_PAGER_used &&
GIT_PAGER='wc >GIT_PAGER_used' &&
export GIT_PAGER &&
$full_command &&

0 comments on commit 0d16451

Please sign in to comment.