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

Jeff King
2024-08-19T08:51:07Z

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).

-Peff