Re: pgsql: Add TAP test to automate the equivalent of check_guc

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Christoph Berg <myon(at)debian(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Add TAP test to automate the equivalent of check_guc
Date: 2022-02-11 17:29:29
Message-ID: 20220211172929.GF31460@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Fri, Feb 11, 2022 at 10:41:27AM -0500, Tom Lane wrote:
> Justin Pryzby <pryzby(at)telsasoft(dot)com> writes:
> > On Fri, Feb 11, 2022 at 09:59:55AM -0500, Tom Lane wrote:
> >> This seems like a pretty bad idea even if it weren't failing outright.
> >> We should be examining the version of the file that's in the source
> >> tree; the one in the installation tree might have version-skew
> >> problems, if you've not yet done "make install".
>
> > My original way used the source tree, but Michael thought it would be an issue
> > for "installcheck" where the config may not be available.
>
> Yeah, you are at risk either way, but in practice nobody is going to be
> running these TAP tests without a source tree.
>
> > This is what I had written:
> > FROM (SELECT regexp_split_to_table(pg_read_file('postgresql.conf'), '\n') AS ln) conf
>
> That's not using the source tree either, but the copy in the
> cluster-under-test. I'd fear it to be unstable in the buildfarm, where
> animals can append whatever they please to the config file being used by
> tests.

The important test is that "new GUCs exist in sample.conf". The converse test
is less interesting, so I think the important half would be okay.

I see the BF client appends to postgresql.conf.
https://github.com/PGBuildFarm/client-code/blob/main/run_build.pl#L1374

It could use "include", which was added in v8.2. Or it could add a marker,
like pg_regress does:
src/test/regress/pg_regress.c: fputs("\n# Configuration added by pg_regress\n\n", pg_conf);

If it were desirable to also check that "things that look like GUCs in the conf
are actually GUCs", either of those would avoid false positives.

Or, is it okay to use ABS_SRCDIR?

+\getenv abs_srcdir PG_ABS_SRCDIR
+\set filename :abs_srcdir '../../../../src/backend/utils/misc/postgresql.conf.sample'
+-- test that GUCS are in postgresql.conf
+SELECT lower(name) FROM tab_settings_flags WHERE NOT not_in_sample EXCEPT
+SELECT regexp_replace(ln, '^#?([_[:alpha:]]+) (= .*|[^ ]*$)', '\1') AS guc
+FROM (SELECT regexp_split_to_table(pg_read_file(:'filename'), '\n') AS ln) conf
+WHERE ln ~ '^#?[[:alpha:]]'
+ORDER BY 1;
+
+-- test that lines in postgresql.conf that look like GUCs are GUCs
+SELECT regexp_replace(ln, '^#?([_[:alpha:]]+) (= .*|[^ ]*$)', '\1') AS guc
+FROM (SELECT regexp_split_to_table(pg_read_file(:'filename'), '\n') AS ln) conf
+WHERE ln ~ '^#?[[:alpha:]]'
+EXCEPT SELECT lower(name) FROM tab_settings_flags WHERE NOT not_in_sample
+ORDER BY 1;

--
Justin

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2022-02-11 17:34:31 Re: pgsql: Add TAP test to automate the equivalent of check_guc
Previous Message Tom Lane 2022-02-11 15:41:27 Re: pgsql: Add TAP test to automate the equivalent of check_guc

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-02-11 17:34:31 Re: pgsql: Add TAP test to automate the equivalent of check_guc
Previous Message Andrew Dunstan 2022-02-11 17:11:40 Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints