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