git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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.

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