Re: sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Jeff King <peff@peff.net>]

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