Remove unneeded PGDATABASE setting from TAP tests
Hi,
Some of the TAP tests are explicitly setting PGDATABASE environment
variable to 'postgres', which isn't needed because the TAP test's perl
library PostgreSQL::Test::Cluster already sets it once and for all.
I've attached a patch that removes all such unneeded PGDATABASE
settings.
Thoughts?
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-Remove-unneeded-PGDATABASE-setting-from-TAP-tests.patchapplication/x-patch; name=v1-0001-Remove-unneeded-PGDATABASE-setting-from-TAP-tests.patchDownload+0-5
On Sun, Dec 31, 2023 at 07:24:08AM +0530, Bharath Rupireddy wrote:
Some of the TAP tests are explicitly setting PGDATABASE environment
variable to 'postgres', which isn't needed because the TAP test's perl
library PostgreSQL::Test::Cluster already sets it once and for all.
I've attached a patch that removes all such unneeded PGDATABASE
settings.Thoughts?
Hmm. I don't see any reason to not do that as these are not really
self-documenting either. How about 011_clusterdb_all.pl for
PGDATABASE?
--
Michael
On Sun, Dec 31, 2023 at 8:36 AM Michael Paquier <michael@paquier.xyz> wrote:
On Sun, Dec 31, 2023 at 07:24:08AM +0530, Bharath Rupireddy wrote:
Some of the TAP tests are explicitly setting PGDATABASE environment
variable to 'postgres', which isn't needed because the TAP test's perl
library PostgreSQL::Test::Cluster already sets it once and for all.
I've attached a patch that removes all such unneeded PGDATABASE
settings.Thoughts?
Hmm. I don't see any reason to not do that as these are not really
self-documenting either. How about 011_clusterdb_all.pl for
PGDATABASE?
Oh, yeah. We can remove that too, after all, PGDATABASE is being set
to postgres the default database which PostgreSQL::Test::Cluster does
set for us. I was earlier swayed by the comment atop the PGDATABASE
setting in 011_clusterdb_all.pl. Attached is v2 patch with this
change.
We perhaps can re-word and retain the comment in 011_clusterdb_all.pl,
which I think unnecessary as the code around 'alldb' if-else condition
in clusterdb.c can tell that.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v2-0001-Remove-unneeded-PGDATABASE-setting-from-TAP-tests.patchapplication/octet-stream; name=v2-0001-Remove-unneeded-PGDATABASE-setting-from-TAP-tests.patchDownload+0-9
On Mon, Jan 01, 2024 at 01:52:10PM +0530, Bharath Rupireddy wrote:
Oh, yeah. We can remove that too, after all, PGDATABASE is being set
to postgres the default database which PostgreSQL::Test::Cluster does
set for us. I was earlier swayed by the comment atop the PGDATABASE
setting in 011_clusterdb_all.pl. Attached is v2 patch with this
change.
Thanks. Applied after keeping the comment in 011_clusterdb_all.pl,
rewording it a bit.
--
Michael