| Message ID | 4805bb6f6c2c96a2c40d1d8359b63b8c7045e0b6.1724053639.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 759b453f9f21c5cda483a38f6693dfcbcb680290 |
| Headers | show |
| Series | Fixups for git-maintenance(1) tests | expand |
On Mon, Aug 19, 2024 at 09:47:59AM +0200, Patrick Steinhardt wrote: > diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh > index 06ab43cfb5..074eadcd1c 100755 > --- a/t/t7900-maintenance.sh > +++ b/t/t7900-maintenance.sh > @@ -967,8 +967,13 @@ test_expect_success '--detach causes maintenance to run in background' ' > git config set maintenance.loose-objects.auto 1 && > git config set maintenance.incremental-repack.enabled true && > > - git maintenance run --detach >out 2>&1 && > - test_must_be_empty out > + # The extra file descriptor gets inherited to the child > + # process, and by reading stdout we thus essentially wait for > + # that descriptor to get closed, which indicates that the child > + # is done, too. > + output=$(git maintenance run --detach 2>&1 9>&1) && > + printf "%s" "$output" >output && > + test_must_be_empty output > ) > ' This looks correct, but should we be doing it for all of the "git maintenance" runs in that script? They're all going to kick off detached gc jobs, I think. -Peff
On Mon, Aug 19, 2024 at 04:49:43AM -0400, Jeff King wrote: > On Mon, Aug 19, 2024 at 09:47:59AM +0200, Patrick Steinhardt wrote: > > > diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh > > index 06ab43cfb5..074eadcd1c 100755 > > --- a/t/t7900-maintenance.sh > > +++ b/t/t7900-maintenance.sh > > @@ -967,8 +967,13 @@ test_expect_success '--detach causes maintenance to run in background' ' > > git config set maintenance.loose-objects.auto 1 && > > git config set maintenance.incremental-repack.enabled true && > > > > - git maintenance run --detach >out 2>&1 && > > - test_must_be_empty out > > + # The extra file descriptor gets inherited to the child > > + # process, and by reading stdout we thus essentially wait for > > + # that descriptor to get closed, which indicates that the child > > + # is done, too. > > + output=$(git maintenance run --detach 2>&1 9>&1) && > > + printf "%s" "$output" >output && > > + test_must_be_empty output > > ) > > ' > > This looks correct, but should we be doing it for all of the "git > maintenance" runs in that script? They're all going to kick off detached > gc jobs, I think. Only those that use `--detach` run in the background. Patrick
On Mon, Aug 19, 2024 at 10:55:07AM +0200, Patrick Steinhardt wrote: > > This looks correct, but should we be doing it for all of the "git > > maintenance" runs in that script? They're all going to kick off detached > > gc jobs, I think. > > Only those that use `--detach` run in the background. I thought since the default for maintenance.autoDetach was true, all of the "--auto" ones would need something similar. I notice a lot of those use "--task", though, so maybe that doesn't count. I'm not clear on all of the rules. -Peff
On Mon, Aug 19, 2024 at 05:12:41AM -0400, Jeff King wrote: > On Mon, Aug 19, 2024 at 10:55:07AM +0200, Patrick Steinhardt wrote: > > > > This looks correct, but should we be doing it for all of the "git > > > maintenance" runs in that script? They're all going to kick off detached > > > gc jobs, I think. > > > > Only those that use `--detach` run in the background. > > I thought since the default for maintenance.autoDetach was true, all of > the "--auto" ones would need something similar. I notice a lot of those > use "--task", though, so maybe that doesn't count. I'm not clear on all > of the rules. `maintenance.autoDetach` only influences the auto-maintenance as executed by `run_auto_maintenance()`. Patrick
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 06ab43cfb5..074eadcd1c 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -967,8 +967,13 @@ test_expect_success '--detach causes maintenance to run in background' ' git config set maintenance.loose-objects.auto 1 && git config set maintenance.incremental-repack.enabled true && - git maintenance run --detach >out 2>&1 && - test_must_be_empty out + # The extra file descriptor gets inherited to the child + # process, and by reading stdout we thus essentially wait for + # that descriptor to get closed, which indicates that the child + # is done, too. + output=$(git maintenance run --detach 2>&1 9>&1) && + printf "%s" "$output" >output && + test_must_be_empty output ) '
One of the recently-added tests in t7900 exercises git-maintanance(1) with the `--detach` flag, which causes it to perform maintenance in the background. We do not wait for the backgrounded process to exit though, which causes the process to leak outside of the test, leading to racy behaviour. Fix this by synchronizing with the process via a separate file descriptor. This is the same workaround as we use in t6500, see the function `run_and_wait_for_auto_gc ()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- t/t7900-maintenance.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)