Hi Junio,
On Sat, May 16, 2015 at 2:37 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Just to avoid misunderstanding, please do not remove 'verbose '
> blindly without thinking while doing so, as you already did 1/3 of
> the necessary job to make things better.
Eh? I thought we established that using "verbose" does not provide
anything more than what "set -x" already provides. So at the very
least, its use should be removed completely.
Here is my understanding of the current situation, please correct me
if I am wrong:
When a test fails, it would be useful to know the exact, unexpanded,
unsubstituted command which failed (additionally with a nice stack
trace and line numbers). However, the output of -v and -x (and by
extension, the "verbose" function) is not very helpful, as it still
requires the debugger to understand the test script.
-v will print out the stdout and stderr of the executed commands, but
it requires the debugger to match up the output of the commands with
the test script to understand where the test failed. It is also not
very helpful in the case of "test", which does not print anything when
it fails.
-x will trace the commands being executed, but the tracing output is
so verbose it still requires the debugger to understand the test
script. e.g:
test "$(cat file)" "expected"
If the above test fails (e.g. the content of the file is
"unexpected"), the tracing output will be:
+ cat file
+ test unexpected = expected
error: last command exited with $?=1
Furthermore, the format of the tracing output is not specified by
POSIX, so we can't count on it being consistent among all shells (e.g.
for users who submit bug reports with the test output)
> You might have noticed, while adding them, there were something
> common that we currently do with a bare 'test' only because we
> haven't identified common needs. As I already said, it may be that
> we often try to see a file has a known single line content (I didn't
> check if that were the case; I am just giving you an example) and
> only because there is no ready-made test_file_contents helper to be
> used, the current tests say
>
> test expected_string = "$(cat file)"
>
> And if that were the case, it is a good thing to have a new helper
> like this
>
> test_file_contents () {
> if test "$(cat "$1")" != "$2"
> then
> echo "Contents of file '$1' is not '$2'"
> false
> fi
> }
>
> in t/test-lib-functions.sh and convert them to say
>
> test_file_contents file expected_string
>
> That would be an improvement (and that is the remaining 2/3 ;-).
Yeah, this kind of comparison with file contents is something that is
done often in t5520, so I agree with adding it.
However, what about these kind of tests:
test new = "$(git show HEAD:file2)"
or these:
test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)
So, perhaps we could introduce a generic function like:
# Compares that the output of $1 eval'ed is identical to $2.
test_output () {
output=$(eval $1)
if "$output" != "$2"
then
echo >&2 "Output of '$1' ('$output') != '$2'"
false
fi
}
So the first example would be:
test_output "git show HEAD:file2" new
And the error output will thus be:
Output of 'git show HEAD:file2' ('some unexpected output') != 'new'
So we know the exact comparison that failed, and we know how the
expected and actual output differs.
What do you think?
Thanks,
Paul
--
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