Dong Wook Lee <sh95119@gmail.com> writes:
> I just wrote test about pg_rowlocks extension.
> I added sql and spec test for locking state.
I think this could be cut down quite a bit. Do we really need
both a SQL test and an isolation test? Seems like you could
easily do everything in the isolation test.
Also, it is not a good idea to go creating superusers in a contrib
test: we support "make installcheck" for these tests, but people don't
especially like new superusers cropping up in their installations.
I doubt that we need *any* of the permissions-ish tests that you
propose adding here; those are not part of the module's own
functionality, and we don't generally have similar tests in other
contrib modules.
If you do keep any of it, remember to drop the roles you create ---
leaving global objects behind is not OK. (For one thing, it
breaks doing repeat "make installcheck"s.)
Another thing that's bad style is the "drop table if exists".
This should be running in an empty database, and if somehow it's
not, destroying pre-existing objects would be pretty unfriendly.
Better to fail at the CREATE.
See also my comments about your pg_buffercache patch, which
largely apply here too.
regards, tom lane