On Fri, Nov 22, 2019 at 3:16 PM vignesh C <vignes...@gmail.com> wrote: > > Thanks for fixing the comments. The changes looks fine to me. I have > fixed the first comment, attached patch has the changes for the same. >
Few comments: -------------------------- 1. There is a lot of duplicate code in this test. Basically, almost the same code is used once to test Drop Database command and dropdb. I think we can create a few sub-functions to avoid duplicate code, but do we really need to test the same thing once via command and then by dropdb utility? I don't think it is required, but even if we want to do so, then I am not sure if this is the right test script. I suggest removing the command related test. 2. ok( TestLib::pump_until( + $killme, + $psql_timeout, + \$killme_stderr, + qr/FATAL: terminating connection due to administrator command/m + ), + "psql query died successfully after SIGTERM"); Extra space before TestLib. 3. +=item pump_until(proc, psql_timeout, stream, untl) I think moving pump_until to TestLib is okay, but if you are making it a generic function to be used from multiple places, it is better to name the variable as 'timeout' instead of 'psql_timeout' 4. Have you ran perltidy, if not, can you please run it? See src/test/perl/README for the recommendation. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
