git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Sipek <jeffpc@josefsipek.net>
To: Per Cederqvist <cederp@opera.com>
Cc: git@vger.kernel.org
Subject: Re: [GUILT 10/28] Run test_failed if the exit status of a test script is bad.
Date: Tue, 6 May 2014 15:56:53 -0400	[thread overview]
Message-ID: <20140506195653.GL1655@meili.valhalla.31bits.net> (raw)
In-Reply-To: <1395387126-13681-11-git-send-email-cederp@opera.com>

On Fri, Mar 21, 2014 at 08:31:48AM +0100, Per Cederqvist wrote:
> There were two problems with the old code:
> 
>  - Since "set -e" is in effect (that is set in scaffold) the run-test
>    script exited immediately if a t-*.sh script failed.  This is not
>    nice, as we want the error report that test_failed prints.
> 
>  - The code ran "cd -" between running the t-*.sh script and checking
>    the exit status, so the exit status was lost.  (Actually, the exit
>    status was saved in $ERR, but nothing ever looked at $ERR.)

Oops :)

> Signed-off-by: Per Cederqvist <cederp@opera.com>
> ---
>  regression/run-tests | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/regression/run-tests b/regression/run-tests
> index a10e796..d39f9ef 100755
> --- a/regression/run-tests
> +++ b/regression/run-tests
> @@ -55,11 +55,16 @@ function run_test
>  
>  	# run the test
>  	cd "$REPODIR" > /dev/null
> -	"$REG_DIR/t-$1.sh" 2>&1 > "$LOGFILE"
> -	ERR=$?
> +	if "$REG_DIR/t-$1.sh" 2>&1 > "$LOGFILE"
> +	then
> +	    ERR=false
> +	else
> +	    ERR=true

I'm going to comment on this here... Coding style.  Guilt is a bit of a
hodge-podge of style as my personal style for shell changed over the years
and various contributors threw in some more.  I need to get better at
spotting style mismatch during review.  With that said, I have two comments
about the above:

(1) I'd put the 'then' on the same line as 'if' but I don't feel strongly
    enough about this to reject this patch.

(2) Tabs for indentation.  I do feel strongly about this one :)

Jeff.

> +	fi
> +
>  	cd - > /dev/null
>  
> -	[ $? -ne 0 ] && test_failed
> +	$ERR && test_failed
>  	diff -u "t-$1.out" "$LOGFILE" || test_failed
>  
>  	echo "done."
> -- 
> 1.8.3.1
> 

