On Fri, Jan 5, 2024 at 5:08 AM Michael Paquier <mich...@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