Exclude tests that inject faults when faultinjector is disabled

If faultinjector is not enabled during configuration, this patch
identifies tests that use faultinjector and excludes them from running.
A new option "--exclude-file" is added to pg_regress.  It specifies a
file having names of tests, one per line, to be excluded from running.

The global makefile is changed so that the exclude file creation and
adding the --exclude-file option to pg_regress (and
pg_isolation*regress) command line is taken care of automatically.
However, one must perform these actions manually if pg_regress needs to
be directly invoked from command line (instead of a make target).
Running tests with faultinjector disabled is not a common developer
workflow, so this should be acceptable.

The advantage of changing global makefile is that all test frameworks
that are based on pg_regress, such as isolation and also contrib modules
need no change to their makefiles.  For example, if one creates a
isolation test with faultinjector, simply add it to the right schedule,
it will automatically be excluded from running when faultinjector is not
enabled.

src/Makefile.global.in

@@ -494,6 +494,24 @@ prove_installcheck = @echo "TAP tests not enabled"
 prove_check = $(prove_installcheck)
 endif
 
+ifeq ($(enable_debug_extensions),yes)
+define faultinjector_prep
+$(top_builddir)/src/test/regress/scan_flaky_fault_injectors.sh &&
+endef
+FAULTINJECTOR_OPTS := --load-extension=gp_inject_fault
+else
+define faultinjector_prep
+# TODO: A similar grep command is used in
+# scan_flaky_fault_injectors.sh script.  Find a way to avoid the
+# duplication.
+grep -sERIli '(select|perform)\s.*gp_inject_fault' sql input specs \
+	| sed 's,^[^/]*/\(.*\)\.[^.]*$$,\1,' \
+	| sort -u \
+	> tests_using_faultinjector.txt &&
+endef
+FAULTINJECTOR_OPTS := --exclude-file=tests_using_faultinjector.txt
+endif
+
 # Installation.
 
 install_bin = @install_bin@
@@ -682,41 +700,47 @@ TEMP_CONF += --temp-config=$(TEMP_CONFIG)
 endif
 
 pg_regress_locale_flags = $(if $(ENCODING),--encoding=$(ENCODING)) $(NOLOCALE)
-pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ tmp_check_iso/ log/ output_iso/
+pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ tmp_check_iso/ log/ output_iso/ tests_using_faultinjector.txt
 
 pg_regress_check = \
+    $(faultinjector_prep) \
     $(with_temp_install) \
     $(top_builddir)/src/test/regress/pg_regress \
     --temp-instance=./tmp_check \
     --inputdir=$(srcdir) \
     --bindir= \
     $(TEMP_CONF) \
-    $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
+    $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) $(FAULTINJECTOR_OPTS)
 pg_regress_installcheck = \
+    $(faultinjector_prep) \
     $(top_builddir)/src/test/regress/pg_regress \
     --inputdir=$(srcdir) \
     --bindir='$(bindir)' \
-    $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
+    $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) $(FAULTINJECTOR_OPTS)
 
 pg_isolation_regress_check = \
+    $(faultinjector_prep) \
     $(with_temp_install) \
     $(top_builddir)/src/test/isolation/pg_isolation_regress \
     --temp-instance=./tmp_check_iso \
     --inputdir=$(srcdir) --outputdir=output_iso \
     --bindir= \
     $(TEMP_CONF) \
-    $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
+    $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) $(FAULTINJECTOR_OPTS)
 pg_isolation_regress_installcheck = \
+    $(faultinjector_prep) \
     $(top_builddir)/src/test/isolation/pg_isolation_regress \
     --inputdir=$(srcdir) --outputdir=output_iso \
     --bindir='$(bindir)' \
-    $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
+    $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) $(FAULTINJECTOR_OPTS)
 
 pg_isolation2_regress_installcheck = \
+    $(faultinjector_prep) \
     $(top_builddir)/src/test/isolation2/pg_isolation2_regress \
     --inputdir=$(srcdir) \
-    --bindir= \
-    $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
+    --bindir='$(bindir)' \
+    $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) $(FAULTINJECTOR_OPTS)
+
 ##########################################################################
 #
 # Customization

src/test/fsync/Makefile

@@ -2,7 +2,6 @@ MODULES=fsync_helper
 PG_CONFIG=pg_config
 
 REGRESS = setup bgwriter_checkpoint
-REGRESS_OPTS = --load-extension=gp_inject_fault
 
 subdir = src/test/fsync
 top_builddir = ../../..

src/test/heap_checksum/Makefile

@@ -2,7 +2,7 @@ MODULES=heap_checksum_helper
 PG_CONFIG=pg_config
 
 REGRESS = setup heap_checksum_corruption
-REGRESS_OPTS = --init-file=../regress/init_file --load-extension=gp_inject_fault
+REGRESS_OPTS = --init-file=../regress/init_file
 
 subdir = src/test/heap_checksum
 top_builddir = ../../..

src/test/isolation2/Makefile

@@ -17,7 +17,7 @@ endif
 override CPPFLAGS := -I$(srcdir) -I$(libpq_srcdir) -I$(srcdir)/../regress $(CPPFLAGS)
 override LDLIBS := $(libpq_pgport) $(LDLIBS)
 
-all: pg_isolation2_regress$(X) all-lib data extended_protocol_test scan_flaky_fault_injectors
+all: pg_isolation2_regress$(X) all-lib data extended_protocol_test
 
 extended_protocol_test: extended_protocol_test.c
 	$(CC) $(CPPFLAGS) -I$(top_builddir)/src/interfaces/libpq -L$(GPHOME)/lib -L$(top_builddir)/src/interfaces/libpq  -o $@ $< -lpq
@@ -44,10 +44,6 @@ explain.pm:
 data:
 	rm -f $@ && $(LN_S) $(top_builddir)/src/test/regress/data
 
-.PHONY: scan_flaky_fault_injectors
-scan_flaky_fault_injectors:
-	$(top_builddir)/src/test/regress/scan_flaky_fault_injectors.sh
-
 pg_isolation2_regress$(X): isolation2_main.o pg_regress.o submake-libpq submake-libpgport
 	$(CC) $(CFLAGS) $(filter %.o,$^) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
 

src/test/regress/GNUmakefile

@@ -40,7 +40,7 @@ EXTRADEFS = '-DHOST_TUPLE="$(host_tuple)"' \
 
 # Build regression test driver
 
-all: pg_regress$(X) scan_flaky_fault_injectors
+all: pg_regress$(X)
 
 pg_regress$(X): pg_regress.o pg_regress_main.o $(WIN32RES) | submake-libpgport
 	$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
@@ -51,10 +51,6 @@ pg_regress.o: override CPPFLAGS += -I$(top_builddir)/src/port $(EXTRADEFS)
 regress_gp.o: override CPPFLAGS += -I$(libpq_srcdir)
 regress_gp.o: override LDFLAGS += -L$(top_builddir)/src/interfaces/libpq
 
-.PHONY: scan_flaky_fault_injectors
-scan_flaky_fault_injectors:
-	$(top_builddir)/src/test/regress/scan_flaky_fault_injectors.sh
-
 twophase_pqexecparams: twophase_pqexecparams.c
 	$(CC) $(CPPFLAGS) -I$(top_builddir)/src/interfaces/libpq -L$(GPHOME)/lib -L$(top_builddir)/src/interfaces/libpq  -o $@ $< -lpq
 
@@ -72,7 +68,7 @@ install: all installdirs
 	$(INSTALL_PROGRAM) explain.pl '$(DESTDIR)$(pgxsdir)/$(subdir)/explain.pl'
 	$(INSTALL_PROGRAM) explain.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/explain.pm'
 	$(INSTALL_PROGRAM) GPTest.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/GPTest.pm'
-
+	$(INSTALL_PROGRAM) scan_flaky_fault_injectors.sh '$(DESTDIR)$(pgxsdir)/$(subdir)/scan_flaky_fault_injectors.sh'
 installdirs:
 	$(MKDIR_P) '$(DESTDIR)$(pgxsdir)/$(subdir)'
 
@@ -180,7 +176,7 @@ query_info_hook_test:
 ## Run tests
 ##
 
-REGRESS_OPTS = --dlpath=. --max-concurrent-tests=20 --init-file=$(srcdir)/init_file $(EXTRA_REGRESS_OPTS) --load-extension=gp_inject_fault
+REGRESS_OPTS = --dlpath=. --max-concurrent-tests=20 --init-file=$(srcdir)/init_file $(EXTRA_REGRESS_OPTS)
 
 check: all tablespace-setup
 	$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)

src/test/regress/pg_regress.c

@@ -87,6 +87,7 @@ bool		debug = false;
 char	   *inputdir = ".";
 char	   *outputdir = ".";
 char	   *tablespacedir = ".";
+char	   *exclude_tests_file = "";
 char	   *prehook = "";
 char	   *bindir = PGBINDIR;
 char	   *launcher = NULL;
@@ -232,6 +233,40 @@ split_to_stringlist(const char *s, const char *delim, _stringlist **listhead)
 	free(sc);
 }
 
+static void
+load_exclude_tests_file(_stringlist **listhead, const char *exclude_tests_file)
+{
+	char buf[1024];
+	FILE *excludefile;
+	int i;
+	excludefile = fopen(exclude_tests_file, "r");
+	if (!excludefile)
+	{
+		fprintf(stderr, _("\ncould not open file %s: %s\n"),
+				exclude_tests_file, strerror(errno));
+		_exit(2);
+	}
+	while (fgets(buf, sizeof(buf), excludefile))
+	{
+		i = strlen(buf);
+		if (buf[i-1] == '\n')
+			buf[i-1] = '\0';
+		add_stringlist_item(&exclude_tests, buf);
+	}
+	if (ferror(excludefile))
+	{
+		fprintf(stderr, _("\ncould not read file %s: %s\n"),
+				exclude_tests_file, strerror(errno));
+		_exit(2);
+	}
+	if (fclose(excludefile))
+	{
+		fprintf(stderr, _("\ncould not close file %s: %s\n"),
+				exclude_tests_file, strerror(errno));
+		_exit(2);
+	}
+}
+
 /*
  * Print a progress banner on stdout.
  */
