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