On Tue, Feb 26, 2019 at 12:43:17PM -0500, Jeff King wrote:
> On Tue, Feb 26, 2019 at 06:04:00PM +0100, SZEDER Gábor wrote:
>
> > > Whereas:
> > >
> > > + test -f doesnotexist
> > > + echo File doesnotexist doesn't exist.
> > > File doesnotexist doesn't exist.
> > > + false
> > > error: last command exited with $?=1
> > >
> > > Gives me the same thing, but I have to read 5 lines instead of 2 that
> > > ultimately don't tell me any more (and a bit of "huh, 'false' returned
> > > 1? Of course! Oh! It's faking things up and it's the 'echo' that
> > > matters...").
> >
> > I didn't find this to be an issue, but because of functions like
> > 'test_seq' and 'test_must_fail' I've thought about suppressing '-x'
> > output for test helpers (haven't actually done anything about it,
> > though).
>
> I'd be curious how you'd do that.
Well, I started replying with "Dunno" and explaining why I don't think
that it can be done with 'test_must_fail'... but then got a bit of a
lightbulb moment. Now look at this:
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 80402a428f..16adcd54c9 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -664,7 +664,15 @@ list_contains () {
# Currently recognized signal names are: sigpipe, success.
# (Don't use 'success', use 'test_might_fail' instead.)
+restore_tracing () {
+ if test -n "$trace"
+ then
+ set -x
+ fi
+} 2>/dev/null 4>/dev/null
+
test_must_fail () {
+ { set +x ; } 2>/dev/null 4>/dev/null
case "$1" in
ok=*)
_test_ok=${1#ok=}
@@ -679,24 +687,29 @@ test_must_fail () {
if test $exit_code -eq 0 && ! list_contains "$_test_ok" success
then
echo >&4 "test_must_fail: command succeeded: $*"
+ restore_tracing
return 1
elif test_match_signal 13 $exit_code && list_contains "$_test_ok" sigpipe
then
+ restore_tracing
return 0
elif test $exit_code -gt 129 && test $exit_code -le 192
then
echo >&4 "test_must_fail: died by signal $(($exit_code - 128)): $*"
+ restore_tracing
return 1
elif test $exit_code -eq 127
then
echo >&4 "test_must_fail: command not found: $*"
+ restore_tracing
return 1
elif test $exit_code -eq 126
then
echo >&4 "test_must_fail: valgrind error: $*"
+ restore_tracing
return 1
fi
- return 0
+ restore_tracing
} 7>&2 2>&4
# Similar to test_must_fail, but tolerates success, too. This is
Yeah, it's a hassle, especially in a function with as many return
paths as 'test_must_fail', but look at its output:
+ test_must_fail git rev-parse nope --
fatal: bad revision 'nope'
+ test_must_fail git rev-parse HEAD --
48ab21c1a5972e0fa9d87da7c5da9982872b8db2
test_must_fail: command succeeded: git rev-parse HEAD --
+ return 1
error: last command exited with $?=1
Not even the 'set +x' shows up in the trace output! Unfortunately,
that line is not particularly pleasing on the eyes, but I don't see
any way around that...
Perhaps we could even go one step further with this 'restore_tracing'
helper and add a parameter specifying its return code, so we could
make it the last command invoked in the test helper function, and then
even that 'return 1' would disappear from the trace output.
Furthermore, this would be helpful in those functions where the last
command's return code is relevant, e.g:
test_cmp() {
{ set +x ; } 2>/dev/null 4>/dev/null
$GIT_TEST_CMP "$@"
restore_tracing $?
}
There are a couple of tricky cases:
- Some test helper functions call other test helper functions, and
in those cases tracing would be enabled upon returning from the
inner helper function. This is not an issue with e.g.
'test_might_fail' or 'test_cmp_config', because the inner helper
function is the last command anyway. However, there is
'test_must_be_empty', 'test_dir_is_empty', 'test_config',
'test_commit', etc. which call the other test helper functions
right at the start or in the middle.
- && chains in test helper functions; we must make sure that the
tracing is restored even in case of a failure.
