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