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

Jeff King [off-list ref] writes:

On Wed, Oct 08, 2014 at 12:17:07AM +0200, Michael Haggerty wrote:

quoted

On 10/07/2014 11:53 PM, Junio C Hamano wrote:

quoted

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.

I actually meant "test" as a namespace to indicate it is part of the
test suite (just like "test_seq" is not testing anything). I think that
is why the names are so silly. We are using the "test" command in our
"test" suite to "test" some conditions.

quoted

I like "verbose_test $foo = $bar" because it puts the word "test" next
to the condition, where the built-in command "test" would otherwise be.

We could even define a command

	verbose () {
		"$@" && return 0
		echo >&2 "command failed: $*"
		return 1
	}

and use it like

	verbose test $foo = $bar

I kind of like this. It is easy to see which shell command is being
invoked, and it would extend naturally to other silent commands.

quoted

Somehow I feel like I'm reinventing something that must already exist...

Yes, we're basically reinventing "set -x" here, with the caveat that we
only _really_ care about showing failed commands. The problem with "set
-x" is that it also wants to apply itself to the test harness itself, so
you end up with a lot of cruft.

As you have done, I've done my share of running tests under "sh -x",
and I agree that it gives too much noise. Which is almost impossible
to read through without a lot of patience, but at least it contains
everything we could ask, short of running a specific step under gdb.

The "verbose" thing that reports only when the expectation is violated
cuts down the cruft (so does your original test_cond/verbose_test),
so it may make it easier than the bare-metal "sh -x".

But it may cut down a bit too much.  Often in a test that consists
of multiple commands chained together with && inside a single
test_expect_success, what leads to the eventual unsatisfied
expectation is an earlier step that succeeds in the sense that it
satisfies the expectation of that step (e.g. "git branch foo &&" and
we only check it did not exit with non-zero status) but did not
actually do what we wanted it to do (e.g. that "git branch" did not
create a new branch 'foo' but created 'refs/foo' by mistake), and
then a later step is tripped by the unsatisified expectation that
was not explicitly tested (e.g. after that "git checkout foo" would
not quite fail but instead of checking out a branch to build upon,
it detaches the HEAD at that commit, because the earlier "git branch"
was faulty but we did not notice it).

So I dunno.  For harder cases we always have to resort to "sh -x",
but "verbose" thing may still help easier cases, just like the use
of test_cmp instead of cmp is helping us while viewing output from
running test with the "-v" option today, and that would be a good
enough improvement.