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 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).