From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
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 14:45:29 -0400 [thread overview]
Message-ID: <20150518184528.GA11463@peff.net> (raw)
In-Reply-To: <xmqq1tigfij8.fsf@gitster.dls.corp.google.com>
On Sat, May 16, 2015 at 12:07:23PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Do we object to having to sprinkle the "verbose" throughout the test
> > scripts?
>
> Yes.
>
> An unconstrained "verbose" that applies to anything would make
> people less careful to come up with more useful abstractions,
> e.g. test_line_count, which I view as a bigger problem.
I don't think that is true at all. They serve two different purposes.
We need "test_line_count" because of portability considerations. But we
do not need "test_eq" for that; the only reason to add it is for
debuggable output. We _could_ add a bunch of little helpers to cover
each situation, but empirically we have not[1], because it's annoying to
do so. Instead we just write a bare "test". I had hoped that "verbose"
would make it sufficiently easy to improve upon the status quo that
people would actually use it[2].
So I don't really buy the argument that "verbose" makes things worse. I
think I _do_ buy the argument that it is not worth the extra effort
given that I implemented "-x" around the same time. I think the
"verbose" output is a little nicer, as I said earlier, but having the
test-writer do literally zero work may be worth it.
-Peff
[1] The exception, AFAIK, is test_path_is_*.
[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.
next prev parent reply other threads:[~2015-05-18 18:45 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 [this message]
2015-05-18 20:41 ` Junio C Hamano
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=20150518184528.GA11463@peff.net \
--to=peff@peff.net \
--cc=davvid@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
--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.