@@ -2081,6 +2116,7 @@ run_schedule(const char *schedule, test_function tfunc)
 		char	   *test = NULL;
 		char	   *c;
 		int			num_tests;
+		int			excluded_tests;
 		bool		inword;
 		int			i;
 		struct timeval start_time;
@@ -2118,6 +2154,7 @@ run_schedule(const char *schedule, test_function tfunc)
 		}
 
 		num_tests = 0;
+		excluded_tests = 0;
 		inword = false;
 		for (c = test;; c++)
 		{
@@ -2146,7 +2183,10 @@ run_schedule(const char *schedule, test_function tfunc)
 					 * array, after all.
 					 */
 					if (should_exclude_test(tests[num_tests - 1]))
+					{
+						excluded_tests++;
 						num_tests--;
+					}
 				}
 				if (*c == '\0')
 					break;		/* loop exit is here */
@@ -2163,19 +2203,20 @@ run_schedule(const char *schedule, test_function tfunc)
 		if (num_tests - 1 >= 0 && should_exclude_test(tests[num_tests - 1]))
 		{
 			num_tests--;
-
-			/* All tests in this line are to be excluded, so go to the next line */
-			if (num_tests == 0)
-				continue;
+			excluded_tests++;
 		}
 
-		if (num_tests == 0)
+		if (num_tests == 0 && excluded_tests == 0)
 		{
 			fprintf(stderr, _("syntax error in schedule file \"%s\" line %d: %s\n"),
 					schedule, line_num, scbuf);
 			exit(2);
 		}
 
+		/* All tests in this line are to be excluded, so go to the next line */
+		if (num_tests == 0)
+			continue;
+
 		if (!cluster_healthy())
 			break;
 
@@ -2345,6 +2386,9 @@ run_single_test(const char *test, test_function tfunc)
 	if (!cluster_healthy())
 		return;
 
+	if (should_exclude_test((char *) test))
+		return;
+
 	status(_("test %-28s ... "), test);
 	pid = (tfunc) (test, &resultfiles, &expectfiles, &tags);
 	INSTR_TIME_SET_CURRENT(starttime);
@@ -2520,6 +2564,7 @@ create_database(const char *dbname)
 		header(_("installing %s"), sl->str);
 		psql_command(dbname, "CREATE EXTENSION IF NOT EXISTS \"%s\"", sl->str);
 	}
+
 }
 
 static void
@@ -2574,7 +2619,7 @@ should_exclude_test(char *test)
 	_stringlist *sl;
 	for (sl = exclude_tests; sl != NULL; sl = sl->next)
 	{
-		if (strcmp(test, sl->str) == 0)
+		if (strncmp(test, sl->str, strlen(sl->str)) == 0)
 			return true;
 	}
 
@@ -2681,6 +2726,7 @@ help(void)
 	/* Please put GPDB specific options here, at the end */
 	printf(_("      --prehook=NAME            pre-hook name (default \"\")\n"));
 	printf(_("      --exclude-tests=TEST      command or space delimited tests to exclude from running\n"));
+	printf(_("      --exclude-file=FILE       file with tests to exclude from running, one test name per line\n"));
     printf(_("      --init-file=GPD_INIT_FILE  init file to be used for gpdiff (could be used multiple times)\n"));
 	printf(_("      --ignore-plans            ignore any explain plan diffs\n"));
 	printf(_("      --print-failure-diffs     Print the diff file to standard out after a failure\n"));
@@ -2738,6 +2784,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
 		{"prehook", required_argument, NULL, 83},
 		{"print-failure-diffs", no_argument, NULL, 84},
 		{"tablespace-dir", required_argument, NULL, 85},
+		{"exclude-file", required_argument, NULL, 87}, /* 86 conflicts with 'V' */
 		{NULL, 0, NULL, 0}
 	};
 
@@ -2877,6 +2924,10 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
 			case 85:
 				tablespacedir = strdup(optarg);
 				break;
+			case 87:
+				exclude_tests_file = strdup(optarg);
+				load_exclude_tests_file(&exclude_tests, exclude_tests_file);
+				break;
 			default:
 				/* getopt_long already emitted a complaint */
 				fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"),
@@ -3191,6 +3242,12 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
 		}
 	}
 
+#ifdef FAULT_INJECTOR
+	header(_("faultinjector enabled"));
+#else
+	header(_("faultinjector not enabled"));
+#endif
+
 	/*
 	 * Create the test database(s) and role(s)
 	 */

src/test/walrep/Makefile

@@ -8,7 +8,6 @@ include $(top_builddir)/src/Makefile.global
 
 REGRESS = setup
 REGRESS += replication_views_mirrored missing_xlog walreceiver generate_ao_xlog generate_aoco_xlog
-REGRESS_OPTS = --load-extension=gp_inject_fault
 
 NO_PGXS = 1
 include $(top_srcdir)/src/makefiles/pgxs.mk