I think we should change the existing psql method to be what you propose as psql_expert. I don't see any advantage in keeping the old one. Many of the existing uses of psql should become what you call psql_check; but we should probably call that psql_ok() instead, and also have a psql_fails() for src/test/recovery/t/001_stream_rep.pl (and others to come).
I agree and that's what I really wanted to do. I just didn't want to produce a massive diff that renames the method across all of src/bin etc too, since I thought that'd be harder to commit and might have backporting consequences.
If you think that's the way to go I'm very happy with that and will proceed.
> +=item $node->backup_fs_hot(backup_name) > + > +Create a backup with a filesystem level copy in $node->backup_dir, > +including transaction logs. Archiving must be enabled as pg_start_backup > +and pg_stop_backup are used. This is not checked or enforced.
Perhaps "WAL archiving or streaming must be enabled ..."
Good point, will do.
I would rather have the patches be submitted with a reasonable approximation at indentation rather than submit a separate indent patch.
The reason I didn't do that is that the indenting in PostgresNode.pm is already well out of whack and, TBH, I didn't want to rebase on top of a perltidy'd version. I can bite the bullet and move the perltidy to the start of the patch series then make sure each subsequent patch is tidy'd but I'd want to be very sure you'd be OK to commit the perltidy of PostgresNode.pm otherwise I'd have to rebase messily all over again...