On Tue, Jun 25, 2024 at 7:40 AM Andres Freund <andres@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