git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: git@vger.kernel.org,
	"Michael J Gruber" <git@drmicha.warpmail.net>,
	"Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH 2/2] test: cope better with use of return for errors
Date: Tue, 09 Aug 2011 10:46:34 +0200	[thread overview]
Message-ID: <4E40F3EA.8020406@viscovery.net> (raw)
In-Reply-To: <20110808011709.GC19551@elie.gateway.2wire.net>

Am 8/8/2011 3:17, schrieb Jonathan Nieder:
> +test_eval_ () {
> +	# This is a separate function because some tests use
> +	# "return" to end a test_expect_success block early.
> +	eval >&3 2>&4 "$*"
> +}
> +
>  test_run_ () {
>  	test_cleanup=:
>  	expecting_failure=$2
> -	eval >&3 2>&4 "$1"
> +	test_eval_ "$1"
>  	eval_ret=$?
>  
>  	if test -z "$immediate" || test $eval_ret = 0 || test -n "$expecting_failure"
>  	then
> -		eval >&3 2>&4 "$test_cleanup"
> +		test_eval_ "$test_cleanup"
>  	fi
>  	if test "$verbose" = "t" && test -n "$HARNESS_ACTIVE"; then
>  		echo ""

This invalidates at least t3900.29, which accesses an unexpanded $3
from the test script. The patch below fixes this case.

I tried to detect other cases by poisoning test_run_ like this:

-       test_eval_ "$1"
+       test_eval_ :\; :\; :\; :\; :\; "$1"

in the hopes that ":;" is an error at the place that uses $1, $2, etc.
t3900.2[89] are the only tests that are uncovered in this way.

I noticed this because I have a patched test-lib.sh that calls
test_eval_ in a similarly modified manner.

--- >8 ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] t3900: do not reference numbered arguments from the test
 script

The call to test_expect_success is nested inside a function, whose
arguments the test code wants to access. But it is not specified that any
unexpanded $1, $2, $3, etc in the test code will access the surrounding
function's arguments. Rather, they will access the arguments of the
function that happens to eval the test code.

Play safe by placing the argument in a named variable.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/t3900-i18n-commit.sh |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
index c06a5ee..3265fac 100755
--- a/t/t3900-i18n-commit.sh
+++ b/t/t3900-i18n-commit.sh
@@ -136,6 +136,7 @@ done
 test_commit_autosquash_flags () {
 	H=$1
 	flag=$2
+	mopt=$3
 	test_expect_success "commit --$flag with $H encoding" '
 		git config i18n.commitencoding $H &&
 		git checkout -b $H-$flag C0 &&
@@ -147,7 +148,7 @@ test_commit_autosquash_flags () {
 		git commit -a -m "intermediate commit" &&
 		test_tick &&
 		echo $H $flag >>F &&
-		git commit -a --$flag HEAD~1 $3 &&
+		git commit -a --$flag HEAD~1 $mopt &&
 		E=$(git cat-file commit '$H-$flag' |
 			sed -ne "s/^encoding //p") &&
 		test "z$E" = "z$H" &&
@@ -160,6 +161,6 @@ test_commit_autosquash_flags () {
 
 test_commit_autosquash_flags eucJP fixup
 
-test_commit_autosquash_flags ISO-2022-JP squash '-m "squash message"'
+test_commit_autosquash_flags ISO-2022-JP squash '-m squash_message'
 
 test_done
-- 
1.7.6.1618.gc932c

  reply	other threads:[~2011-08-09  8:46 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-17 11:33 [PATCH/RFD 0/2] revision.c: --merges, --no-merges and --merges-only Michael J Gruber
2011-03-17 11:33 ` [PATCH/RFD 1/2] revision.c: rename --merges to --merges-only Michael J Gruber
2011-03-17 11:33 ` [PATCH/RFD 2/2] revision.c: introduce --merges to undo --no-merges Michael J Gruber
2011-03-17 19:23 ` [PATCH/RFD 0/2] revision.c: --merges, --no-merges and --merges-only Junio C Hamano
2011-03-17 19:59 ` Jeff King
2011-03-18  7:56   ` Michael J Gruber
2011-03-18  8:22     ` Junio C Hamano
2011-03-18  8:41       ` Michael J Gruber
2011-03-18  8:56       ` Jeff King
2011-03-18 14:50         ` [PATCH 0/3] rev-list and friends: --min-parents, --max-parents Michael J Gruber
2011-03-18 14:50           ` [PATCH 1/3] revision.c: introduce --min-parents and --max-parents Michael J Gruber
2011-03-18 19:34             ` Jeff King
2011-03-21  7:31               ` Michael J Gruber
2011-03-18 20:48             ` Jonathan Nieder
2011-03-18 21:21               ` Junio C Hamano
2011-03-21  9:26               ` Michael J Gruber
2011-03-18 14:50           ` [PATCH 2/3] t6009: use test_commit() from test-lib.sh Michael J Gruber
2011-03-18 14:50           ` [PATCH 3/3] rev-list --min-parents,--max-parents: doc and test and completion Michael J Gruber
2011-03-18 19:48             ` Jeff King
2011-03-21  9:01               ` Michael J Gruber
2011-03-21 10:54                 ` Jeff King
2011-03-21 12:06                   ` Michael J Gruber
2011-03-21 14:54                     ` Junio C Hamano
2011-03-21 14:56                       ` Michael J Gruber
2011-03-21 16:47                         ` Junio C Hamano
2011-03-18 21:14             ` Jonathan Nieder
2011-03-21  8:52               ` Michael J Gruber
2011-03-21 17:49                 ` Jonathan Nieder
2011-03-22  7:38                   ` Michael J Gruber
2011-03-18 14:54           ` [PATCH 0/3] rev-list and friends: --min-parents, --max-parents Michael J Gruber
2011-03-18 19:41           ` Jeff King
2011-03-21  7:42             ` Michael J Gruber
2011-03-21 10:14               ` [PATCHv2 " Michael J Gruber
2011-03-21 10:14                 ` [PATCHv2 1/3] t6009: use test_commit() from test-lib.sh Michael J Gruber
2011-03-21 10:14                 ` [PATCHv2 2/3] revision.c: introduce --min-parents and --max-parents Michael J Gruber
2011-03-21 14:04                   ` Michael J Gruber
2011-03-21 17:45                   ` Junio C Hamano
2011-03-21 17:58                   ` Jonathan Nieder
2011-03-21 10:14                 ` [PATCHv2 3/3] rev-list --min-parents,--max-parents: doc and test and completion Michael J Gruber
2011-03-21 18:45                   ` Jonathan Nieder
2011-03-22  7:55                     ` Michael J Gruber
2011-03-23  0:47                       ` Jonathan Nieder
2011-03-21 10:56                 ` [PATCHv2 0/3] rev-list and friends: --min-parents, --max-parents Jeff King
2011-03-23  9:38                   ` [PATCHv3 0/5]rev-list " Michael J Gruber
2011-03-23  9:38                     ` [PATCHv3 1/5] t6009: use test_commit() from test-lib.sh Michael J Gruber
2011-03-23  9:38                     ` [PATCHv3 2/5] revision.c: introduce --min-parents and --max-parents Michael J Gruber
2011-03-23  9:38                     ` [PATCHv3 3/5] squash! " Michael J Gruber
2011-03-23  9:38                     ` [PATCHv3 4/5] rev-list --min-parents,--max-parents: doc, test and completion Michael J Gruber
2011-03-23  9:38                     ` [PATCHv3 5/5] fixup! " Michael J Gruber
2011-03-23 14:48                     ` [PATCHv3 0/5]rev-list and friends: --min-parents, --max-parents Jeff King
2011-03-23 17:12                       ` Junio C Hamano
2011-03-23 18:06                     ` Junio C Hamano
2011-03-24  8:21                     ` Jonathan Nieder
2011-03-24  8:55                       ` Michael J Gruber
2011-03-24  9:42                         ` Jonathan Nieder
2011-08-08  1:13                       ` [PATCH/RFC 0/2] test_when_finished and returning early Jonathan Nieder
2011-08-08  1:15                         ` [PATCH 1/2] test: simplify return value of test_run_ Jonathan Nieder
2011-08-08  1:17                         ` [PATCH 2/2] test: cope better with use of return for errors Jonathan Nieder
2011-08-09  8:46                           ` Johannes Sixt [this message]
2011-08-09 15:36                             ` Jeff King
2011-08-11  7:05                               ` [PATCH v2] t3900: do not reference numbered arguments from the test script Johannes Sixt
2011-08-11  7:11                                 ` Jonathan Nieder
2011-08-11 21:49                                   ` Junio C Hamano
2011-08-08  1:26                         ` [PATCH/RFC 0/2] test_when_finished and returning early Jeff King
2011-03-18  9:07     ` [PATCH/RFD 0/2] revision.c: --merges, --no-merges and --merges-only Jeff King
2011-03-18  9:42       ` Michael J Gruber
2011-03-18  9:54         ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E40F3EA.8020406@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=avarab@gmail.com \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).