On Fri, Mar 28, 2025 at 10:44 AM Michael Paquier <mich...@paquier.xyz> wrote: > > On Thu, Mar 27, 2025 at 06:31:14PM +0900, Michael Paquier wrote: > > With all that in mind and more documentation added to the test, I've > > applied 0001, so let's see what the buildfarm has to say. The CI was > > stable, so it's a start. > > The buildfarm (particularly the Windows members that worried me), have > reported back and I am not seeing any failures, so we should be good > with 72c2f36d5727.
Thank you for review and additional modification to the patch. I'm glad the patch was made in time for this release, even if it was just a partial one. On Fri, Mar 28, 2025 at 8:57 AM Michael Paquier <mich...@paquier.xyz> wrote: > > I am not sure that I'll have the time to look at 0002 for this release > > cycle, could it be possible to get a rebase for it? > Here is a simple rebase that I have been able to assemble this > morning. I won't have the space to review it for this release cycle > unfortunately, but at least it works in the CI. I'm sorry I couldn't respond to your request :( > I am moving this patch entry to the next CF for v19, as a result of > that. OK Thanks :) On Thu, Mar 27, 2025 at 6:31 PM Michael Paquier <mich...@paquier.xyz> wrote: > I have spent quite a bit of time on the review 0001 with the new > tests to get something in for this release, and there was quite a bit > going on there: > - The script should set PGSYSCONFDIR, or it could grab data that > depend on the host. This can use the temporary folder created in the > test. > - On the same ground, we need a similar tweak for PGSERVICEFILE or we > would go into pqGetHomeDirectory() and look at a HOME folder (WIN32 > and non-WIN32). > > With that addressed, there could be much more tests, like for cases > where PGSERVICEFILE is set but points to a file that does not exist, > more combinations between URIs, connection parameters and PGSERVICE, > for success and failure cases, empty service file, etc. > > Another thing that I've noticed to be useful to cover is the case > based on the hardcoded service file name pg_service.conf in > PGSYSCONFDIR, which is used as a fallback in the code if the service > name cannot be found in the initial PGSERVICEFILE, acting as a > fallback option. As long as PGSYSCONFDIR is set, we could test one in > isolation using the temporary folder created by the test. I check and modify 0002 patch (adding servicefile option and its regression tests) in light of the above and committed 0001 patch (regression test of existing features) toward next release :) --- Great Regards, Ryo Kanbayashi
