[PATCH] t6500: don't run detached auto gc at the end of the test script
- From
SZEDER Gábor <szeder.dev@gmail.com>
- Date
- Apr 10, 2017, 12:59 UTC
- Message-ID
- <20170410125911.6800-1-szeder.dev@gmail.com>
The last test in 't6500-gc', 'background auto gc does not run if gc.log is present and recent but does if it is old', added in a831c06a2 (gc: ignore old gc.log files, 2017-02-10), may sporadically trigger an error message from the test harness:
rm: cannot remove 'trash directory.t6500-gc/.git/objects': Directory not empty
The test in question ends with executing an auto gc in the backround, which occasionally takes so long that it's still running when 'test_done' is about to remove the trash directory. This 'rm -rf $trash' in the foreground might race with the detached auto gc to create and delete files and directories, and gc might (re-)create a path that 'rm' already visited and removed, triggering the above error when 'rm' attempts to remove its parent directory.
Commit bb05510e5 (t5510: run auto-gc in the foreground, 2016-05-01) fixed the same problem in a different test script by simply disallowing background gc. Unfortunately, what worked there is not applicable here, because the purpose of this test is to check the behavior of a detached auto gc.
Move the detached auto gc execution further away from the end of the test script by splitting the test in two: run the 'background auto gc runs if an old gc.log is present' part first, leaving only the aborting auto gc executions in the last test. Run the first test's detached auto gc in a dedicated sub-repository to prevent it from interfering with the subseqent test's gc. Also add a comment at the end of the test script to warn developers of future tests about this issue.
While this change doesn't completely eliminate the possibility of this race, it significantly and seemingly sufficiently reduces its probability. Running t6500 in a loop while my box was under heavy CPU and I/O load usually triggered the error under 15 seconds, but with this patch it run overnight without incident.
(Alternatively, we could check the presence of the gc.pid file after starting the detached auto gc, and wait while it's there. However, this would create a different race: the auto gc process creates the pid file after it goes to the background, so our check in the foreground might racily look for the pid file before it's even created, and thus would not wait for the background gc to finish. Furthermore, this would open up the possibility of the test hanging if something were to go wrong with the gc process and the pid file were not removed.)
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- t/t6500-gc.sh | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh index 08de2e8ab..2dc202c9b 100755 --- a/t/t6500-gc.sh +++ b/t/t6500-gc.sh @@ -67,7 +67,27 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre test_line_count = 2 new # There is one new pack and its .idx ' -test_expect_success 'background auto gc does not run if gc.log is present and recent but does if it is old' ' +test_expect_success 'background auto gc runs if an old gc.log is present' ' + # The detached auto gc process can run long enough in the + # background to racily interfere with the gc execution in the + # subsequent test, hence the dedicated repository. + test_create_repo other && + ( + cd other && + test_commit foo && + git repack && + test_commit bar && + git repack && + git config gc.autopacklimit 1 && + git config gc.autodetach true && + git config gc.logexpiry 2.days && + echo fleem >.git/gc.log && + test-chmtime =-345600 .git/gc.log && # 4 days + git gc --auto + ) +' + +test_expect_success 'background auto gc does not run if gc.log is present' ' test_commit foo && test_commit bar && git repack && @@ -77,10 +97,12 @@ test_expect_success 'background auto gc does not run if gc.log is present and re test_must_fail git gc --auto 2>err && test_i18ngrep "^error:" err && test_config gc.logexpiry 5.days && - test-chmtime =-345600 .git/gc.log && - test_must_fail git gc --auto && - test_config gc.logexpiry 2.days && - git gc --auto + test-chmtime =-345600 .git/gc.log && # 4 days + test_must_fail git gc --auto ' +# DO NOT leave a detached auto gc process running near the end of the +# test script: it can run long enough in the background to racily +# interfere with the cleanup in 'test_done'. + test_done
-- 2.12.2.613.g9c5b79913