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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> On Tue, Oct 07, 2014 at 01:35:15PM -0700, Junio C Hamano wrote:
>
>> 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.

Oh, I wasn't going in that direction when I mentioned "-f"; I just
wanted to say that 'test "$@"' is clearly about 'test' (/bin/test or
shell built-in) and less about 'compare string'.  I do not think it
is necessarily a good direction to go in to replace test-path-is-file
with the test_cond thing; after all, type specific tests have chance
to report breakage of expectation in type specifc ways, e.g.

	test_path_is_file () {
		test -f "$1" && return 0
		echo >&2 "expected '$1' to be file"
		if test -e "$1"
                then
	               	echo >&2 "but it is missing"
		else
                	echo >&2 "but it is a non-file"
			ls >&2 -ld "$1"
		fi
                return 1
	}

But that is also just in theory ;-).

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

Hmph, your 'test' in that name is a generic verb "we check that...",
which I think aligns better with the other test_foo functions.  When
I suggested 'test_verbose', 'test' in that name was specifically
meant to refer to the 'test' command.

Still "test_cond" feels somewhat funny, as "we check that..." always
validates some condition, but I don't think of anything better X-<.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]