Add TAP test to automate the equivalent of check_guc, take two

src/backend/utils/misc/check_guc is a script that cross-checks the
consistency of the GUCs with postgresql.conf.sample, making sure that
its format is in line with what guc.c has.  It has never been run
automatically, and has rotten over the years, creating a lot of false
positives as per a report from Justin Pryzby.

d10e41d has introduced a SQL function to publish the most relevant flags
associated to a GUC, with tests added in the main regression test suite
to make sure that we avoid most of the inconsistencies in the GUC
settings, based on recent reports, but there was nothing able to
cross-check postgresql.conf.sample with the contents of guc.c.

This commit adds a TAP test that covers the remaining gap.  It emulates
the most relevant checks that check_guc did, so as any format mistakes
are detected in postgresql.conf.sample at development stage, with the
following checks:
- Check that parameters marked as NOT_IN_SAMPLE are not in the sample
file.
- Check that there are no dead entries in postgresql.conf.sample for
parameters not marked as NOT_IN_SAMPLE.
- Check that no parameters are missing from the sample file if listed in
guc.c without NOT_IN_SAMPLE.

The idea of building a list of the GUCs by parsing the sample file comes
from Justin, and he wrote the regex used in the patch to find all the
GUCs (this same formatting rule basically applies for the last 20~ years
or so).  In order to test this patch, I have played with manual
modifications of postgresql.conf.sample and guc.c, making sure that we
detect problems with the GUC rules and the sample file format.

The test is located in src/test/modules/test_misc, which is the best
location I could think about for such sanity checks, rather than the
main regression test suite (src/test/regress) to avoid a new type of
dependency with the source tree.

The first attempt of this patch was b0a55f4, where the location of
postgresql.conf.sample was retrieved using pg_config --sharedir.  This
has proven to be an issue for distributions that patch pg_config to
enforce the installation paths at some wanted location (like Debian),
that may not exist when the test is run, hence causing a failure.
Instead of that, as per a suggestion from Andres Freund, rely on the
fact that the test is always executed from its directory in the source
tree and use a relative path to find the sample file.  This works for
the CI, VPATH builds and on Windows, and tests like the recovery one
added in f47ed79 rely on that already.

Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/[email protected]

diff --git a/src/test/modules/test_misc/t/003_check_guc.pl b/src/test/modules/test_misc/t/003_check_guc.pl
new file mode 100644 (file)
index 0000000..60459ef
--- /dev/null
+++ b/src/test/modules/test_misc/t/003_check_guc.pl
@@ -0,0 +1,110 @@
+# Tests to cross-check the consistency of GUC parameters with
+# postgresql.conf.sample.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->start;
+
+# Grab the names of all the parameters that can be listed in the
+# configuration sample file.  config_file is an exception, it is not
+# in postgresql.conf.sample but is part of the lists from guc.c.
+my $all_params = $node->safe_psql(
+   'postgres',
+   "SELECT name
+     FROM pg_settings
+   WHERE NOT 'NOT_IN_SAMPLE' = ANY (pg_settings_get_flags(name)) AND
+       name <> 'config_file'
+     ORDER BY 1");
+# Note the lower-case conversion, for consistency.
+my @all_params_array = split("\n", lc($all_params));
+
+# Grab the names of all parameters marked as NOT_IN_SAMPLE.
+my $not_in_sample = $node->safe_psql(
+   'postgres',
+   "SELECT name
+     FROM pg_settings
+   WHERE 'NOT_IN_SAMPLE' = ANY (pg_settings_get_flags(name))
+     ORDER BY 1");
+my @not_in_sample_array = split("\n", lc($not_in_sample));
+
+# TAP tests are executed in the directory of the test, in the source tree,
+# even for VPATH builds, so rely on that to find postgresql.conf.sample.
+my $rootdir     = "../../../..";
+my $sample_file = "$rootdir/src/backend/utils/misc/postgresql.conf.sample";
+
+# List of all the GUCs found in the sample file.
+my @gucs_in_file;
+
+# Read the sample file line-by-line, checking its contents to build a list
+# of everything known as a GUC.
+my $num_tests = 0;
+open(my $contents, '<', $sample_file)
+  || die "Could not open $sample_file: $!";
+while (my $line = <$contents>)
+{
+   # Check if this line matches a GUC parameter:
+   # - Each parameter is preceded by "#", but not "# " in the sample
+   # file.
+   # - Valid configuration options are followed immediately by " = ",
+   # with one space before and after the equal sign.
+   if ($line =~ m/^#?([_[:alpha:]]+) = .*/)
+   {
+       # Lower-case conversion matters for some of the GUCs.
+       my $param_name = lc($1);
+
+       # Ignore some exceptions.
+       next if $param_name eq "include";
+       next if $param_name eq "include_dir";
+       next if $param_name eq "include_if_exists";
+
+       # Update the list of GUCs found in the sample file, for the
+       # follow-up tests.
+       push @gucs_in_file, $param_name;
+   }
+}
+
+close $contents;
+
+# Cross-check that all the GUCs found in the sample file match the ones
+# fetched above.  This maps the arrays to a hash, making the creation of
+# each exclude and intersection list easier.
+my %gucs_in_file_hash  = map { $_ => 1 } @gucs_in_file;
+my %all_params_hash    = map { $_ => 1 } @all_params_array;
+my %not_in_sample_hash = map { $_ => 1 } @not_in_sample_array;
+
+my @missing_from_file = grep(!$gucs_in_file_hash{$_}, @all_params_array);
+is(scalar(@missing_from_file),
+   0, "no parameters missing from postgresql.conf.sample");
+
+my @missing_from_list = grep(!$all_params_hash{$_}, @gucs_in_file);
+is(scalar(@missing_from_list), 0, "no parameters missing from guc.c");
+
+my @sample_intersect = grep($not_in_sample_hash{$_}, @gucs_in_file);
+is(scalar(@sample_intersect),
+   0, "no parameters marked as NOT_IN_SAMPLE in postgresql.conf.sample");
+
+# These would log some information only on errors.
+foreach my $param (@missing_from_file)
+{
+   print("found GUC $param in guc.c, missing from postgresql.conf.sample\n");
+}
+foreach my $param (@missing_from_list)
+{
+   print(
+       "found GUC $param in postgresql.conf.sample, with incorrect info in guc.c\n"
+   );
+}
+foreach my $param (@sample_intersect)
+{
+   print(
+       "found GUC $param in postgresql.conf.sample, marked as NOT_IN_SAMPLE\n"
+   );
+}
+
+done_testing();