Re: t7527 intermittent failure on macOS APFS and possible fix

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 12, 2022, 18:08 UTC
Message-ID: <xmqqmtc9baht.fsf@gitster.g>
In-Reply-To: <YvZbGAf+82WtNXcJ@danh.dev>

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

Show 10 quoted lines

> Running t7527 on macOS with encrypted APFS filesystem.
> I observes intermittent failure, however, when I manually check the
> test cases, they're all passed.
>
> I suspected fileystem caching issue.
> I added those sync-s into test steps and the test pass.
> I'm not sure if this is the intending "fix" for the tests
> since we're testing the fsmonitor with t7527.
>
> Please advise!

fsmonitor experts, please do.

My gut feeling is that, unless the production code internally calls the equivalent of "sync" done here and the failure in tests are coming from the fact that the "sync" is missing in "test-tool fsmonitor-client" (i.e. test-tool does not emulate the real world closely enough and fails in cases where the machinery does not fail in the real world), these "sync" calls only sweep the problem under the rug.

Show 26 quoted lines

> P/S: When debugging, I also found out that:
> "test-tool fsmonitor-client query" doesn't write the final newline
> character, thus making the output harder to read. The diff also have
> the final newline added.
>
> ----- 8< -------
> diff --git a/t/helper/test-fsmonitor-client.c b/t/helper/test-fsmonitor-client.c
> index 54a4856c48..98d6cf1440 100644
> --- a/t/helper/test-fsmonitor-client.c
> +++ b/t/helper/test-fsmonitor-client.c
> @@ -55,6 +55,7 @@ static int do_send_query(const char *token)
>  
>  	write_in_full(1, answer.buf, answer.len);
>  	strbuf_release(&answer);
> +	write_in_full(1, "\n", 1);
>  
>  	return 0;
>  }
> @@ -77,6 +78,7 @@ static int do_send_flush(void)
>  
>  	write_in_full(1, answer.buf, answer.len);
>  	strbuf_release(&answer);
> +	write_in_full(1, "\n", 1);
>  
>  	return 0;
>  }

Aren't these protocol drivers? If the protocol is defined without the trailing LF, would it make sense to update only the sending end to do this? Or does the protocol makes it clear that a trailing LF, or lack of it, should be tolerated by all the implementations?

If we are absolutely sure that no implementation of the other side will get upset by seeing an extra LF, It would be fine, but as the original code wants to call write_in_full(), it would be more preferrable to do it this way instead, I suspect.

+	strbuf_complete(&answer, '\n');
	write_in_full(1, answer.buf, answer.len);
	strbuf_release(&answer);