On Mon, Jul 18, 2022 at 03:11:51PM +0800, Julien Rouhaud wrote: > So first, even if we can test 99% of the features with just testing the views > output, I think it's should use the TAP framework since the tests will have to > mess with the pg_ident/pg_hba files. It's way easier to modify the auth > files, > and it uses a dedicated instance so we don't have to worry about breaking > other > test that would run concurrently.
Agreed. > That may still be workable by splitting the output per newline (and possibly > removing the first prompt before sending the query text), and remove the first > and last entry (assuming you want to test somewhat sane data, and not e.g. run > the regression test on a database containing a newline), but then you have to > also account for possible escape sequences, for instance if you use > enable-bracketed-paste. In that case, the output becomes something like > > dbname=# SELECT 1; > [?2004l > 1 > [?2004hpostgres=# > > It could probably be handled with some regexp to remove escape sequences and > remove empty lines, but it seems like really fragile, and thus a very bad > idea. Hmm. Indeed, that sounds fragile. And I don't really think that we should make more testing infrastructure a requirement for this patch as being able to maintain a connection while running the tests is something we've bumped on for a bit of time. > I'm not really sure what should be done here. The best compromise I can think > of is to split the tests in 3 parts: > > 1) view reporting with various inclusions using safe_psql() You mean in the case where the HBA and indent files can be loaded, so as it is possible to peek at the system views without the EXEC_BACKEND problem, right? > 2) log error reporting This one should be reliable and stable enough by parsing the logs of the backend, thanks to connect_ok() and connect_fails(). > 3) view reporting with various inclusions errors, using safe_psql() > > And when testing 3), detect first if we can still connect after introducing > errors. If not, assume this is Windows / EXEC_BACKEND and give up here > without > reporting an error. Otherwise continue, and fail the test if we later can't > connect anymore. As discussed previously, detecting that the build is using > the fork emulation code path doesn't seem worthwhile so guessing it from the > psql error may be a better approach. Yeah, we could do that. Now we may also fail on other patterns, so we would need to make sure that a set of expected error outputs are the ones generated? I'd be fine to give up testing the error output generated in the system views at the end. Without a persistent connection state with the same kind of APIs as any of the drivers able to do so, that's going to be a PITA to maintain. -- Michael
signature.asc
Description: PGP signature