On 2019-Jun-27, Tom Lane wrote:
> Further on the rolenames test mess: I started to work on removing
> that script's creation of out-of-spec user names, but my heart just
> sank to the floor when I noticed that it was also doing stuff like
> this:
>
> ALTER USER ALL SET application_name to 'SLAP';
> ALTER USER ALL RESET application_name;
>
> The extent to which that's Not OK inside a production installation
> is hard to overstate.
Uh-oh. I don't remember doing that, but evidently I did :-(
> At the same time, I can see that we'd want to have some coverage
> for that code path, so just deleting those tests isn't attractive.
Yeah ...
> What I propose that we do instead is invent an empty "module" under
> src/test/modules/ and install rolenames as a test for that.
Hmm, that's an idea, yes.
> Now, this doesn't in itself fix the problem that my proposed patch will
> emit warnings about the rolenames test script creating "Public" and so on.
> We could fix that by maintaining a variant expected-file that includes
> those warnings, but probably a less painful answer is just to jack
> client_min_messages up to ERROR for that short segment of the test script.
+1
> We could make the new subdirectory be something specific like
> "src/test/modules/test_rolenames", but I think very likely we'll be
> wanting some additional test scripts that we likewise deem unsafe to
> run during "installcheck". So I'd rather choose a more generic module
> name, but I'm not sure what ... "unsafe_tests"?
+0 for unsafe_tests, -1 for test_rolenames.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
