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: Sverre Rabbelier <srabbelier@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Tay Ray Chuan <rctay89@gmail.com>
Subject: Re: [PATCH] Fix a false negative in t5512 when run using sh -x
Date: Mon, 10 May 2010 13:03:25 +0200	[thread overview]
Message-ID: <4BE7E7FD.7080607@viscovery.net> (raw)
In-Reply-To: <20100510103427.GA4806@progeny.tock>

Am 5/10/2010 12:34, schrieb Jonathan Nieder:
> Sverre Rabbelier wrote:
> 
>> Don't we do this ('test_cmp' on expected output) in many other places
>> as well? Why is this different?
> 
> The problem arises with test_cmp on expected output from stderr, since
> sh -x will mingle the trace of a function with the standard error
> stream.  bash provides BASH_XTRACEFD to work around this misdesign,
> but other shells might not be so helpful.
> 
> Here’s a workaround, ugly as sin.  It overreaches a little because I
> did not bother to check which tests grep and which tests test_cmp
> their output.
> 
>  t/t0040-parse-options.sh       |    6 +++---
>  t/t1300-repo-config.sh         |    2 +-
>  t/t1450-fsck.sh                |    2 +-
>  t/t1503-rev-parse-verify.sh    |   20 ++++++++++----------
>  t/t1506-rev-parse-diagnosis.sh |   24 ++++++++++++------------
>  t/t2204-add-ignored.sh         |    8 ++++----
>  t/t3030-merge-recursive.sh     |    6 +++---
>  t/t3400-rebase.sh              |    2 +-
>  t/t3501-revert-cherry-pick.sh  |    2 +-
>  t/t3800-mktag.sh               |    2 +-
>  t/t4011-diff-symlink.sh        |    2 +-
>  t/t4014-format-patch.sh        |    6 +++---
>  t/t4120-apply-popt.sh          |    2 +-
>  t/t4124-apply-ws-rule.sh       |    2 +-
>  t/t4133-apply-filenames.sh     |    6 +++---
>  t/t5300-pack-object.sh         |    2 +-
>  t/t5400-send-pack.sh           |    2 +-
>  t/t5406-remote-rejects.sh      |    4 +++-
>  t/t5505-remote.sh              |    4 ++--
>  t/t5510-fetch.sh               |    2 +-
>  t/t5512-ls-remote.sh           |    2 +-
>  t/t6024-recursive-merge.sh     |    2 +-
>  t/t6030-bisect-porcelain.sh    |    6 +++---
>  t/t7110-reset-merge.sh         |   10 +++++-----
>  t/t7201-co.sh                  |    2 +-
>  t/t7610-mergetool.sh           |    6 +++---
>  t/t8003-blame.sh               |    4 ++--
>  t/t9001-send-email.sh          |   14 +++++++-------
>  t/t9108-git-svn-glob.sh        |    2 +-
>  29 files changed, 78 insertions(+), 76 deletions(-)

/me slaps forehead

I carry around a patch that logs "set -x" output to a file on Windows to
help debug difficult to reproduce test failures. I had already inserted
"set +x" in all test cases where it matters, but t5512 is a new case since
I have this patch.

I can just amend my patch, no problem, and we solve the issue as you
propose (or perhaps not - it *is* ugly as hell).

FWIW, test scripts that need this treatement are only:

t/t0040-parse-options.sh
t/t1503-rev-parse-verify.sh
t/t4014-format-patch.sh
t/t5505-remote.sh

and now t/t5512-ls-remote.sh as well.

-- Hannes

  reply	other threads:[~2010-05-10 11:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-10  7:19 [PATCH] Fix a false negative in t5512 when run using sh -x Johannes Sixt
2010-05-10  7:28 ` Tay Ray Chuan
2010-05-10  9:09 ` Sverre Rabbelier
2010-05-10 10:23   ` Johannes Sixt
2010-05-10 10:45     ` Sverre Rabbelier
2010-05-10 10:34   ` Jonathan Nieder
2010-05-10 11:03     ` Johannes Sixt [this message]
2010-05-10 11:31       ` Jonathan Nieder
2010-05-10 11:34         ` Johannes Sixt

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=4BE7E7FD.7080607@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=rctay89@gmail.com \
    --cc=srabbelier@gmail.com \
    /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).