Hi Ævar,

thanks for digging out this old thread, I really could have done never
thinking about it again!

On Wed, 10 Mar 2021, Ævar Arnfjörð Bjarmason wrote:

On Tue, Jan 29 2019, Johannes Schindelin via GitGitGadget wrote:

quoted

From: Johannes Schindelin <redacted>

It seems that every once in a while in the Git for Windows SDK, there
are some transient file locking issues preventing the test clean up to
delete the trash directory. Let's be gentle and try again five seconds
later, and only error out if it still fails the second time.

This change helps Windows, and does not hurt any other platform
(normally, it is highly unlikely that said deletion fails, and if it
does, normally it will fail again even 5 seconds later).

Signed-off-by: Johannes Schindelin <redacted>
---
 t/test-lib.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index f31a1c8f79..9c0ca5effb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -1104,7 +1104,11 @@ test_done () {
 			error "Tests passed but trash directory already removed before test cleanup; aborting"

 			cd "$TRASH_DIRECTORY/.." &&
-			rm -fr "$TRASH_DIRECTORY" ||
+			rm -fr "$TRASH_DIRECTORY" || {
+				# try again in a bit
+				sleep 5;
+				rm -fr "$TRASH_DIRECTORY"
+			} ||
 			error "Tests passed but test cleanup failed; aborting"
 		fi
 		test_at_end_hook_

I saw this sleep while reading some test-lib.sh code, doesn't this break
df4c0d1a79 (test-lib: abort when can't remove trash directory,
2017-04-20) for non-Windows platforms?

It does not really break it, it just delays the inevitable failure.

Your CL for v3 suggests this was only encountered in Azure VMs:
https://lore.kernel.org/git/pull.31.v3.git.gitgitgadget@gmail.com/ (local)

If by "CL" you refer to the cover letter, then I might have made it sound
as if it was only encountered in the Azure Pipelines agents. I vaguely
seem to remember seeing something like this quite often on my personal
machine, too, though. Most likely Microsoft Defender going a little wild.

Aside from this obscure issue, wouldn't it make more sense to have some
optional "I'm under CI" flag to skip the teardown one test at a time as
we're probably about to shut off the transitory VM soon?

No, I'm not under CI, and I did encounter these issues. And they abruptly
stopped with the patch you apparently still want to discuss ;-)

I skip some tests, but the test suite creates ~950MB of trash for
me. Maybe cheaper for some to just keep that around and have it all
removed at the end.

I don't understand this statement. Or was there a question in it that
you'd like me to answer?

Ciao,
Johannes