Thread: Repeatability of installcheck for test_oat_hooks

Hi all,
(Andrew in CC.)

When running installcheck multiple times under src/test/modules/, we
are getting two failures as of test_pg_dump and test_oat_tests,
because these keep around some roles created by the tests.

Keeping around a role for test_pg_dump has been discussed already,
where the buildfarm can use that for pg_upgrade, and because there are
many objects that depend on the role created:
https://www.postgresql.org/message-id/20180904203012.GG20696@paquier.xyz

Note that it would require a DROP OWNED BY and DROP ROLE, but anyway:
--- a/src/test/modules/test_pg_dump/sql/test_pg_dump.sql
+++ b/src/test/modules/test_pg_dump/sql/test_pg_dump.sql
@@ -106,3 +106,5 @@ ALTER EXTENSION test_pg_dump DROP SERVER s0;
 ALTER EXTENSION test_pg_dump DROP TABLE test_pg_dump_t1;
 ALTER EXTENSION test_pg_dump DROP TYPE test_pg_dump_e1;
 ALTER EXTENSION test_pg_dump DROP VIEW test_pg_dump_v1;
+DROP OWNED BY regress_dump_test_role;
+DROP ROLE regress_dump_test_role;

As far as I can see, test_oat_hook has no need to keep around the
extra role it creates as part of the regression tests, because at the
end of the test there are no objects that depend on it.  Wouldn't it
be better to make the test self-isolated?  NO_INSTALLCHECK is set in
the module because of the issue with caching and the namespace search
hooks, but it seems to me that we'd better make the test self-isolated
in the long term, like in the attached.

Thoughts?
--
Michael

On Tue, Jun 28, 2022 at 10:12:48AM +0900, Michael Paquier wrote:
> As far as I can see, test_oat_hook has no need to keep around the
> extra role it creates as part of the regression tests, because at the
> end of the test there are no objects that depend on it.  Wouldn't it
> be better to make the test self-isolated?  NO_INSTALLCHECK is set in
> the module because of the issue with caching and the namespace search
> hooks, but it seems to me that we'd better make the test self-isolated
> in the long term, like in the attached.

And actually, I have found a second issue here.  The tests issue a
GRANT on work_mem, like that:
GRANT SET ON PARAMETER work_mem TO PUBLIC;

This has as effect to leave around an entry in pg_parameter_acl, which
is designed this way in aclchk.c.  However, this interacts with
guc_privs.sql in unsafe_tests, because those tests include similar
queries GRANT queries, also on work_mem.  So, if one issues an
installcheck on test_oat_modules followed by an installcheck in
unsafe_tests, the latter fails.  I think that we'd better add an extra
REVOKE to clear the contents of pg_parameter_acl.
--
Michael