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
