On Fri, Jan 5, 2024 at 5:08 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> >> I suggest we move test_injection_points from src/test/modules to
> >> contrib/ and rename it as "injection_points". The test files may still
> >> be named as test_injection_point. The TAP tests in 0003 and 0004 once
> >> moved to their appropriate places, will load injection_point extension
> >> and use it. That way predefined injection point callbacks will also be
> >> available for others to use.
> >
> > Rather than defining a module somewhere that tests would need to load,
> > should we just put the common callbacks in the core server? Unless there's
> > a strong reason to define them elsewhere, that could be a nice way to save
> > a step in the tests.
>
> Nah, having some pre-existing callbacks existing in the backend is
> against the original minimalistic design spirit. These would also
> require an SQL interface, and the interface design also depends on the
> functions registering them when pushing down custom conditions.
> Pushing that down to extensions to do what they want will lead to less
> noise, particularly if you consider that we will most likely want to
> tweak the callback interfaces for backpatched bugs. That's also why I
> think contrib/ is not a good idea, src/test/modules/ serving the
> actual testing purpose here.
Well, you have already showed that the SQL interface created for the
test module is being used for testing a core feature. The tests for
that should stay somewhere near the other tests for those features.
Using an extension named "test_injection_point" and which resides in a
test module for testing core features doesn't look great. Hence
suggestion to move it to contrib.
--
Best Wishes,
Ashutosh Bapat
