git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: SZEDER Gábor <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>,
	git@vger.kernel.org, SZEDER Gábor <szeder.dev@gmail.com>
Subject: [PATCH 1/2] t9400-git-cvsserver-server: don't rely on the output of 'test_cmp'
Date: Thu,  8 Mar 2018 13:38:43 +0100
Message-ID: <20180308123844.15163-2-szeder.dev@gmail.com> (raw)
In-Reply-To: <20180308123844.15163-1-szeder.dev@gmail.com>

The test 'cvs update (-p)' redirects and checks 'test_cmp's stdout and
even its stderr.  The commit introducing this test in 6e8937a084
(cvsserver: Add test for update -p, 2008-03-27) doesn't discuss why,
in fact its log message only consists of that subject line.  Anyway,
weird as it is, it kind of made sense due to the way that test was
structured:

After a bit of preparation, this test updates four files via CVS and
checks their contents using 'test_cmp', but it does so in a for loop
iterating over the names of those four files.  Now, the exit status of
a for loop is the exit status of the last command executed in the
loop, meaning that the test can't simply rely on the exit code of
'test_cmp' in the loop's body.  Instead, the test works it around by
relying on the stdout of 'test_cmp' being silent on success and
showing the diff on failure, as it appends the stdout of all four
'test_cmp' invocations to a single file and checks that file's
emptiness after the loop (with 'test -z "$(cat ...)"', no less; there
was no 'test_must_be_empty' back then).  Furthermore, the test
redirects the stderr of those 'test_cmp' invocations to this file,
too: while 'test_cmp' itself doesn't output anything to stderr, the
invoked 'diff' or 'cmp' commands do send their error messages there,
e.g. if they can't open a file because its name was misspelled.

This also makes this test fail when the test script is run with '-x'
tracing (and using a shell other than a Bash version supporting
BASH_XTRACEFD), because 'test_cmp's stderr contains the trace of the
'diff' command executed inside the helper function, throwing off the
subsequent emptiness check.

Unroll that for loop, so we can check the files' contents the usual
way and rely on 'test_cmp's exit code failing the && chain.  Extract
updating a file via CVS and verifying its contents using 'test_cmp'
into a helper function requiring the file's name as parameter to avoid
much of the repetition resulting from unrolling the loop.

After this change t9400 passes with '-x', even when running with
/bin/sh.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 t/t9400-git-cvsserver-server.sh | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index c30660d606..1f67d2822f 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -437,6 +437,11 @@ test_expect_success 'cvs update (merge no-op)' \
     GIT_CONFIG="$git_config" cvs -Q update &&
     test_cmp merge ../merge'
 
+check_cvs_update_p () {
+    GIT_CONFIG="$git_config" cvs update -p "$1" >"$1".out &&
+    test_cmp "$1".out ../"$1"
+}
+
 cd "$WORKDIR"
 test_expect_success 'cvs update (-p)' '
     touch really-empty &&
@@ -447,12 +452,10 @@ test_expect_success 'cvs update (-p)' '
     git push gitcvs.git >/dev/null &&
     cd cvswork &&
     GIT_CONFIG="$git_config" cvs update &&
-    rm -f failures &&
-    for i in merge no-lf empty really-empty; do
-        GIT_CONFIG="$git_config" cvs update -p "$i" >$i.out
-	test_cmp $i.out ../$i >>failures 2>&1
-    done &&
-    test -z "$(cat failures)"
+    check_cvs_update_p merge &&
+    check_cvs_update_p no-lf &&
+    check_cvs_update_p empty &&
+    check_cvs_update_p really-empty
 '
 
 cd "$WORKDIR"
-- 
2.16.2.603.g180c1428f0


  reply index

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-08 12:38 [PATCH 0/2] Make cvs tests pass with '-x' tracing and /bin/sh SZEDER Gábor
2018-03-08 12:38 ` SZEDER Gábor [this message]
2018-03-08 18:51   ` [PATCH 1/2] t9400-git-cvsserver-server: don't rely on the output of 'test_cmp' Eric Sunshine
2018-03-08 21:49     ` SZEDER Gábor
2018-03-08 22:01       ` Eric Sunshine
2018-03-08 22:07         ` Eric Sunshine
2018-03-08 22:38         ` SZEDER Gábor
2018-03-08 22:44         ` [PATCH v1.1 " SZEDER Gábor
2018-03-08 23:33           ` Junio C Hamano
2018-03-08 23:41             ` SZEDER Gábor
2018-03-08 23:44               ` Junio C Hamano
2018-03-09 11:20                 ` SZEDER Gábor
2018-03-08 12:38 ` [PATCH 2/2] t9402-git-cvsserver-refs: don't check the stderr of a subshell SZEDER Gábor
2018-03-13  0:05 ` [PATCH 0/2] Make cvs tests pass with '-x' tracing and /bin/sh Jeff King

Reply instructions:

You may reply publically 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=20180308123844.15163-2-szeder.dev@gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox