Thread: pgsql: Drop test user when done with it.
Drop test user when done with it. Commit d7f8d26d9 added a test case that created a user, but forgot to drop it again. This is no good; for one thing, it causes repeated "make installcheck" runs to fail. Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/f31111bbe81db0e84fb486c6423a234c47091b30 Modified Files -------------- src/test/regress/expected/stats_ext.out | 1 + src/test/regress/sql/stats_ext.sql | 1 + 2 files changed, 2 insertions(+)
On Mon, 24 Jun 2019 at 17:37, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Drop test user when done with it. > > Commit d7f8d26d9 added a test case that created a user, but forgot > to drop it again. This is no good; for one thing, it causes repeated > "make installcheck" runs to fail. > Ah, I see .. yes, my bad. Thanks for fixing. Regards, Dean
On Mon, Jun 24, 2019 at 04:37:13PM +0000, Tom Lane wrote: > Drop test user when done with it. > > Commit d7f8d26d9 added a test case that created a user, but forgot > to drop it again. This is no good; for one thing, it causes repeated > "make installcheck" runs to fail. If we are on that, we still have src/test/modules/test_pg_dump/ which is not repeatable with multiple installchecks: https://www.postgresql.org/message-id/20181130163728.GE3415@tamriel.snowman.net -- Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes: > On Mon, Jun 24, 2019 at 04:37:13PM +0000, Tom Lane wrote: >> Commit d7f8d26d9 added a test case that created a user, but forgot >> to drop it again. This is no good; for one thing, it causes repeated >> "make installcheck" runs to fail. > If we are on that, we still have src/test/modules/test_pg_dump/ which > is not repeatable with multiple installchecks: > https://www.postgresql.org/message-id/20181130163728.GE3415@tamriel.snowman.net OK, hadn't run into that personally, but let's fix that too. Anything that can be run with "installcheck" has to satisfy the restrictions of being re-runnable and careful about what global names it uses. regards, tom lane
I wrote: > Michael Paquier <michael@paquier.xyz> writes: >> If we are on that, we still have src/test/modules/test_pg_dump/ which >> is not repeatable with multiple installchecks: >> https://www.postgresql.org/message-id/20181130163728.GE3415@tamriel.snowman.net > OK, hadn't run into that personally, but let's fix that too. Anything > that can be run with "installcheck" has to satisfy the restrictions > of being re-runnable and careful about what global names it uses. Actually, now that I re-read that thread, maybe what we need to do is mark test_pg_dump as something not to be run by "make installcheck"? I'm not quite sure what *does* run it, but the context is evidently that it's supposed to be run by some overarching script that's then going to run pg_dump on the ending database state. So it's not meant to be run against a generic pre-existing installation, and it doesn't have to follow the rules for being safe for that --- but then we need to be sure that it doesn't get called that way if someone does "make installcheck" in a parent directory. regards, tom lane
Greetings,
On Wed, Jun 26, 2019 at 10:51 Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> If we are on that, we still have src/test/modules/test_pg_dump/ which
>> is not repeatable with multiple installchecks:
>> https://www.postgresql.org/message-id/20181130163728.GE3415@tamriel.snowman.net
> OK, hadn't run into that personally, but let's fix that too. Anything
> that can be run with "installcheck" has to satisfy the restrictions
> of being re-runnable and careful about what global names it uses.
Actually, now that I re-read that thread, maybe what we need to do is
mark test_pg_dump as something not to be run by "make installcheck"?
I'm not quite sure what *does* run it, but the context is evidently
that it's supposed to be run by some overarching script that's then
going to run pg_dump on the ending database state. So it's not meant
to be run against a generic pre-existing installation, and it doesn't
have to follow the rules for being safe for that --- but then we need
to be sure that it doesn't get called that way if someone does "make
installcheck" in a parent directory.
On my phone atm, but this feels very deja vu...
Isn’t this the one run from pg_upgrade’s tests? We don’t want to break that (and hopefully we haven’t but maybe something did...). Pretty sure we had nearly the same discussion this past fall...
Thanks,
Stephen
On 2019-Jun-26, Stephen Frost wrote: > Isn’t this the one run from pg_upgrade’s tests? We don’t want to break that > (and hopefully we haven’t but maybe something did...). Pretty sure we had > nearly the same discussion this past fall... https://postgr.es/m/20180904203012.GG20696@paquier.xyz ? -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Greetings, * Alvaro Herrera (alvherre@2ndquadrant.com) wrote: > On 2019-Jun-26, Stephen Frost wrote: > > > Isn’t this the one run from pg_upgrade’s tests? We don’t want to break that > > (and hopefully we haven’t but maybe something did...). Pretty sure we had > > nearly the same discussion this past fall... > > https://postgr.es/m/20180904203012.GG20696@paquier.xyz ? Yes, thanks, and specifically this: https://www.postgresql.org/message-id/20181130163728.GE3415%40tamriel.snowman.net Adding 'DROP IF EXISTS' to the top to have things cleaned up so that the run can be repeated is fine. Removing things at the end of the test script would defeat the entire purpose of those tests. Thanks! Stephen
Attachment
On Wed, Jun 26, 2019 at 03:07:33PM -0400, Stephen Frost wrote: > Yes, thanks, and specifically this: > > https://www.postgresql.org/message-id/20181130163728.GE3415%40tamriel.snowman.net > > Adding 'DROP IF EXISTS' to the top to have things cleaned up so that the > run can be repeated is fine. Removing things at the end of the test > script would defeat the entire purpose of those tests. The problem is that if you do an installcheck, then the role persists in the instance of Postgres installed. That's not a good thing, and contrary to the test policy. And your solution does nothing about that. -- Michael