Date: Fri, 15 Jul 2016 23:54:56 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgreSQL.org
Subject: Re: Regression tests vs existing users in an installation
Message-ID: <20160716035456.GA181422@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <16638.1468620817@sss.pgh.pa.us>
User-Agent: Mutt/1.5.23 (2014-03-12)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

Tom Lane wrote:
> We've talked before about how the regression tests should be circumspect
> about what role names they create/drop, so as to avoid possibly blowing
> up an installation's existing users during "make installcheck".  In
> particular I believe there was consensus that such names should begin
> with, or at least include, "regress".  I got around today to instrumenting
> CreateRole to see what names we were actually creating, and was quite
> depressed as to how thoroughly that guideline is being ignored (see
> attached).
> 
> I propose to go through the regression tests and fix this (in HEAD only).

I would propose that we have one test run near the beginning or right at
the beginning of the serial schedule that sets up a small number of
roles to cover most of the needs of every other test, so that most such
other tests do not need to create any roles at all.  (Of course, there
would be a few cases where this would defeat the purpose of the test
because creating or dropping the role is precisely what is being
created; those cases would have additional roles, with the proposed
prefix.)

So currently we have 97 roles?  Probably we can get away with a dozen or
so, maybe even less than that.

> What I'm inclined to do with this is to reduce the test to be something
> like
> 
> BEGIN;
> CREATE ROLE "Public";
> CREATE ROLE "None";
> CREATE ROLE "current_user";
> CREATE ROLE "session_user";
> CREATE ROLE "user";
> ROLLBACK;
> 
> with maybe a couple of ALTERs and GRANTs inside the transaction to verify
> that the names can be referenced as well as created.  This would be safe
> against modifying any conflicting existing users; the only bad consequence
> would be a phony failure of the test.
>
> I thought about trying to preserve all the existing test cases while still
> keeping these roles inside a transaction, by inserting savepoints around
> the intentional failures.  But there are enough intentional failures in
> rolenames.sql that that would be really tedious.  The existing test cases
> seem enormously duplicative to me anyway, so I think a fairly short
> transaction with a few tests would be sufficient to cover this territory.


> A more aggressive answer would be to decide we don't need these test cases
> at all and drop them.

Hmm ... I think a blanket removal would go against generally accepted
principle that our tests need to cover more functionality.

Maybe we did go overboard on that one and the rolled-back creation is
enough test for that functionality.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers