{"thread":{"id":"64824","subject":"[PATCH 2/2] t0610-reftable-basics: mitigate a flaky test on cygwin","startedAt":"2026-01-16T20:42:58Z","lastAt":"2026-01-20T05:49:13Z","messageCount":4,"participants":["Ramsay Jones","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"534088","messageId":"f46e023b-1925-41b2-9842-42e7cb727056@ramsayjones.plus.com","threadId":"64824","inReplyTo":null,"subject":"[PATCH 2/2] t0610-reftable-basics: mitigate a flaky test on cygwin","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2026-01-16T20:39:56Z","receivedAt":"2026-01-16T20:42:58Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nTest #29 ('ref transaction: corrupted tables cause failure') started to\nfail intermittently for me (from v2.52.0-rc0) when running the testsuite\nwith '-j8'. (Also, having moved to a new laptop and windows 11, rather\nthan windows 10). If the test is run by hand, or without any parallelism,\nthen it passes without issue.\n\nWhen the test fails (e.g. 1 out of 32 parallel runs) the cause is due to\na permission error while corrupting a table file:\n\n  ./test-lib.sh: line 1010: .git/reftable/0x000000000001-0x000000000002-d89bb8ee.ref: Permission denied\n\nThis corruption is done in a shell loop, directly after a 'test_commit',\nwhich uses an ': >\"$f\"' expression to truncate the file. Adding a sleep\nof one second after the 'test_commit' and before the shell loop fixes\nthe test (it is not clear why). Replacing the redirection shell expression\nwith a 'test-tool truncate \"$f\" 0' invocation also provides a fix, which\ncould simply be another way to change the timing sufficiently to win the\nrace.\n\nDuring a debug session, I tried looking at the strace output for the\nshell redirection:\n\n  $ rm /tmp/hello; echo hello >/tmp/hello; ls -l /tmp/hello\n  -rw-r--r-- 1 ramsay None 6 Nov 10 17:25 /tmp/hello\n  $\n\n  $ strace -o zzz bash -c ': >/tmp/hello'\n  $\n\nSimilarly, for the test-tool solution:\n\n  $ strace -o xxx ./t/helper/test-tool truncate /tmp/hello 0\n  $\n\nWhen comparing the output, the differences seemed to be what you would\nexpect and, if anything, the shell redirect probably would have taken\nlonger than the test-tool solution (many fcntl() calls to dup the stdout\nto the <fd>).  The call to the win32 api NtCreateFile() was identical,\napart from the first (FileHandle) parameter, of course.\n\nIn order to fix this flaky test on cygwin, despite not knowing why it\nworks, replace the shell redirection with the above 'test-tool truncate'\ninvocation.\n\nHelped-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>\n---\n t/t0610-reftable-basics.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh\nindex 6575528f21..e19e036898 100755\n--- a/t/t0610-reftable-basics.sh\n+++ b/t/t0610-reftable-basics.sh\n@@ -207,7 +207,7 @@ test_expect_success 'ref transaction: corrupted tables cause failure' '\n \t\ttest_commit file1 &&\n \t\tfor f in .git/reftable/*.ref\n \t\tdo\n-\t\t\t: >\"$f\" || return 1\n+\t\t\ttest-tool truncate \"$f\" 0 || return 1\n \t\tdone &&\n \t\ttest_must_fail git update-ref refs/heads/main HEAD\n \t)\n-- \n2.52.0\n"},{"id":"534170","messageId":"aW3UO3ff9aNc7HQz@pks.im","threadId":"64824","inReplyTo":"f46e023b-1925-41b2-9842-42e7cb727056@ramsayjones.plus.com","subject":"Re: [PATCH 2/2] t0610-reftable-basics: mitigate a flaky test on cygwin","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-19T06:50:35Z","receivedAt":"2026-01-19T06:50:41Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Jan 16, 2026 at 08:39:56PM +0000, Ramsay Jones wrote:\n> \n> Test #29 ('ref transaction: corrupted tables cause failure') started to\n> fail intermittently for me (from v2.52.0-rc0) when running the testsuite\n> with '-j8'. (Also, having moved to a new laptop and windows 11, rather\n> than windows 10). If the test is run by hand, or without any parallelism,\n> then it passes without issue.\n> \n> When the test fails (e.g. 1 out of 32 parallel runs) the cause is due to\n> a permission error while corrupting a table file:\n> \n>   ./test-lib.sh: line 1010: .git/reftable/0x000000000001-0x000000000002-d89bb8ee.ref: Permission denied\n\nThis rings a bell. I remember that we discussed a case at some point in\ntime where a redirect converted to `test-tool truncate` fixed a flake on\nCygwin.\n\n> This corruption is done in a shell loop, directly after a 'test_commit',\n> which uses an ': >\"$f\"' expression to truncate the file. Adding a sleep\n> of one second after the 'test_commit' and before the shell loop fixes\n> the test (it is not clear why). Replacing the redirection shell expression\n> with a 'test-tool truncate \"$f\" 0' invocation also provides a fix, which\n> could simply be another way to change the timing sufficiently to win the\n> race.\n> \n> During a debug session, I tried looking at the strace output for the\n> shell redirection:\n> \n>   $ rm /tmp/hello; echo hello >/tmp/hello; ls -l /tmp/hello\n>   -rw-r--r-- 1 ramsay None 6 Nov 10 17:25 /tmp/hello\n>   $\n> \n>   $ strace -o zzz bash -c ': >/tmp/hello'\n>   $\n> \n> Similarly, for the test-tool solution:\n> \n>   $ strace -o xxx ./t/helper/test-tool truncate /tmp/hello 0\n>   $\n> \n> When comparing the output, the differences seemed to be what you would\n> expect and, if anything, the shell redirect probably would have taken\n> longer than the test-tool solution (many fcntl() calls to dup the stdout\n> to the <fd>).  The call to the win32 api NtCreateFile() was identical,\n> apart from the first (FileHandle) parameter, of course.\n\nToo bad. I stil wonder whether it is the extra process that we spawn\nthat ends up fixing the issue.\n\n> In order to fix this flaky test on cygwin, despite not knowing why it\n> works, replace the shell redirection with the above 'test-tool truncate'\n> invocation.\n> \n> Helped-by: Patrick Steinhardt <ps@pks.im>\n\nOh, so is this the exact case that we were talking about? If so, it\nmight make sense to link to the mail thread so that folks can also read\na bit into our discussion around this.\n\n> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>\n> ---\n>  t/t0610-reftable-basics.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh\n> index 6575528f21..e19e036898 100755\n> --- a/t/t0610-reftable-basics.sh\n> +++ b/t/t0610-reftable-basics.sh\n> @@ -207,7 +207,7 @@ test_expect_success 'ref transaction: corrupted tables cause failure' '\n>  \t\ttest_commit file1 &&\n>  \t\tfor f in .git/reftable/*.ref\n>  \t\tdo\n> -\t\t\t: >\"$f\" || return 1\n> +\t\t\ttest-tool truncate \"$f\" 0 || return 1\n>  \t\tdone &&\n>  \t\ttest_must_fail git update-ref refs/heads/main HEAD\n>  \t)\n\nIn any case, if it seems to reliably fix the issue I'd say we just merge\nit. It's unfortunate that we haven't been able to figure out the root\ncause, but so be it.\n\nThanks!\n\nPatrick\n"},{"id":"534187","messageId":"f4599b1e-78df-44f3-a9b8-ed28411e169c@ramsayjones.plus.com","threadId":"64824","inReplyTo":"aW3UO3ff9aNc7HQz@pks.im","subject":"Re: [PATCH 2/2] t0610-reftable-basics: mitigate a flaky test on cygwin","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2026-01-19T17:10:46Z","receivedAt":"2026-01-19T17:13:57Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 19/01/2026 6:50 am, Patrick Steinhardt wrote:\n> On Fri, Jan 16, 2026 at 08:39:56PM +0000, Ramsay Jones wrote:\n>>\n>> Test #29 ('ref transaction: corrupted tables cause failure') started to\n>> fail intermittently for me (from v2.52.0-rc0) when running the testsuite\n>> with '-j8'. (Also, having moved to a new laptop and windows 11, rather\n>> than windows 10). If the test is run by hand, or without any parallelism,\n>> then it passes without issue.\n>>\n>> When the test fails (e.g. 1 out of 32 parallel runs) the cause is due to\n>> a permission error while corrupting a table file:\n>>\n>>   ./test-lib.sh: line 1010: .git/reftable/0x000000000001-0x000000000002-d89bb8ee.ref: Permission denied\n> \n> This rings a bell. I remember that we discussed a case at some point in\n> time where a redirect converted to `test-tool truncate` fixed a flake on\n> Cygwin.\n\nIndeed, the mail thread starts at:\n\n  https://lore.kernel.org/git/f22c95ad-43c8-41de-8315-e707224e830b@ramsayjones.plus.com/\n\n>> This corruption is done in a shell loop, directly after a 'test_commit',\n>> which uses an ': >\"$f\"' expression to truncate the file. Adding a sleep\n>> of one second after the 'test_commit' and before the shell loop fixes\n>> the test (it is not clear why). Replacing the redirection shell expression\n>> with a 'test-tool truncate \"$f\" 0' invocation also provides a fix, which\n>> could simply be another way to change the timing sufficiently to win the\n>> race.\n>>\n>> During a debug session, I tried looking at the strace output for the\n>> shell redirection:\n>>\n>>   $ rm /tmp/hello; echo hello >/tmp/hello; ls -l /tmp/hello\n>>   -rw-r--r-- 1 ramsay None 6 Nov 10 17:25 /tmp/hello\n>>   $\n>>\n>>   $ strace -o zzz bash -c ': >/tmp/hello'\n>>   $\n>>\n>> Similarly, for the test-tool solution:\n>>\n>>   $ strace -o xxx ./t/helper/test-tool truncate /tmp/hello 0\n>>   $\n>>\n>> When comparing the output, the differences seemed to be what you would\n>> expect and, if anything, the shell redirect probably would have taken\n>> longer than the test-tool solution (many fcntl() calls to dup the stdout\n>> to the <fd>).  The call to the win32 api NtCreateFile() was identical,\n>> apart from the first (FileHandle) parameter, of course.\n> \n> Too bad. I stil wonder whether it is the extra process that we spawn\n> that ends up fixing the issue.\n\nWell, a 'sleep 1' before the shell loop also fixes the issue. I hate to\nmention the 'windows delays updating some file attributes until after the\nprocess has exited' conspiracy theory, but ... :) (yeah, I just don't\nthink that is possible, except ...)\n\n>> In order to fix this flaky test on cygwin, despite not knowing why it\n>> works, replace the shell redirection with the above 'test-tool truncate'\n>> invocation.\n>>\n>> Helped-by: Patrick Steinhardt <ps@pks.im>\n> \n> Oh, so is this the exact case that we were talking about? If so, it\n> might make sense to link to the mail thread so that folks can also read\n> a bit into our discussion around this.\n\nIndeed! I thought about referencing the email thread, but I decided that it\ndidn't really offer any more supporting evidence than the commit message\n(in fact less - it doesn't mention the 'strace' scan).\n\nI can add that (again [1]), if you think it's worth it, but I just re-read\nthe email thread and I'm not convinced it offers much extra value. So, I would\nrather not re-roll, but I will if you think it worth it. Let me know.\n\n[1] https://lore.kernel.org/git/f22c95ad-43c8-41de-8315-e707224e830b@ramsayjones.plus.com/\n\n>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>\n>> ---\n>>  t/t0610-reftable-basics.sh | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh\n>> index 6575528f21..e19e036898 100755\n>> --- a/t/t0610-reftable-basics.sh\n>> +++ b/t/t0610-reftable-basics.sh\n>> @@ -207,7 +207,7 @@ test_expect_success 'ref transaction: corrupted tables cause failure' '\n>>  \t\ttest_commit file1 &&\n>>  \t\tfor f in .git/reftable/*.ref\n>>  \t\tdo\n>> -\t\t\t: >\"$f\" || return 1\n>> +\t\t\ttest-tool truncate \"$f\" 0 || return 1\n>>  \t\tdone &&\n>>  \t\ttest_must_fail git update-ref refs/heads/main HEAD\n>>  \t)\n> \n> In any case, if it seems to reliably fix the issue I'd say we just merge\n> it. It's unfortunate that we haven't been able to figure out the root\n> cause, but so be it.\n\nAgreed!\n\nThanks!\n\nATB,\nRamsay Jones\n\n\n\n"},{"id":"534210","messageId":"aW8XUqlbiJgr8Eib@pks.im","threadId":"64824","inReplyTo":"f4599b1e-78df-44f3-a9b8-ed28411e169c@ramsayjones.plus.com","subject":"Re: [PATCH 2/2] t0610-reftable-basics: mitigate a flaky test on cygwin","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-20T05:49:06Z","receivedAt":"2026-01-20T05:49:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Jan 19, 2026 at 05:10:46PM +0000, Ramsay Jones wrote:\n> On 19/01/2026 6:50 am, Patrick Steinhardt wrote:\n> > On Fri, Jan 16, 2026 at 08:39:56PM +0000, Ramsay Jones wrote:\n> >> In order to fix this flaky test on cygwin, despite not knowing why it\n> >> works, replace the shell redirection with the above 'test-tool truncate'\n> >> invocation.\n> >>\n> >> Helped-by: Patrick Steinhardt <ps@pks.im>\n> > \n> > Oh, so is this the exact case that we were talking about? If so, it\n> > might make sense to link to the mail thread so that folks can also read\n> > a bit into our discussion around this.\n> \n> Indeed! I thought about referencing the email thread, but I decided that it\n> didn't really offer any more supporting evidence than the commit message\n> (in fact less - it doesn't mention the 'strace' scan).\n> \n> I can add that (again [1]), if you think it's worth it, but I just re-read\n> the email thread and I'm not convinced it offers much extra value. So, I would\n> rather not re-roll, but I will if you think it worth it. Let me know.\n> \n> [1] https://lore.kernel.org/git/f22c95ad-43c8-41de-8315-e707224e830b@ramsayjones.plus.com/\n\nFair enough, let's just keep it as-is. Thanks!\n\nPatrick\n"}]}