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
