All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Paul Tan <pyokagan@gmail.com>,
	git@vger.kernel.org, Stefan Beller <sbeller@google.com>,
	David Aguilar <davvid@gmail.com>
Subject: Re: sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false
Date: Mon, 18 May 2015 13:41:31 -0700	[thread overview]
Message-ID: <xmqq8uclbouc.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20150518184528.GA11463@peff.net> (Jeff King's message of "Mon, 18 May 2015 14:45:29 -0400")

Jeff King <peff@peff.net> writes:

> [2] For the most part, actually, I think the status quo is using
>     test_cmp, and most of the uses of "test" are old. Using test_cmp is
>     more verbose, but it does have the advantage of checking the exit
>     code of the sub-programs (and also its output is much saner for
>     multi-line comparisons, but that isn't relevant to Paul's patches).
>
>     I'd be in favor of helpers that made it shorter to use test_cmp.
>     E.g.:
>
>       # "actual" contains "foo\n"; we save one line of "echo"
>       git foo >actual &&
>       test_cmp_str foo actual
>
>       # same, but take multiline input on stdin. Saves one line of "cat"
>       git foo >actual &&
>       test_cmp_str - actual <<-\EOF
>       foo
>       EOF
>
>       # similar, but it runs the command for us and compares its
>       # stdout, saving another line. Note that we take stdout as-is,
>       # so you get no opportunity to post-process it (but you
>       # can still do it the "long" way as above if you need to).
>       test_cmp_cmd foo git foo
>
>       # same, with stdin; obviously this doesn't work if you need to
>       # send something over stdin to the command, but again, you can
>       # fallback to the old way.
>       test_cmp_cmd - git foo <<-\EOF
>       foo
>       EOF
>
>    These also go in the direction of "verbose", by providing generic
>    solutions rather than specific ones. So by the argument you made
>    above, this is a bad thing.

I do not agree with that.  Perhaps "unconstained" was an unfit
phrase.

"test_cmp_cmd expected-string git foo" may be "test $a = $b" inside,
but its debug error message could be made to say "output from 'git
foo' is different from 'expected-string'", which is a very good
thing.  Contrast that with:

	verbose test "$(git foo)" = "expected-string"

which would not give you that information (and you'd need to do the
"-x" thing to find out where the string that is different from the
expected string came from).

Ultimately that is because 'test' that an individual test script
writes in it does not constrain what is on each side of comparison
(for that matter, it does not even constrain that what is tested is
comparison between two things), which makes it very hard to give
useful diagnoses.  'test' itself operates at too low a level, and
wrapping it with 'verbose' does not recover the information already
lost when invoking 'test'.

So, I suspect we want more or less the same thing.

We agree that we need shorter, easier to use, and less error prone
idioms, like test_cmp (comparing two files and show the differences
as diagnosis), test_cmp_rev (comparing two extended SHA-1
expressions as object names), and test_line_count (checking the
number of lines in a file).  I just do not think "verbose" that can
apply to any command will help us move in that direction very much,
and I find that "verbose" applied to something overly versatile like
"test" is a prime example that it does not help us much.

  reply	other threads:[~2015-05-18 20:41 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-13  9:52 [PATCH 0/4] make pull.ff=true override merge.ff Paul Tan
2015-05-13  9:52 ` [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false Paul Tan
2015-05-14 13:06   ` Johannes Schindelin
2015-05-14 16:45     ` Junio C Hamano
2015-05-14 16:53       ` sh -x -i -v with continuous integration, was " Johannes Schindelin
2015-05-14 17:06         ` Junio C Hamano
2015-05-16 15:28           ` Jeff King
2015-05-16 19:07             ` Junio C Hamano
2015-05-18 18:45               ` Jeff King
2015-05-18 20:41                 ` Junio C Hamano [this message]
2015-05-18 20:50                   ` Jeff King
2015-05-18 20:58                     ` Junio C Hamano
2015-05-16 12:33         ` Paul Tan
2015-05-13  9:52 ` [PATCH 2/4] pull: make pull.ff=true override merge.ff Paul Tan
2015-05-13  9:52 ` [PATCH 3/4] Documentation/config.txt: clarify that pull.ff overrides merge.ff Paul Tan
2015-05-13  9:52 ` [PATCH 4/4] pull: parse pull.ff as a bool or string Paul Tan
2015-05-14 12:59 ` [PATCH 0/4] make pull.ff=true override merge.ff Johannes Schindelin
  -- strict thread matches above, loose matches on Subject: below --
2015-05-19 13:22 sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false Johannes Schindelin

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=xmqq8uclbouc.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    --cc=pyokagan@gmail.com \
    --cc=sbeller@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.