On Mon, Jul 24, 2023 at 1:10 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Jul 24, 2023 at 08:31:01AM +0530, Bharath Rupireddy wrote:
> > I also added a note atop worker_spi.c that the module also
> > demonstrates how to write core (SQL) tests and extended (TAP) tests.
>
> In terms of runtime the benefits are here for me. Note that with the
> first part of the test (previously in sql/), we don't lose coverage
> with the loop of the workers so I agree that only checking that these
> are launched is OK once worker_spi is in shared_preload_libraries.
> However, I think that we should make sure that they are connected to
> the correct database 'mydb'. I have updated the test to do that.
>
> So, what do you think about the attached?
I disagree with removing SQL tests from the worker_spi module. As said
upthread, it makes the worker_spi a fully demonstrable
extension/module - one can just take it, start adding required
functionality and test-cases (both SQL and TAP) for a new module. I
agree that moving to TAP tests will reduce test run time by 1.9
seconds, but to me personally this is not an optimization we must be
doing at the expense of demonstrability.
Having said that, others might have a different opinion here.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com