From 800d93f314b0f7c10193e48b259f87800cb85d84 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 11 Jan 2021 13:43:19 -0500 Subject: [PATCH] Allow pg_regress.c wrappers to postprocess test result files. Add an optional callback to regression_main() that, if provided, is invoked on each test output file before we try to compare it to the expected-result file. The main and isolation test programs don't need this (yet). In pg_regress_ecpg, add a filter that eliminates target-host details from "could not connect" error reports. This filter doesn't do anything as of this commit, but it will be needed by the next one. In the long run we might want to provide some more general, perhaps pattern-based, filtering mechanism for test output. For now, this will solve the immediate problem. Discussion: https://postgr.es/m/BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com --- src/interfaces/ecpg/test/pg_regress_ecpg.c | 95 ++++++++++++++++++++-- src/test/isolation/isolation_main.c | 5 +- src/test/regress/pg_regress.c | 29 ++++--- src/test/regress/pg_regress.h | 24 ++++-- src/test/regress/pg_regress_main.c | 5 +- 5 files changed, 134 insertions(+), 24 deletions(-) diff --git a/src/interfaces/ecpg/test/pg_regress_ecpg.c b/src/interfaces/ecpg/test/pg_regress_ecpg.c index 31dd507ea0a..8d43fd65bad 100644 --- a/src/interfaces/ecpg/test/pg_regress_ecpg.c +++ b/src/interfaces/ecpg/test/pg_regress_ecpg.c @@ -23,13 +23,16 @@ #include "lib/stringinfo.h" +/* + * Create a filtered copy of sourcefile, removing any path + * appearing in #line directives; for example, replace + * #line x "./../bla/foo.h" with #line x "foo.h". + * This is needed because the path part can vary depending + * on compiler, platform, build options, etc. + */ static void -ecpg_filter(const char *sourcefile, const char *outfile) +ecpg_filter_source(const char *sourcefile, const char *outfile) { - /* - * Create a filtered copy of sourcefile, replacing #line x - * "./../bla/foo.h" with #line x "foo.h" - */ FILE *s, *t; StringInfoData linebuf; @@ -76,6 +79,66 @@ ecpg_filter(const char *sourcefile, const char *outfile) fclose(t); } +/* + * Remove the details of "could not connect to ...: " error messages + * in a test result file, since the target host/pathname and/or port + * can vary. Rewrite the result file in-place. + * + * At some point it might be interesting to unify this with + * ecpg_filter_source, but building a general pattern matcher + * is no fun, nor does it seem desirable to introduce a + * dependency on an external one. + */ +static void +ecpg_filter_stderr(const char *resultfile, const char *tmpfile) +{ + FILE *s, + *t; + StringInfoData linebuf; + + s = fopen(resultfile, "r"); + if (!s) + { + fprintf(stderr, "Could not open file %s for reading\n", resultfile); + exit(2); + } + t = fopen(tmpfile, "w"); + if (!t) + { + fprintf(stderr, "Could not open file %s for writing\n", tmpfile); + exit(2); + } + + initStringInfo(&linebuf); + + while (pg_get_line_buf(s, &linebuf)) + { + char *p1 = strstr(linebuf.data, "could not connect to "); + + if (p1) + { + char *p2 = strstr(p1, ": "); + + if (p2) + { + memmove(p1 + 17, p2, strlen(p2) + 1); + /* we don't bother to fix up linebuf.len */ + } + } + fputs(linebuf.data, t); + } + + pfree(linebuf.data); + fclose(s); + fclose(t); + if (rename(tmpfile, resultfile) != 0) + { + fprintf(stderr, "Could not overwrite file %s with %s\n", + resultfile, tmpfile); + exit(2); + } +} + /* * start an ecpg test process for specified file (including redirection), * and return process ID @@ -139,7 +202,7 @@ ecpg_start_test(const char *testname, add_stringlist_item(expectfiles, expectfile_source); add_stringlist_item(tags, "source"); - ecpg_filter(insource, outfile_source); + ecpg_filter_source(insource, outfile_source); snprintf(cmd, sizeof(cmd), "\"%s\" >\"%s\" 2>\"%s\"", @@ -167,6 +230,21 @@ ecpg_start_test(const char *testname, return pid; } +static void +ecpg_postprocess_result(const char *filename) +{ + int nlen = strlen(filename); + + /* Only stderr files require filtering, at the moment */ + if (nlen > 7 && strcmp(filename + nlen - 7, ".stderr") == 0) + { + char *tmpfile = psprintf("%s.tmp", filename); + + ecpg_filter_stderr(filename, tmpfile); + pfree(tmpfile); + } +} + static void ecpg_init(int argc, char *argv[]) { @@ -176,5 +254,8 @@ ecpg_init(int argc, char *argv[]) int main(int argc, char *argv[]) { - return regression_main(argc, argv, ecpg_init, ecpg_start_test); + return regression_main(argc, argv, + ecpg_init, + ecpg_start_test, + ecpg_postprocess_result); } diff --git a/src/test/isolation/isolation_main.c b/src/test/isolation/isolation_main.c index 50893aa3abb..eff9c4f2dfe 100644 --- a/src/test/isolation/isolation_main.c +++ b/src/test/isolation/isolation_main.c @@ -145,5 +145,8 @@ isolation_init(int argc, char **argv) int main(int argc, char *argv[]) { - return regression_main(argc, argv, isolation_init, isolation_start_test); + return regression_main(argc, argv, + isolation_init, + isolation_start_test, + NULL /* no postfunc needed */ ); } diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 5cfb4c4a491..b284cc88c40 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -731,7 +731,7 @@ static void initialize_environment(void) { /* - * Set default application_name. (The test_function may choose to + * Set default application_name. (The test_start_function may choose to * override this, but if it doesn't, we have something useful in place.) */ setenv("PGAPPNAME", "pg_regress", 1); @@ -1616,7 +1616,8 @@ log_child_failure(int exitstatus) * Run all the tests specified in one schedule file */ static void -run_schedule(const char *schedule, test_function tfunc) +run_schedule(const char *schedule, test_start_function startfunc, + postprocess_result_function postfunc) { #define MAX_PARALLEL_TESTS 100 char *tests[MAX_PARALLEL_TESTS]; @@ -1730,7 +1731,7 @@ run_schedule(const char *schedule, test_function tfunc) if (num_tests == 1) { status(_("test %-28s ... "), tests[0]); - pids[0] = (tfunc) (tests[0], &resultfiles[0], &expectfiles[0], &tags[0]); + pids[0] = (startfunc) (tests[0], &resultfiles[0], &expectfiles[0], &tags[0]); INSTR_TIME_SET_CURRENT(starttimes[0]); wait_for_tests(pids, statuses, stoptimes, NULL, 1); /* status line is finished below */ @@ -1756,7 +1757,7 @@ run_schedule(const char *schedule, test_function tfunc) tests + oldest, i - oldest); oldest = i; } - pids[i] = (tfunc) (tests[i], &resultfiles[i], &expectfiles[i], &tags[i]); + pids[i] = (startfunc) (tests[i], &resultfiles[i], &expectfiles[i], &tags[i]); INSTR_TIME_SET_CURRENT(starttimes[i]); } wait_for_tests(pids + oldest, statuses + oldest, @@ -1769,7 +1770,7 @@ run_schedule(const char *schedule, test_function tfunc) status(_("parallel group (%d tests): "), num_tests); for (i = 0; i < num_tests; i++) { - pids[i] = (tfunc) (tests[i], &resultfiles[i], &expectfiles[i], &tags[i]); + pids[i] = (startfunc) (tests[i], &resultfiles[i], &expectfiles[i], &tags[i]); INSTR_TIME_SET_CURRENT(starttimes[i]); } wait_for_tests(pids, statuses, stoptimes, tests, num_tests); @@ -1801,6 +1802,8 @@ run_schedule(const char *schedule, test_function tfunc) { bool newdiff; + if (postfunc) + (*postfunc) (rl->str); newdiff = results_differ(tests[i], rl->str, el->str); if (newdiff && tl) { @@ -1867,7 +1870,8 @@ run_schedule(const char *schedule, test_function tfunc) * Run a single test */ static void -run_single_test(const char *test, test_function tfunc) +run_single_test(const char *test, test_start_function startfunc, + postprocess_result_function postfunc) { PID_TYPE pid; instr_time starttime; @@ -1882,7 +1886,7 @@ run_single_test(const char *test, test_function tfunc) bool differ = false; status(_("test %-28s ... "), test); - pid = (tfunc) (test, &resultfiles, &expectfiles, &tags); + pid = (startfunc) (test, &resultfiles, &expectfiles, &tags); INSTR_TIME_SET_CURRENT(starttime); wait_for_tests(&pid, &exit_status, &stoptime, NULL, 1); @@ -1900,6 +1904,8 @@ run_single_test(const char *test, test_function tfunc) { bool newdiff; + if (postfunc) + (*postfunc) (rl->str); newdiff = results_differ(test, rl->str, el->str); if (newdiff && tl) { @@ -2083,7 +2089,10 @@ help(void) } int -regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc) +regression_main(int argc, char *argv[], + init_function ifunc, + test_start_function startfunc, + postprocess_result_function postfunc) { static struct option long_options[] = { {"help", no_argument, NULL, 'h'}, @@ -2554,12 +2563,12 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc for (sl = schedulelist; sl != NULL; sl = sl->next) { - run_schedule(sl->str, tfunc); + run_schedule(sl->str, startfunc, postfunc); } for (sl = extra_tests; sl != NULL; sl = sl->next) { - run_single_test(sl->str, tfunc); + run_single_test(sl->str, startfunc, postfunc); } /* diff --git a/src/test/regress/pg_regress.h b/src/test/regress/pg_regress.h index d04f7721aa8..c6d015c8402 100644 --- a/src/test/regress/pg_regress.h +++ b/src/test/regress/pg_regress.h @@ -27,12 +27,23 @@ typedef struct _stringlist struct _stringlist *next; } _stringlist; -typedef PID_TYPE(*test_function) (const char *, - _stringlist **, - _stringlist **, - _stringlist **); +/* + * Callback function signatures for test programs that use regression_main() + */ + +/* Initialize at program start */ typedef void (*init_function) (int argc, char **argv); +/* Launch one test case */ +typedef PID_TYPE(*test_start_function) (const char *testname, + _stringlist **resultfiles, + _stringlist **expectfiles, + _stringlist **tags); + +/* Postprocess one result file (optional) */ +typedef void (*postprocess_result_function) (const char *filename); + + extern char *bindir; extern char *libdir; extern char *datadir; @@ -48,7 +59,10 @@ extern const char *basic_diff_opts; extern const char *pretty_diff_opts; int regression_main(int argc, char *argv[], - init_function ifunc, test_function tfunc); + init_function ifunc, + test_start_function startfunc, + postprocess_result_function postfunc); + void add_stringlist_item(_stringlist **listhead, const char *str); PID_TYPE spawn_process(const char *cmdline); void replace_string(struct StringInfoData *string, diff --git a/src/test/regress/pg_regress_main.c b/src/test/regress/pg_regress_main.c index a218bae247c..8dc4941c240 100644 --- a/src/test/regress/pg_regress_main.c +++ b/src/test/regress/pg_regress_main.c @@ -119,5 +119,8 @@ psql_init(int argc, char **argv) int main(int argc, char *argv[]) { - return regression_main(argc, argv, psql_init, psql_start_test); + return regression_main(argc, argv, + psql_init, + psql_start_test, + NULL /* no postfunc needed */ ); }