Re: [PATCH 2/3] t7900: exercise detaching via trace2 regions

[Patrick Steinhardt <ps@pks.im>]

On Mon, Aug 19, 2024 at 04:51:05AM -0400, Jeff King wrote:
> On Mon, Aug 19, 2024 at 09:48:02AM +0200, Patrick Steinhardt wrote:
> 
> > In t7900, we exercise the `--detach` logic by checking whether the
> > command ended up writing anything to its output or not. This supposedly
> > works because we close stdin, stdout and stderr when daemonizing. But
> > one, it breaks on platforms where daemonize is a no-op, like Windows.
> > And second, that git-maintenance(1) outputs anything at all in these
> > tests is a bug in the first place that we'll fix in a subsequent commit.
> > 
> > Introduce a new trace2 region around the detach which allows us to more
> > explicitly check whether the detaching logic was executed. This is a
> > much more direct way to exercise the logic, provides a potentially
> > useful signal to tracing logs and also works alright on platforms which
> > do not have the ability to daemonize.
> 
> Nice, this is so much cleaner than the way the existing test worked. The
> code looks good, but...
> 
> > diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> > index 074eadcd1c..46a61d66fb 100755
> > --- a/t/t7900-maintenance.sh
> > +++ b/t/t7900-maintenance.sh
> > @@ -950,8 +950,9 @@ test_expect_success '--no-detach causes maintenance to not run in background' '
> >  		# We have no better way to check whether or not the task ran in
> >  		# the background than to verify whether it output anything. The
> >  		# next testcase checks the reverse, making this somewhat safer.
> > -		git maintenance run --no-detach >out 2>&1 &&
> > -		test_line_count = 1 out
> > +		GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
> > +			git maintenance run --no-detach >out 2>&1 &&
> > +		! test_region maintenance detach trace.txt
> >  	)
> >  '
> 
> ...I think this "we have no better way..." comment is now out of date
> (and can probably just be dropped).

Oops, yes, that one is definitely stale. I'll drop it in the next
version of this patch series.

Patrick