On Sun, Mar 03, 2019 at 05:04:59PM +0100, SZEDER Gábor wrote:
> > > - && chains in test helper functions; we must make sure that the
> > > tracing is restored even in case of a failure.
>
> Actually, the && chain is not really an issue, because we can simply
> break the && chain at the very end:
>
> test_func () {
> { disable_tracing ; } 2>/dev/null 4>&2
> do this &&
> do that
> restore_tracing
> }
>
> and make restore_tracing exit with $? (like you did above in pop_x()).
Yeah, good point.
> > Yeah, there is no "goto out" to help give a common exit point from the
> > function. You could probably do it with a wrapper, like:
>
> Yeah, the wrapper works.
> There are only a few test helper functions with multiple 'return'
> statements, and refactoring them to have a single 'return $ret' at the
> end worked, too.
Yeah, that might be less sneaky than this wrapper business. Or we could
just do a few basic wrappers. The non-portable bit in my wrapper
suggestion was the renaming of the old function. But if we accept just:
real_foo() {
... do stuff with multiple returns ...
}
disable_function_tracing real_foo foo
then that is pretty trivial to do with an eval. It does disallow your
"wrap all functions at once", but I think that is OK. We might want to
only do a subset anyway.
> We should also be careful and don't switch on tracing when returning
> from test helper functions invoked outside of tests, e.g.
> 'test_create_repo' while initializing the trash directory or
> 'test_set_port' while sourcing a daemon-specific lib.
Yeah, it would probably make sense in the "push" half to check that we
are actually tracing at that moment.
> On a mostly unrelated note, but I just noticed it while playing around
> with this: 't0000'-basic.sh' runs its internal tests with $SHELL_PATH
> instead of $TEST_SHELL_PATH. I'm not sure whether that's right or
> wrong.
I'd say probably wrong, though it likely doesn't matter that much in
practice.
-Peff
