Re: Add basic tests for the low-level backup method.

On Fri, Mar 15, 2024 at 09:40:38AM +1300, David Steele wrote:
> Is the missing test in meson the reason we did not see test failures for
> Windows in CI?

The test has to be listed in src/test/recovery/meson.build or the CI
would ignore it.

>> The second LOG is something that can be acted on.  I've added some
>> debugging to the parsing of the backup_label file in the backend, and
>> noticed that the first fscanf() for START WAL LOCATION is failing
>> because the last %c is detected as \r rather than \n.  Tweaking the
>> contents stored from pg_backend_stop() with a sed won't help, because
>> the issue is that we write the CRLFs with append_to_file, and the
>> startup process cannot cope with that.  The simplest method I can
>> think of is to use binmode, as of the attached.
>
> Yeah, that makes sense.

I am wondering if there is a better trick here that would not require
changes in the backend to make the backup_label parsing more flexible,
though.

>> I am attaching an updated patch with all that fixed, which is stable
>> in the CI and any tests I've run.  Do you have any comments about
>
> These changes look good to me. Sure wish we had an easier to way to test
> commits in the build farm.

That's why these tests are not that easy, they can be racy.  I've run
the test 5~10 times in the CI this time to gain more confidence, and
saw zero failures with the stability fixes in place including Windows.
I've applied it now, as I can still monitor the buildfarm for a few
more days.  Let's see what happens, but that should be better.
--
Michael