On Mon, Jul 24, 2023 at 1:10 PM Michael Paquier <mich...@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
