On Tue, Jun 25, 2024 at 7:40 AM Andres Freund <and...@anarazel.de> wrote: > > Hi, > > On 2024-06-25 13:26:23 +0300, Heikki Linnakangas wrote: > > While fixing a recent bug on visibility on a standby [1], I wrote a > > regression test that uses BackgroundPsql to run some queries in a > > long-running psql session. The problem is that that was refactored in v17, > > commit 664d757531. The test I wrote for v17 doesn't work as it is on > > backbranches. Options: > > > > 1. Write the new test differently on backbranches. Before 664d757531, the > > test needs to work a lot harder to use the background psql session, calling > > pump() etc. That's doable, but as noted in the discussion that led to > > 664d757531, it's laborious and error-prone. > > > > 2. Backport commit 664d757531. This might break out-of-tree perl tests that > > use the background_psql() function. I don't know if any such tests exist, > > and they would need to be changed for v17 anyway, so that seems acceptable. > > Anyone aware of any extensions using the perl test modules? > > > > 3. Backport commit 664d757531, but keep the existing background_psql() > > function unchanged. Add a different constructor to get the v17-style > > BackgroundPsql session, something like "$node->background_psql_new()". > > > > I'm leaning towards 3. We might need to backport more perl tests that use > > background_psql() in the future, backporting the test module will make that > > easier. > > > > Thoughts? > > Yes, I've wished for this a couple times. I think 2 or 3 would be reasonable. > I think 1) often just leads to either tests not being written or being > fragile...
+1 to backporting background psql! I'm also okay with 2 or 3. But note that for 2, there are several tests with skip_all and at least one of them uses background_psql (031_recovery_conflict.pl), so we'll just have to remember to update those. I assume that is easy enough to do if you grep for background_psql -- but, just in case you were going to be 100% test-driven :) - Melanie
