On Thu, May 14, 2015 at 10:06:57AM -0700, Junio C Hamano wrote:
> And I would say that "verbose test" is not a useful way to make the
> test output more verbose for Human's sake; GIT_TEST_OPTS="-v -x -i"
> on the other hand is.
>
> I am very tempted to suggest doing this ;-)
>
> t/test-lib-functions.sh | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 0698ce7..c6c67e8 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -638,8 +638,14 @@ test_cmp_bin() {
> }
>
> # Call any command "$@" but be more verbose about its
> -# failure. This is handy for commands like "test" which do
> -# not output anything when they fail.
> +# failure. This may seem handy for commands like "test" which do
> +# not output anything when they fail, but in practice not
> +# very useful for things like 'test "$actual" = "$expect"',
> +# as this only shows the actual values (i.e. after $actual and
> +# $expect are turned into the values in these variables).
> +#
> +# In short, do not add use of this; this is kept only to
> +# avoid having to remove its use (for now).
Sorry, but I completely disagree here (no surprise, as I was the one who
added "verbose").
It's true that it does not tell you the original commands that were
handed to the shell. But neither does the output of "test_cmp". However,
combined with the output of the test code snippet that we already
provide, it is often clear.
E.g., let's "break" a verbose test from the test suite and break it:
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 1b2e981..f3206a3 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -113,7 +113,7 @@ test_expect_success 'diff-filter=M' '
actual=$(git log --pretty="format:%s" --diff-filter=M HEAD) &&
expect=$(echo second) &&
- verbose test "$actual" = "$expect"
+ verbose test "$actual" = "foo-$expect"
'
Right now the output is:
expecting success:
actual=$(git log --pretty="format:%s" --diff-filter=M HEAD) &&
expect=$(echo second) &&
verbose test "$actual" = "foo-$expect"
command failed: 'test' 'second' '=' 'foo-second'
not ok 10 - diff-filter=M
which IMHO is pretty helpful. If we take away the "verbose", we get:
expecting success:
actual=$(git log --pretty="format:%s" --diff-filter=M HEAD) &&
expect=$(echo second) &&
test "$actual" = "foo-$expect"
not ok 10 - diff-filter=M
which leaves you with head-scratching (did log exit non-zero, or did the
comparison fail?). If we add in "-x", we get:
expecting success:
actual=$(git log --pretty="format:%s" --diff-filter=M HEAD) &&
expect=$(echo second) &&
test "$actual" = "foo-$expect"
+ git log --pretty=format:%s --diff-filter=M HEAD
+ actual=second
+ echo second
+ expect=second
+ test second = foo-second
error: last command exited with $?=1
not ok 10 - diff-filter=M
That essentially gives us the same data, but with a lot of other cruft.
In this case, I don't think it's too bad (the test isn't that long), but
some of the "-x" cases can get pretty verbose. I guess things are a bit
better since I added the "last command exited..." in red, but I still
think the "verbose" version is the most straightforward.
Do we object to having to sprinkle the "verbose" throughout the test
scripts? IMHO it is not any worse than test_cmp. But if that is a
problem, would it be too magical to do something like:
test () {
verbose command test "$@"
}
(but only inside the test snippets)?
-Peff
--
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