Re: [PATCH] Fix a false negative in t5512 when run using sh -x

From: Johannes Sixt <j.sixt@viscovery.net>
Date: May 10, 2010, 11:03 UTC
Message-ID: <4BE7E7FD.7080607@viscovery.net>
In-Reply-To: <20100510103427.GA4806@progeny.tock>

Am 5/10/2010 12:34, schrieb Jonathan Nieder:

Show 44 quoted lines

> 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