Re: [PATCH] Trapping exit in tests, using return for errors

[Junio C Hamano]

Pavel Roskin [EMAIL PROTECTED] writes:

 This patch does following:

 All instances of exit, exit 1 and (exit 1) in tests have been
 replaced with return 1.  In fact, (exit 1) had no effect.

Are you sure about all of the above?

You are right about ... || exit in the expect_success tests;
they are broken.  But '(exit 1)' or just saying 'false' at the
end should have done the right thing.  Worse yet, return does
not seem to really work as expected, except if all you want to
do was to tell the test driver I failed, in which case
bloopl would work just as well.

prompt$ cat k.sh
what=$1
eval '
echo foo
case '$what' in
false)
(exit 1) ;;
exit)
exit 1 ;;
return)
return 1 ;;
esac
'
echo status $?
prompt$ bash k.sh exit
foo
prompt$ bash k.sh false
foo
status 1
prompt$ bash k.sh return
foo
k.sh: line 20: return: can only `return' from a function or sourced script
status 1
prompt$ 

-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Trapping exit in tests, using return for errors

[Junio C Hamano]

Sorry, sent it out without finishing.  The worst is return.
With ksh, ash, and dash, the script itself exits with status
code 1 (presumably you are trapping it with trap -- exit,
though).

prompt$ bash k.sh exit
foo
prompt$ bash k.sh false
foo
status 1
prompt$ bash k.sh return
foo
k.sh: line 20: return: can only `return' from a function or sourced script
status 1
prompt$ ash k.sh exit
foo
prompt$ ash k.sh false
foo
status 1
prompt$ ash k.sh return
foo
prompt$ ksh k.sh exit
foo
prompt$ ksh k.sh false
foo
status 1
prompt$ ksh k.sh return
foo

-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Trapping exit in tests, using return for errors

[Junio C Hamano]

Junio C Hamano [EMAIL PROTECTED] writes:

 Sorry, sent it out without finishing.  The worst is return.

Ah, my mistake.  You have the eval that can eval return in a
function and let that return return from that function.
Cleverly done.

Thanks.

-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Trapping exit in tests, using return for errors

[Pavel Roskin]

Hi, Junio!

On Wed, 2005-08-10 at 23:31 -0700, Junio C Hamano wrote:
 Junio C Hamano [EMAIL PROTECTED] writes:
 
  Sorry, sent it out without finishing.  The worst is return.
 
 Ah, my mistake.  You have the eval that can eval return in a
 function and let that return return from that function.
 Cleverly done.

I'm glad you appreciate it.  One more fix on top of the last patch is
needed.

return from a test would leave the exit trap set, which could cause a
spurious error message if it's the last test in the script or
--immediate is used.

The easiest solution would be to have a global trap that is set when
test-lib.sh is sourced and unset either by test_done(), error() or by
test_failure_() with --immediate.  This patch also depends on the patch
that adds test_done() the the scripts that don't have it.

Signed-off-by: Pavel Roskin [EMAIL PROTECTED]

diff --git a/t/test-lib.sh b/t/test-lib.sh
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -36,6 +36,7 @@ unset SHA1_FILE_DIRECTORY
 
 error () {
echo * error: $*
+   trap - exit
exit 1
 }
 
@@ -74,6 +75,8 @@ fi
 test_failure=0
 test_count=0
 
+trap 'echo 5 FATAL: Unexpected exit with code $?; exit 1' exit
+
 
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.
@@ -89,7 +92,7 @@ test_failure_ () {
say FAIL $test_count: $1
shift
echo $@ | sed -e 's/^//'
-   test $immediate ==  || exit 1
+   test $immediate ==  || { trap - exit; exit 1; }
 }
 
 
@@ -98,10 +101,8 @@ test_debug () {
 }
 
 test_run_ () {
-   trap 'echo 5 FATAL: Unexpected exit with code $?; exit 1' exit
eval 3 24 $1
eval_ret=$?
-   trap - exit
return 0
 }
 
@@ -132,6 +133,7 @@ test_expect_success () {
 }
 
 test_done () {
+   trap - exit
case $test_failure in
0)  
# We could:


-- 
Regards,
Pavel Roskin

-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Trapping exit in tests, using return for errors

[Pavel Roskin]

Hello!

I have noticed that make test fails without any explanations when the
merge utility is missing.  I don't think tests should be silent in
case of failure.

It turned out that the particular test was using exit to interrupt the
test in case of an error.  This caused the whole test script to exit.
No further tests would be run even if --immediate wasn't specified.
No error message was printed.

This patch does following:

All instances of exit, exit 1 and (exit 1) in tests have been
replaced with return 1.  In fact, (exit 1) had no effect.

File descriptor 5 is duplicated from file descriptor 1.  This is needed
to print important error messages from tests.

New function test_run_() has been introduced.  Any return in the test
would merely cause that function to return without skipping calls to
test_failure_() and test_ok_().  The new function also traps exit and
treats it like a fatal error (in case somebody reintroduces exit in
the tests).

test_expect_failure() and test_expect_success() check both the result of
eval and the return value of test_run_().  If the later is not 0, it's
always a failure because it indicates the the test didn't complete.

Signed-off-by: Pavel Roskin [EMAIL PROTECTED]

diff --git a/t/t1001-read-tree-m-2way.sh b/t/t1001-read-tree-m-2way.sh
--- a/t/t1001-read-tree-m-2way.sh
+++ b/t/t1001-read-tree-m-2way.sh
@@ -100,7 +100,7 @@ test_expect_success \
  git-checkout-cache -u -f -q -a 
  git-update-cache --add yomin 
  read_tree_twoway $treeH $treeM 
- git-ls-files --stage 4.out || exit
+ git-ls-files --stage 4.out || return 1
  diff -u M.out 4.out 4diff.out
  compare_change 4diff.out expected 
  check_cache_at yomin clean'
@@ -114,7 +114,7 @@ test_expect_success \
  git-update-cache --add yomin 
  echo yomin yomin yomin 
  read_tree_twoway $treeH $treeM 
- git-ls-files --stage 5.out || exit
+ git-ls-files --stage 5.out || return 1
  diff -u M.out 5.out 5diff.out
  compare_change 5diff.out expected 
  check_cache_at yomin dirty'
@@ -215,7 +215,7 @@ test_expect_success \
  echo nitfol nitfol nitfol 
  git-update-cache --add nitfol 
  read_tree_twoway $treeH $treeM 
- git-ls-files --stage 14.out || exit
+ git-ls-files --stage 14.out || return 1
  diff -u M.out 14.out 14diff.out
  compare_change 14diff.out expected 
  check_cache_at nitfol clean'
@@ -229,7 +229,7 @@ test_expect_success \
  git-update-cache --add nitfol 
  echo nitfol nitfol nitfol nitfol 
  read_tree_twoway $treeH $treeM 
- git-ls-files --stage 15.out || exit
+ git-ls-files --stage 15.out || return 1
  diff -u M.out 15.out 15diff.out
  compare_change 15diff.out expected 
  check_cache_at nitfol dirty'
diff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh
--- a/t/t1002-read-tree-m-u-2way.sh
+++ b/t/t1002-read-tree-m-u-2way.sh
@@ -73,7 +73,7 @@ test_expect_success \
 'rm -f .git/index 
  git-update-cache --add yomin 
  git-read-tree -m -u $treeH $treeM 
- git-ls-files --stage 4.out || exit
+ git-ls-files --stage 4.out || return 1
  diff --unified=0 M.out 4.out 4diff.out
  compare_change 4diff.out expected 
  check_cache_at yomin clean 
@@ -90,7 +90,7 @@ test_expect_success \
  git-update-cache --add yomin 
  echo yomin yomin yomin 
  git-read-tree -m -u $treeH $treeM 
- git-ls-files --stage 5.out || exit
+ git-ls-files --stage 5.out || return 1
  diff --unified=0 M.out 5.out 5diff.out
  compare_change 5diff.out expected 
  check_cache_at yomin dirty 
@@ -192,7 +192,7 @@ test_expect_success \
  echo nitfol nitfol nitfol 
  git-update-cache --add nitfol 
  git-read-tree -m -u $treeH $treeM 
- git-ls-files --stage 14.out || exit
+ git-ls-files --stage 14.out || return 1
  diff --unified=0 M.out 14.out 14diff.out
  compare_change 14diff.out expected 
  sum bozbar frotz actual14.sum 
@@ -212,7 +212,7 @@ test_expect_success \
  git-update-cache --add nitfol 
  echo nitfol nitfol nitfol nitfol 
  git-read-tree -m -u $treeH $treeM 
- git-ls-files --stage 15.out || exit
+ git-ls-files --stage 15.out || return 1
  diff --unified=0 M.out 15.out 15diff.out
  compare_change 15diff.out expected 
  check_cache_at nitfol dirty 
diff --git a/t/t1005-read-tree-m-2way-emu23.sh 
b/t/t1005-read-tree-m-2way-emu23.sh
--- a/t/t1005-read-tree-m-2way-emu23.sh
+++ b/t/t1005-read-tree-m-2way-emu23.sh
@@ -120,7 +120,7 @@ test_expect_success \
  git-checkout-cache -u -f -q -a 
  git-update-cache --add yomin 
  read_tree_twoway $treeH $treeM 
- git-ls-files --stage 4.out || exit
+ git-ls-files --stage 4.out || return 1
  diff -u M.out 4.out 4diff.out
  compare_change 4diff.out expected 
  check_cache_at yomin clean'
@@ -136,7 +136,7 @@ test_expect_success \
  git-update-cache --add yomin 
  echo yomin yomin yomin 
  read_tree_twoway