git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: git@vger.kernel.org
Cc: "Jeff King" <peff@peff.net>, "SZEDER Gábor" <szeder.dev@gmail.com>
Subject: [PATCH 2/3] t: teach 'test_must_fail' to save the command's stderr to a file
Date: Fri,  9 Feb 2018 03:42:34 +0100	[thread overview]
Message-ID: <20180209024235.3431-3-szeder.dev@gmail.com> (raw)
In-Reply-To: <20180209024235.3431-1-szeder.dev@gmail.com>

To check that a git command fails and to inspect its error message we
usually execute a command like this throughout our test suite:

  test_must_fail git command --option 2>output.err

Note that this command doesn't limit the redirection to the git
command, but it redirects the standard error of the 'test_must_fail'
helper function as well.  This is wrong for several reasons:

  - If that git command were to succeed or die for an unexpected
    reason e.g. signal, then 'test_must_fail's helpful error message
    would end up in the given file instead of on the terminal or in
    't/test-results/$T.out', when the test is run with '-v' or
    '--verbose-log', respectively.

  - If the test is run with '-x', the trace of executed commands in
    'test_must_fail' will go to stderr as well (except for more recent
    Bash versions supporting $BASH_XTRACEFD), and then in turn will
    end up in the given file.

  - If the git command's error message is checked verbatim with
    'test_cmp', then the '-x' trace will cause the test to fail.

  - Though rather unlikely, if the git command's error message is
    checked with 'test_i18ngrep', then its pattern might match
    'test_must_fail's error message (or '-x' trace) instead of the
    given git command's, potentially hiding a bug.

Teach the 'test_must_fail' helper the 'stderr=<file>' option to save
the executed git command's standard error to that file, so we can
avoid all the bad side effects of redirecting the whole thing's
standard error.

The same issues apply to the 'test_might_fail' helper function as
well, but since it's implemented as a thin wrapper around
'test_must_fail', no modifications were necessary and 'test_might_fail
stderr=output.err git command' will just work.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---

Notes:
    I considered adding an analogous 'stdout=<file>' option as well, but in
    the end haven't, because:
    
      - 'test_must_fail' doesn't print anything to its standard output, so a
        plain '>out' redirection at the end of the line doesn't have any
        undesirable side effects.
    
      - We would need four conditions to cover all possible combinations of
        'stdout=' and 'stderr='.  Considering the above point, I don't think
        it's worth it.

 t/README                |  6 ++++++
 t/test-lib-functions.sh | 35 +++++++++++++++++++++++++----------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/t/README b/t/README
index 1a1361a806..2528359e9d 100644
--- a/t/README
+++ b/t/README
@@ -430,6 +430,10 @@ Don't:
    use 'test_must_fail git cmd'.  This will signal a failure if git
    dies in an unexpected way (e.g. segfault).
 
+   Don't redirect 'test_must_fail's standard error like
+   'test_must_fail git cmd 2>err'.  Instead, run 'test_must_fail
+   stderr=err git cmd'.
+
    On the other hand, don't use test_must_fail for running regular
    platform commands; just use '! cmd'.  We are not in the business
    of verifying that the world given to us sanely works.
@@ -670,6 +674,8 @@ library for your script to use.
        Multiple signals can be specified as a comma separated list.
        Currently recognized signal names are: sigpipe, success.
        (Don't use 'success', use 'test_might_fail' instead.)
+     stderr=<file>:
+       Save <git-command>'s standard error to <file>.
 
  - test_might_fail [<options>] <git-command>
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 26b149ac1d..d2f561477c 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -618,18 +618,33 @@ list_contains () {
 #     Multiple signals can be specified as a comma separated list.
 #     Currently recognized signal names are: sigpipe, success.
 #     (Don't use 'success', use 'test_might_fail' instead.)
+#   stderr=<file>:
+#     Save the git command's standard error to <file>.
 
 test_must_fail () {
-	case "$1" in
-	ok=*)
-		_test_ok=${1#ok=}
-		shift
-		;;
-	*)
-		_test_ok=
-		;;
-	esac
-	"$@"
+	_test_ok= _test_stderr=
+	while test $# -gt 0
+	do
+		case "$1" in
+		stderr=*)
+			_test_stderr=${1#stderr=}
+			shift
+			;;
+		ok=*)
+			_test_ok=${1#ok=}
+			shift
+			;;
+		*)
+			break
+			;;
+		esac
+	done
+	if test -n "$_test_stderr"
+	then
+		"$@" 2>"$_test_stderr"
+	else
+		"$@"
+	fi
 	exit_code=$?
 	if test $exit_code -eq 0 && ! list_contains "$_test_ok" success
 	then
-- 
2.16.1.180.g07550b0b1b


  parent reply	other threads:[~2018-02-09  2:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-09  2:42 [PATCH 0/3] Teach 'test_must_fail' to save the command's stderr to a file SZEDER Gábor
2018-02-09  2:42 ` [PATCH 1/3] t: document 'test_must_fail ok=<signal-name>' SZEDER Gábor
2018-02-09  2:42 ` SZEDER Gábor [this message]
2018-02-09  3:11   ` [PATCH 2/3] t: teach 'test_must_fail' to save the command's stderr to a file Eric Sunshine
2018-02-09 14:21   ` Jeff King
2018-02-09 18:36     ` Junio C Hamano
2018-02-09 18:57       ` Jeff King
2018-02-09 19:03         ` Jeff King
2018-02-23 23:26     ` SZEDER Gábor
2018-02-09  2:42 ` [PATCH 3/3] t1404: use 'test_must_fail stderr=<file>' SZEDER Gábor
2018-02-09  3:16   ` Eric Sunshine
2018-02-09  3:33     ` SZEDER Gábor
2018-02-09 18:22       ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20180209024235.3431-3-szeder.dev@gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).