-- 
I'm somewhere between geek and normal.
		- Linus Torvalds

  reply	other threads:[~2014-05-06 19:57 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-21  7:31 [GUILT 00/28] Teach guilt import-commit how create legal patch names, and more Per Cederqvist
2014-03-21  7:31 ` [GUILT 01/28] The tests should not fail if guilt.diffstat is set Per Cederqvist
2014-03-21  7:31 ` [GUILT 02/28] Allow "guilt delete -f" to run from a dir which contains spaces Per Cederqvist
2014-03-21  7:31 ` [GUILT 03/28] Added test case for "guilt delete -f" Per Cederqvist
2014-03-21  7:31 ` [GUILT 04/28] Allow "guilt import-commit" to run from a dir which contains spaces Per Cederqvist
2014-03-23 17:04   ` Jeff Sipek
2014-03-23 19:57     ` Per Cederqvist
2014-03-23 20:07       ` Jeff Sipek
2014-03-23 21:13         ` Per Cederqvist
2014-05-06 18:27           ` Jeff Sipek
2014-03-21  7:31 ` [GUILT 05/28] "guilt new": Accept more than 4 arguments Per Cederqvist
2014-03-21  7:31 ` [GUILT 06/28] Fix and simplify the do_get_patch function Per Cederqvist
2014-03-23 17:09   ` Jeff Sipek
2014-03-23 21:03     ` Per Cederqvist
2014-05-06 19:08       ` Jeff Sipek
2014-05-07 20:44         ` Per Cederqvist
2014-03-21  7:31 ` [GUILT 07/28] Added test cases for "guilt fold" Per Cederqvist
2014-05-06 19:40   ` Jeff Sipek
2014-05-07 20:59     ` Per Cederqvist
2014-05-07 21:06       ` Jeff Sipek
2014-05-08 19:41         ` Per Cederqvist
2014-05-08 19:59           ` Jeff Sipek
2014-03-21  7:31 ` [GUILT 08/28] Added more test cases for "guilt new": empty patches Per Cederqvist
2014-03-21  7:31 ` [GUILT 09/28] Test suite: properly check the exit status of commands Per Cederqvist
2014-03-21  7:31 ` [GUILT 10/28] Run test_failed if the exit status of a test script is bad Per Cederqvist
2014-05-06 19:56   ` Jeff Sipek [this message]
2014-03-21  7:31 ` [GUILT 11/28] test suite: remove pointless redirection Per Cederqvist
2014-05-06 19:57   ` Jeff Sipek
2014-03-21  7:31 ` [GUILT 12/28] "guilt header": more robust header selection Per Cederqvist
2014-03-21  7:31 ` [GUILT 13/28] Check that "guilt header '.*'" fails Per Cederqvist
2014-03-21  7:31 ` [GUILT 14/28] Use "git check-ref-format" to validate patch names Per Cederqvist
2014-03-21  7:31 ` [GUILT 15/28] Produce legal patch names in guilt-import-commit Per Cederqvist
2014-03-21  7:31 ` [GUILT 16/28] Fix backslash handling when creating names of imported patches Per Cederqvist
2014-03-21  7:31 ` [GUILT 17/28] "guilt graph" no longer loops when no patches are applied Per Cederqvist
2014-05-06 20:10   ` Jeff Sipek
2014-03-21  7:31 ` [GUILT 18/28] guilt-graph: Handle commas in branch names Per Cederqvist
2014-05-06 20:13   ` Jeff Sipek
2014-03-21  7:31 ` [GUILT 19/28] Check that "guilt graph" works when working on a branch with a comma Per Cederqvist
2014-05-06 20:15   ` Jeff Sipek
2014-05-13 21:33   ` Jeff Sipek
2014-05-13 21:35     ` Jeff Sipek
2014-03-21  7:31 ` [GUILT 20/28] "guilt graph": Handle patch names containing quotes Per Cederqvist
2014-03-21  7:57   ` Eric Sunshine
2014-05-06 20:24     ` Jeff Sipek
2014-05-08 20:18       ` Per Cederqvist
2014-03-21  7:31 ` [GUILT 21/28] The log.decorate setting should not influence import-commit Per Cederqvist
2014-05-06 20:41   ` Jeff Sipek
2014-03-21  7:32 ` [GUILT 22/28] The log.decorate setting should not influence patchbomb Per Cederqvist
2014-05-06 20:41   ` Jeff Sipek
2014-03-21  7:32 ` [GUILT 23/28] The log.decorate setting should not influence guilt rebase Per Cederqvist
2014-05-06 20:41   ` Jeff Sipek
2014-03-21  7:32 ` [GUILT 24/28] disp no longer processes backslashes Per Cederqvist
2014-05-07 21:15   ` Jeff Sipek
2014-03-21  7:32 ` [GUILT 25/28] "guilt push" now fails when there are no more patches to push Per Cederqvist
2014-05-07 22:04   ` Jeff Sipek
2014-05-08 21:37     ` Per Cederqvist
2014-03-21  7:32 ` [GUILT 26/28] "guilt pop" now fails when there are no more patches to pop Per Cederqvist
2014-03-21  7:32 ` [GUILT 27/28] Minor testsuite fix Per Cederqvist
2014-05-07 21:09   ` Jeff Sipek
2014-03-21  7:32 ` [GUILT 28/28] Added guilt.reusebranch configuration option Per Cederqvist
2014-03-21 20:39 ` [GUILT 00/28] Teach guilt import-commit how create legal patch names, and more Jeff Sipek

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=20140506195653.GL1655@meili.valhalla.31bits.net \
    --to=jeffpc@josefsipek.net \
    --cc=cederp@opera.com \
    --cc=git@vger.kernel.org \
    /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).