Re: [PATCH v2 4/5] tests: do not let lazy prereqs inside `test_expect_*` turn off tracing

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Peff,

On Thu, 26 Mar 2020, Jeff King wrote:

> On Wed, Mar 25, 2020 at 05:41:20AM +0000, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The `test_expect_*` functions use `test_eval_` and so does
> > `test_run_lazy_prereq_`. If tracing is enabled via the `-x` option,
> > `test_eval_` turns on tracing while evaluating the code block, and turns
> > it off directly after it.
> >
> > This is unwanted for nested invocations.
> >
> > One somewhat surprising example of this is when running a test that
> > calls `test_i18ngrep`: that function requires the `C_LOCALE_OUTPUT`
> > prereq, and that prereq is a lazy one, so it is evaluated via
> > `test_eval_`, the command tracing is turned off, and the test case
> > continues to run _without tracing the commands_.
>
> Good catch. I didn't see this when looking at the GPG stuff earlier
> because I didn't nest the lazy prereqs. But it is worth fixing
> regardless, because as you note it comes up in other places.
>
> > @@ -926,7 +927,8 @@ test_eval_ () {
> >  		test_eval_ret_=$?
> >  		if want_trace
> >  		then
> > -			set +x
> > +			test 1 = $_trace_level && set +x
> > +			_trace_level=$(($_trace_level-1))
> >  		fi
> >  	} 2>/dev/null 4>&2
>
> I briefly wondered if adding more logic here might upset our delicate
> balance of avoiding writing cruft to the "-x" output. But the answer is
> "no", due to the descriptor hackery at the end of that {} block.
>
> Of course, any test evaluating a lazy prereq already gets tons of cruft
> anyway, because the outer level of tracing sees all of our internal
> function calls checking the prereq and setting up the inner level of
> tracing. But there's not much we can do about that.

We could turn off the tracing specifically in those cases. At some point,
though, it strikes me as rather ridiculous through how many hoops we jump
just because our test suite framework is implemented in portable Unix
shell script, as opposed to a more powerful language such as, say, C.

For example, we could prevent the `test_eval_ret_=$?` line from being
traced by default, simply by redirecting fd `4` to `/dev/null`. At that
stage, we would of course have to open yet another fd to support the use
case where the `-x` is passed to `sh` itself (`sh -x t0000-*.sh -i -v`).

In the context of this here patch series, which really is about enabling
the GPG-related tests on Windows, I will refrain from going down that
particular rabbit hole. My ego might get stuck.

Ciao,
Dscho