Re: [PATCH 08/16] t5304: use helper to report failure of "test foo = bar"

From: Jeff King <peff@peff.net>
Date: Oct 7, 2014, 21:29 UTC
Message-ID: <20141007212941.GA24508@peff.net>
In-Reply-To: <xmqq61fveg1j.fsf@gitster.dls.corp.google.com>

On Tue, Oct 07, 2014 at 01:35:15PM -0700, Junio C Hamano wrote:

Show 5 quoted lines

> Yeah, if we are going to reduce it down to the above implementation,
> intereseting things like "test -f $frotz" will become possible and
> "cmp-str" stops making sense.  It really is about "We run test and
> expect it to yield true.  Report the failure a bit more prominently
> under the '-v' option to help us debug".

We already have test_path_is_file to do the same thing just for "-f". We could in theory switch all of those to this new, more generic wrapper. I don't know if it is worth doing a mass-conversion, but we could discourage test_path_is_file in new tests. We could also implement test_path_is_{dir,file} on top of this.

There is also test_path_is_missing, which would need the negated form. We'd either need a "test_not_cond", or to allow:

  test_cond ! -e path

That is specified by POSIX. I seem to recall that we ran into problems using it with some shells, but I note there is currently some use of it in t5304. So perhaps it is fine.

> So among the ones you listed, test_verbose may be the least silly, I
> would think.

Somehow test_verbose seems to me like checking the "verbose" option of the test suite. I prefer "test_cond", but I do not feel too strongly, if you want to override me.

Show 8 quoted lines

> > (any quoting or whitespace may be wrong, of course, but that's OK; it's
> > for human consumption, and is just a hint).
> 
> If we really cared, we could do
> 
> 	echo >&2 "command failed: test $(git rev-parse --sq-quote "$@")"
> 
> perhaps?

Yeah, that would work. I am always a little hesitant sticking git commands into our test infrastructure, since we may end up masking errors due to our own bug. But we can probably rely on --sq-quote working sanely (and anyway, we're not even affecting the test outcome here).

-Peff