Thread: pgsql: Fix 007_sync_rep.pl to notice failures in ALTER SYSTEM SET.
Fix 007_sync_rep.pl to notice failures in ALTER SYSTEM SET. If a test case tried to set an invalid value of synchronous_standby_names, the test script didn't detect that, which seems like a bad idea. Noticed while testing a proposed patch that broke some of these test cases. Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/fb57f40eec503d637bf01c298f5cb2472f0d4fdb Modified Files -------------- src/test/recovery/t/007_sync_rep.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
On Mon, Aug 26, 2019 at 09:03:08PM +0000, Tom Lane wrote: > Fix 007_sync_rep.pl to notice failures in ALTER SYSTEM SET. > > If a test case tried to set an invalid value of synchronous_standby_names, > the test script didn't detect that, which seems like a bad idea. > Noticed while testing a proposed patch that broke some of these > test cases. Just for the note. I think that this needs a closer lookup and I am afraid that there are more issues like this one. For example, in 012_subtransactions.pl, pg_reload_conf() is called with PostgresNode::psql without checking for an error which is a bad idea after switching to synchonous replication. Most of the transactions done in this scrupt should also complain immediately on an error. -- Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes: > On Mon, Aug 26, 2019 at 09:03:08PM +0000, Tom Lane wrote: >> Fix 007_sync_rep.pl to notice failures in ALTER SYSTEM SET. > Just for the note. I think that this needs a closer lookup and I am > afraid that there are more issues like this one. Yeah, it might be a good idea to make a sweep to see what other tests should be using safe_psql. Or, more radically, we could think about redefining PostgresNode::psql to complain about errors, and you have to use something else if you want it not to? regards, tom lane
On Tue, Aug 27, 2019 at 11:38:28PM -0400, Tom Lane wrote: > Yeah, it might be a good idea to make a sweep to see what other tests > should be using safe_psql. I found more of these while browsing all the tests, so I am just going to start a new thread with a patch. > Or, more radically, we could think about redefining PostgresNode::psql > to complain about errors, and you have to use something else if you > want it not to? Not sure about redesigning that as it has been around for a couple of years now, and that would mean perhaps a third wrapper on top of PostgresNode::psql. -- Michael