Thread: Test 041_checkpoint_at_promote.pl faild in installcheck due to missing injection_points

Maxim Orlov <orlovmg@gmail.com> writes:
> After rebasing one of my old patches, I'm hit to a problem with the
> installcheck test for 041_checkpoint_at_promote.pl.

src/test/recovery/README points out that

  Also, to use "make installcheck", you must have built and installed
  contrib/pg_prewarm, contrib/pg_stat_statements and contrib/test_decoding
  in addition to the core code.

I suspect this needs some additional verbiage about also installing
src/test/modules/injection_points if you've enabled injection points.

(I think we haven't noticed because most people just use "make check"
instead.)

            regards, tom lane

Maxim Orlov <orlovmg@gmail.com> writes:
> So, my point here: installcheck-world doesn't
> work on the current master branch until I explicitly install
> injection_points extension.  In my view, it's a bit wired, since
> neither test_decoding, pg_stat_statements or pg_prewarm demand it.

Ugh.  The basic issue here is that "make install-world" doesn't
install anything from underneath src/test/modules, which I recall
as being an intentional decision.  Rather than poking a hole in
that policy for injection_points, I wonder if we should move it
to contrib.

            regards, tom lane

I wrote:
> Ugh.  The basic issue here is that "make install-world" doesn't
> install anything from underneath src/test/modules, which I recall
> as being an intentional decision.  Rather than poking a hole in
> that policy for injection_points, I wonder if we should move it
> to contrib.

... which would also imply writing documentation and so forth,
and it'd mean that injection_points starts to show up in end-user
installations.  (That would happen with the alternative choice of
hacking install-world to include src/test/modules/injection_points,
too.)  While you could argue that that'd be helpful for extension
authors who'd like to use injection_points in their own tests, I'm
not sure that it's where we want to go with that module.  It's only
meant as test scaffolding, and I don't think we've analyzed the
implications of some naive user installing it.

We do, however, need to preserve the property that installcheck
works after install-world.  I'm starting to think that maybe
the 041 test should be hacked to silently skip if it doesn't
find injection_points available.  (We could then remove some of
the makefile hackery that's supporting the current behavior.)
Probably the same needs to happen in each other test script
that's using injection_points --- I imagine that Maxim's
test is simply failing here first.

            regards, tom lane

On 2024-Aug-20, Tom Lane wrote:

> We do, however, need to preserve the property that installcheck
> works after install-world.  I'm starting to think that maybe
> the 041 test should be hacked to silently skip if it doesn't
> find injection_points available.

Yeah, I like this option.  Injection points require to be explicitly
enabled in configure, so skipping that test when injection_points can't
be found seems reasonable.

This also suggests that EXTRA_INSTALL should have injection_points only
when the option is enabled.

I've been curious about what exactly does this Makefile line
  export enable_injection_points enable_injection_points
achieve.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/

On 2024-Aug-23, Michael Paquier wrote:

> On Tue, Aug 20, 2024 at 12:30:35PM -0400, Alvaro Herrera wrote:
> > Yeah, I like this option.  Injection points require to be explicitly
> > enabled in configure, so skipping that test when injection_points can't
> > be found seems reasonable.
> 
> My apologies for the delay in doing something here.
> 
> The simplest thing would be to scan pg_available_extensions after the
> first node is started, like in the attached.  What do you think?

Hmm, I think you could test whether "--enable-injection-points" is in
the pg_config output in $node->config_data('--configure').  You don't
need to have started the node for that.

> > This also suggests that EXTRA_INSTALL should have injection_points only
> > when the option is enabled.
> 
> If the switch is disabled, the path is just ignored, so I don't see
> any harm in keeping it listed all the time.

Okay.

> > I've been curious about what exactly does this Makefile line
> >   export enable_injection_points enable_injection_points
> > achieve.
> 
> Without this line, the TAP tests would not be able to know if
> injection points are enabled or not, no?  Well, it is true that we
> could also do something like a scan of pg_config.h in the installation
> path, but this is consistent with what ./configure uses.

Right, I figured out afterwards that what that does is export the
make-level variable as an environment variable.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2024-Aug-23, Michael Paquier wrote:
>>> I've been curious about what exactly does this Makefile line
>>> export enable_injection_points enable_injection_points
>>> achieve.

>> Without this line, the TAP tests would not be able to know if
>> injection points are enabled or not, no?

> Right, I figured out afterwards that what that does is export the
> make-level variable as an environment variable.

It exports it twice, though, which is pretty confusing.

            regards, tom lane