On Tue, 3 Jan 2023 at 16:01, vignesh C <vignes...@gmail.com> wrote: > > On Tue, 29 Nov 2022 at 00:57, Daniel Gustafsson <dan...@yesql.se> wrote: > > > > > On 28 Nov 2022, at 20:02, Nikolay Shaplov <dh...@nataraj.su> wrote: > > > > > From my reviewer's point of view patch is ready for commit. > > > > > > Thank you for your patience :-) > > > > Thanks for review. > > > > The attached tweaks a few comments and attempts to address the compiler > > warning > > error in the CFBot CI. Not sure I entirely agree with the compiler there > > but > > here is an attempt to work around it at least (by always copying the va_list > > for the logfile). It also adds a missing va_end for the logfile va_list. > > > > I hope this is close to a final version of this patch (commitmessage needs a > > bit of work though). > > > > > PS Should I change commitfest status? > > > > Sure, go ahead. > > > > The patch does not apply on top of HEAD as in [1], please post a rebased > patch: > === Applying patches on top of PostgreSQL commit ID > 92957ed98c5c565362ce665266132a7f08f6b0c0 === > === applying patch > ./v14-0001-Change-pg_regress-output-format-to-be-TAP-compli.patch > patching file meson.build > Hunk #1 FAILED at 2968. > 1 out of 1 hunk FAILED -- saving rejects to file meson.build.rej
Attached a rebased patch on top of HEAD to try and see if we can close this patch in this commitfest. Regards, Vignesh
From 005bcf13037577a8682e3a6365b1b52bb2e48492 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson <dgustafs...@postgresql.org> Date: Fri, 6 Jan 2023 11:09:10 +0530 Subject: [PATCH v15] Change pg_regress output format to be TAP compliant This converts pg_regress output format to emit TAP complient output while keeping it as human readable as possible for use without TAP test harnesses. As verbose harness related information isn't really supported by TAP this also reduces the verbosity of pg_regress runs which makes scrolling through log output in buildfarm/CI runs a bit easier as well. TAP format testing is also enabled in Meson as of this. Discussion: https://postgr.es/m/bd4b107d-7e53-4794-acba-275beb432...@yesql.se --- meson.build | 1 + src/test/regress/pg_regress.c | 575 ++++++++++++++++++---------------- 2 files changed, 314 insertions(+), 262 deletions(-) diff --git a/meson.build b/meson.build index 45fb9dd616..7195937d17 100644 --- a/meson.build +++ b/meson.build @@ -3024,6 +3024,7 @@ foreach test_dir : tests env.prepend('PATH', temp_install_bindir, test_dir['bd']) test_kwargs = { + 'protocol': 'tap', 'priority': 10, 'timeout': 1000, 'depends': test_deps + t.get('deps', []), diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 40e6c231a3..a05f423c75 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -68,6 +68,26 @@ const char *basic_diff_opts = "-w"; const char *pretty_diff_opts = "-w -U3"; #endif +/* For parallel tests, testnames are indented when printed for grouping */ +#define PARALLEL_INDENT 7 +/* + * The width of the testname field when printing to ensure vertical alignment + * of test runtimes. This number is somewhat arbitrarily chosen to match the + * older pre-TAP output format. + */ +#define TESTNAME_WIDTH 36 + +typedef enum TAPtype +{ + DIAG = 0, + BAIL, + NOTE, + DETAIL, + TEST_STATUS, + PLAN, + NONE +} TAPtype; + /* options settable from command line */ _stringlist *dblist = NULL; bool debug = false; @@ -103,6 +123,7 @@ static const char *sockdir; static const char *temp_sockdir; static char sockself[MAXPGPATH]; static char socklock[MAXPGPATH]; +static StringInfo failed_tests = NULL; static _resultmap *resultmap = NULL; @@ -116,12 +137,29 @@ static int fail_ignore_count = 0; static bool directory_exists(const char *dir); static void make_directory(const char *dir); -static void header(const char *fmt,...) pg_attribute_printf(1, 2); -static void status(const char *fmt,...) pg_attribute_printf(1, 2); +static void test_status_print(bool ok, const char *testname, double runtime, bool parallel, bool ignore); +static void test_status_ok(const char *testname, double runtime, bool parallel); +static void test_status_failed(const char *testname, bool ignore, double runtime, bool parallel); +static void bail_out(bool noatexit, const char *fmt,...) pg_attribute_printf(2, 3); +static void emit_tap_output(TAPtype type, const char *fmt,...) pg_attribute_printf(2, 3); +static void emit_tap_output_v(TAPtype type, const char *fmt, va_list argp) pg_attribute_printf(2, 0); + static StringInfo psql_start_command(void); static void psql_add_command(StringInfo buf, const char *query,...) pg_attribute_printf(2, 3); static void psql_end_command(StringInfo buf, const char *database); +/* + * Convenience macros for printing TAP output with a more shorthand syntax + * aimed at making the code more readable. + */ +#define plan(x) emit_tap_output(PLAN, "1..%i\n", (x)) +#define note(...) emit_tap_output(NOTE, __VA_ARGS__) +#define note_detail(...) emit_tap_output(DETAIL, __VA_ARGS__) +#define diag(...) emit_tap_output(DIAG, __VA_ARGS__) +#define note_end() emit_tap_output(NONE, "\n"); +#define bail_noatexit(...) bail_out(true, __VA_ARGS__) +#define bail(...) bail_out(false, __VA_ARGS__) + /* * allow core files if possible. */ @@ -134,9 +172,7 @@ unlimit_core_size(void) getrlimit(RLIMIT_CORE, &lim); if (lim.rlim_max == 0) { - fprintf(stderr, - _("%s: could not set core size: disallowed by hard limit\n"), - progname); + diag(_("could not set core size: disallowed by hard limit\n")); return; } else if (lim.rlim_max == RLIM_INFINITY || lim.rlim_cur < lim.rlim_max) @@ -202,53 +238,153 @@ split_to_stringlist(const char *s, const char *delim, _stringlist **listhead) } /* - * Print a progress banner on stdout. + * Bailing out is for unrecoverable errors which prevents further testing to + * occur and after which the test run should be aborted. By passing noatexit + * as true the process will terminate with _exit(2) and skipping registered + * exit handlers, thus avoid any risk of bottomless recursion calls to exit. */ static void -header(const char *fmt,...) +bail_out(bool noatexit, const char *fmt,...) { - char tmp[64]; va_list ap; va_start(ap, fmt); - vsnprintf(tmp, sizeof(tmp), fmt, ap); + emit_tap_output_v(BAIL, fmt, ap); va_end(ap); - fprintf(stdout, "============== %-38s ==============\n", tmp); - fflush(stdout); + if (noatexit) + _exit(2); + + exit(2); } -/* - * Print "doing something ..." --- supplied text should not end with newline - */ static void -status(const char *fmt,...) +test_status_print(bool ok, const char *testname, double runtime, bool parallel, bool ignore) { - va_list ap; + int testnumber; + int padding; - va_start(ap, fmt); - vfprintf(stdout, fmt, ap); - fflush(stdout); - va_end(ap); + testnumber = fail_count + fail_ignore_count + success_count; + padding = TESTNAME_WIDTH; - if (logfile) + /* + * Calculate the width for the testname field required to align runtimes + * vertically. + */ + if (parallel) + padding -= PARALLEL_INDENT; + + /* + * There is no NLS translation here as "(not) ok" and "SKIP" are protocol. + * Testnumbers are padded to 5 characters to ensure that testnames align + * vertically (assuming at most 9999 tests). Testnames are indented 8 + * spaces in case they run as part of a parallel group. The position for + * the runtime is offset based on that indentation. + */ + emit_tap_output(TEST_STATUS, "%sok %-5i%*s %*s%-*s %8.0f ms%s\n", + (ok ? "" : "not "), + testnumber, + /* If ok, indent with four spaces matching "not " */ + (ok ? (int) strlen("not ") : 0), "", + /* If parallel, indent to indicate grouping */ + (parallel ? PARALLEL_INDENT : 0), "", + /* Testnames are padded to align runtimes */ + padding, testname, + runtime, + /* Add an (igngored) comment on the SKIP line to clarify */ + (ignore ? " # SKIP (ignored)" : "")); +} + +static void +test_status_ok(const char *testname, double runtime, bool parallel) +{ + success_count++; + + test_status_print(true, testname, runtime, parallel, false); +} + +static void +test_status_failed(const char *testname, bool ignore, double runtime, bool parallel) +{ + /* + * Save failed tests in a buffer such that we can print a summary at the + * end with diag() to ensure it's shown even under test harnesses. + */ + if (!failed_tests) + failed_tests = makeStringInfo(); + else + appendStringInfoChar(failed_tests, ','); + + appendStringInfo(failed_tests, " %s", testname); + + if (ignore) { - va_start(ap, fmt); - vfprintf(logfile, fmt, ap); - va_end(ap); + fail_ignore_count++; + appendStringInfoString(failed_tests, " (ignored)"); } + else + fail_count++; + + test_status_print(false, testname, runtime, parallel, ignore); +} + + +static void +emit_tap_output(TAPtype type, const char *fmt,...) +{ + va_list argp; + + va_start(argp, fmt); + emit_tap_output_v(type, fmt, argp); + va_end(argp); } -/* - * Done "doing something ..." - */ static void -status_end(void) +emit_tap_output_v(TAPtype type, const char *fmt, va_list argp) { - fprintf(stdout, "\n"); - fflush(stdout); + va_list argp_logfile; + FILE *fp; + + /* Make a copy of the va args for printing to the logfile */ + va_copy(argp_logfile, argp); + + /* + * Diagnostic output will be hidden by prove unless printed to stderr. The + * Bail message is also printed to stderr to aid debugging under a harness + * which might otherwise not emit such an important message. + */ + if (type == DIAG || type == BAIL) + fp = stderr; + else + fp = stdout; + + /* + * Non-protocol output such as diagnostics or notes must be prefixed by a + * '#' character. We print the Bail message like this too. + */ + if (type == NOTE || type == DIAG || type == BAIL) + { + fprintf(fp, "# "); + if (logfile) + fprintf(logfile, "# "); + } + vfprintf(fp, fmt, argp); if (logfile) - fprintf(logfile, "\n"); + vfprintf(logfile, fmt, argp_logfile); + + /* + * If this was a Bail message, the bail protocol message must go to stdout + * separately. + */ + if (type == BAIL) + { + fprintf(stdout, "Bail Out!\n"); + if (logfile) + fprintf(logfile, "Bail Out!\n"); + } + + va_end(argp_logfile); + fflush(NULL); } /* @@ -272,9 +408,9 @@ stop_postmaster(void) r = system(buf); if (r != 0) { - fprintf(stderr, _("\n%s: could not stop postmaster: exit code was %d\n"), - progname, r); - _exit(2); /* not exit(), that could be recursive */ + /* Not using the normal bail() as we want _exit */ + bail_noatexit(_("could not stop postmaster: exit code was %d\n"), + r); } postmaster_running = false; @@ -332,9 +468,8 @@ make_temp_sockdir(void) temp_sockdir = mkdtemp(template); if (temp_sockdir == NULL) { - fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), - progname, template, strerror(errno)); - exit(2); + bail(_("could not create directory \"%s\": %s\n"), + template, strerror(errno)); } /* Stage file names for remove_temp(). Unsafe in a signal handler. */ @@ -456,9 +591,8 @@ load_resultmap(void) /* OK if it doesn't exist, else complain */ if (errno == ENOENT) return; - fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"), - progname, buf, strerror(errno)); - exit(2); + bail(_("could not open file \"%s\" for reading: %s\n"), + buf, strerror(errno)); } while (fgets(buf, sizeof(buf), f)) @@ -477,26 +611,20 @@ load_resultmap(void) file_type = strchr(buf, ':'); if (!file_type) { - fprintf(stderr, _("incorrectly formatted resultmap entry: %s\n"), - buf); - exit(2); + bail(_("incorrectly formatted resultmap entry: %s\n"), buf); } *file_type++ = '\0'; platform = strchr(file_type, ':'); if (!platform) { - fprintf(stderr, _("incorrectly formatted resultmap entry: %s\n"), - buf); - exit(2); + bail(_("incorrectly formatted resultmap entry: %s\n"), buf); } *platform++ = '\0'; expected = strchr(platform, '='); if (!expected) { - fprintf(stderr, _("incorrectly formatted resultmap entry: %s\n"), - buf); - exit(2); + bail(_("incorrectly formatted resultmap entry: %s\n"), buf); } *expected++ = '\0'; @@ -742,13 +870,13 @@ initialize_environment(void) } if (pghost && pgport) - printf(_("(using postmaster on %s, port %s)\n"), pghost, pgport); + note(_("(using postmaster on %s, port %s)\n"), pghost, pgport); if (pghost && !pgport) - printf(_("(using postmaster on %s, default port)\n"), pghost); + note(_("(using postmaster on %s, default port)\n"), pghost); if (!pghost && pgport) - printf(_("(using postmaster on Unix socket, port %s)\n"), pgport); + note(_("(using postmaster on Unix socket, port %s)\n"), pgport); if (!pghost && !pgport) - printf(_("(using postmaster on Unix socket, default port)\n")); + note(_("(using postmaster on Unix socket, default port)\n")); } load_resultmap(); @@ -797,35 +925,27 @@ current_windows_user(const char **acct, const char **dom) if (!OpenProcessToken(GetCurrentProcess(), TOKEN_READ, &token)) { - fprintf(stderr, - _("%s: could not open process token: error code %lu\n"), - progname, GetLastError()); - exit(2); + bail(_("could not open process token: error code %lu"), + GetLastError()); } if (!GetTokenInformation(token, TokenUser, NULL, 0, &retlen) && GetLastError() != 122) { - fprintf(stderr, - _("%s: could not get token information buffer size: error code %lu\n"), - progname, GetLastError()); - exit(2); + bail(_("could not get token information buffer size: error code %lu"), + GetLastError()); } tokenuser = pg_malloc(retlen); if (!GetTokenInformation(token, TokenUser, tokenuser, retlen, &retlen)) { - fprintf(stderr, - _("%s: could not get token information: error code %lu\n"), - progname, GetLastError()); - exit(2); + bail(_("could not get token information: error code %lu\n"), + GetLastError()); } if (!LookupAccountSid(NULL, tokenuser->User.Sid, accountname, &accountnamesize, domainname, &domainnamesize, &accountnameuse)) { - fprintf(stderr, - _("%s: could not look up account SID: error code %lu\n"), - progname, GetLastError()); - exit(2); + bail(_("could not look up account SID: error code %lu\n"), + GetLastError()); } free(tokenuser); @@ -974,7 +1094,7 @@ psql_start_command(void) StringInfo buf = makeStringInfo(); appendStringInfo(buf, - "\"%s%spsql\" -X", + "\"%s%spsql\" -X -q", bindir ? bindir : "", bindir ? "/" : ""); return buf; @@ -1030,8 +1150,7 @@ psql_end_command(StringInfo buf, const char *database) if (system(buf->data) != 0) { /* psql probably already reported the error */ - fprintf(stderr, _("command failed: %s\n"), buf->data); - exit(2); + bail(_("command failed: %s\n"), buf->data); } /* Clean up */ @@ -1072,9 +1191,7 @@ spawn_process(const char *cmdline) pid = fork(); if (pid == -1) { - fprintf(stderr, _("%s: could not fork: %s\n"), - progname, strerror(errno)); - exit(2); + bail(_("could not fork: %s\n"), strerror(errno)); } if (pid == 0) { @@ -1089,9 +1206,10 @@ spawn_process(const char *cmdline) cmdline2 = psprintf("exec %s", cmdline); execl(shellprog, shellprog, "-c", cmdline2, (char *) NULL); - fprintf(stderr, _("%s: could not exec \"%s\": %s\n"), - progname, shellprog, strerror(errno)); - _exit(1); /* not exit() here... */ + /* Not using the normal bail() here as we want _exit */ + bail_noatexit(_("could not exec \"%s\": %s\n"), + shellprog, + strerror(errno)); } /* in parent */ return pid; @@ -1129,8 +1247,8 @@ file_size(const char *file) if (!f) { - fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"), - progname, file, strerror(errno)); + diag(_("could not open file \"%s\" for reading: %s\n"), + file, strerror(errno)); return -1; } fseek(f, 0, SEEK_END); @@ -1151,8 +1269,8 @@ file_line_count(const char *file) if (!f) { - fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"), - progname, file, strerror(errno)); + diag(_("could not open file \"%s\" for reading: %s\n"), + file, strerror(errno)); return -1; } while ((c = fgetc(f)) != EOF) @@ -1193,9 +1311,8 @@ make_directory(const char *dir) { if (mkdir(dir, S_IRWXU | S_IRWXG | S_IRWXO) < 0) { - fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), - progname, dir, strerror(errno)); - exit(2); + bail(_("could not create directory \"%s\": %s\n"), + dir, strerror(errno)); } } @@ -1245,8 +1362,7 @@ run_diff(const char *cmd, const char *filename) r = system(cmd); if (!WIFEXITED(r) || WEXITSTATUS(r) > 1) { - fprintf(stderr, _("diff command failed with status %d: %s\n"), r, cmd); - exit(2); + bail(_("diff command failed with status %d: %s\n"), r, cmd); } #ifdef WIN32 @@ -1256,8 +1372,7 @@ run_diff(const char *cmd, const char *filename) */ if (WEXITSTATUS(r) == 1 && file_size(filename) <= 0) { - fprintf(stderr, _("diff command not found: %s\n"), cmd); - exit(2); + bail(_("diff command not found: %s\n"), cmd); } #endif @@ -1328,9 +1443,8 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul alt_expectfile = get_alternative_expectfile(expectfile, i); if (!alt_expectfile) { - fprintf(stderr, _("Unable to check secondary comparison files: %s\n"), - strerror(errno)); - exit(2); + bail(_("Unable to check secondary comparison files: %s\n"), + strerror(errno)); } if (!file_exists(alt_expectfile)) @@ -1445,9 +1559,8 @@ wait_for_tests(PID_TYPE * pids, int *statuses, instr_time *stoptimes, if (p == INVALID_PID) { - fprintf(stderr, _("failed to wait for subprocesses: %s\n"), - strerror(errno)); - exit(2); + bail(_("failed to wait for subprocesses: %s\n"), + strerror(errno)); } #else DWORD exit_status; @@ -1456,9 +1569,8 @@ wait_for_tests(PID_TYPE * pids, int *statuses, instr_time *stoptimes, r = WaitForMultipleObjects(tests_left, active_pids, FALSE, INFINITE); if (r < WAIT_OBJECT_0 || r >= WAIT_OBJECT_0 + tests_left) { - fprintf(stderr, _("failed to wait for subprocesses: error code %lu\n"), - GetLastError()); - exit(2); + bail(_("failed to wait for subprocesses: error code %lu\n"), + GetLastError()); } p = active_pids[r - WAIT_OBJECT_0]; /* compact the active_pids array */ @@ -1477,7 +1589,7 @@ wait_for_tests(PID_TYPE * pids, int *statuses, instr_time *stoptimes, statuses[i] = (int) exit_status; INSTR_TIME_SET_CURRENT(stoptimes[i]); if (names) - status(" %s", names[i]); + note_detail(" %s", names[i]); tests_left--; break; } @@ -1496,21 +1608,21 @@ static void log_child_failure(int exitstatus) { if (WIFEXITED(exitstatus)) - status(_(" (test process exited with exit code %d)"), - WEXITSTATUS(exitstatus)); + diag(_("(test process exited with exit code %d)\n"), + WEXITSTATUS(exitstatus)); else if (WIFSIGNALED(exitstatus)) { #if defined(WIN32) - status(_(" (test process was terminated by exception 0x%X)"), - WTERMSIG(exitstatus)); + diag(_("(test process was terminated by exception 0x%X)\n"), + WTERMSIG(exitstatus)); #else - status(_(" (test process was terminated by signal %d: %s)"), - WTERMSIG(exitstatus), pg_strsignal(WTERMSIG(exitstatus))); + diag(_("(test process was terminated by signal %d: %s)\n"), + WTERMSIG(exitstatus), pg_strsignal(WTERMSIG(exitstatus))); #endif } else - status(_(" (test process exited with unrecognized status %d)"), - exitstatus); + diag(_("(test process exited with unrecognized status %d)\n"), + exitstatus); } /* @@ -1542,9 +1654,8 @@ run_schedule(const char *schedule, test_start_function startfunc, scf = fopen(schedule, "r"); if (!scf) { - fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"), - progname, schedule, strerror(errno)); - exit(2); + bail(_("could not open file \"%s\" for reading: %s\n"), + schedule, strerror(errno)); } while (fgets(scbuf, sizeof(scbuf), scf)) @@ -1582,9 +1693,8 @@ run_schedule(const char *schedule, test_start_function startfunc, } else { - fprintf(stderr, _("syntax error in schedule file \"%s\" line %d: %s\n"), - schedule, line_num, scbuf); - exit(2); + bail(_("syntax error in schedule file \"%s\" line %d: %s\n"), + schedule, line_num, scbuf); } num_tests = 0; @@ -1600,9 +1710,8 @@ run_schedule(const char *schedule, test_start_function startfunc, if (num_tests >= MAX_PARALLEL_TESTS) { - fprintf(stderr, _("too many parallel tests (more than %d) in schedule file \"%s\" line %d: %s\n"), - MAX_PARALLEL_TESTS, schedule, line_num, scbuf); - exit(2); + bail(_("too many parallel tests (more than %d) in schedule file \"%s\" line %d: %s\n"), + MAX_PARALLEL_TESTS, schedule, line_num, scbuf); } sav = *c; *c = '\0'; @@ -1624,14 +1733,12 @@ run_schedule(const char *schedule, test_start_function startfunc, if (num_tests == 0) { - fprintf(stderr, _("syntax error in schedule file \"%s\" line %d: %s\n"), - schedule, line_num, scbuf); - exit(2); + bail(_("syntax error in schedule file \"%s\" line %d: %s\n"), + schedule, line_num, scbuf); } if (num_tests == 1) { - status(_("test %-28s ... "), tests[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); @@ -1639,16 +1746,15 @@ run_schedule(const char *schedule, test_start_function startfunc, } else if (max_concurrent_tests > 0 && max_concurrent_tests < num_tests) { - fprintf(stderr, _("too many parallel tests (more than %d) in schedule file \"%s\" line %d: %s\n"), - max_concurrent_tests, schedule, line_num, scbuf); - exit(2); + bail(_("too many parallel tests (more than %d) in schedule file \"%s\" line %d: %s\n"), + max_concurrent_tests, schedule, line_num, scbuf); } else if (max_connections > 0 && max_connections < num_tests) { int oldest = 0; - status(_("parallel group (%d tests, in groups of %d): "), - num_tests, max_connections); + note(_("parallel group (%d tests, in groups of %d): "), + num_tests, max_connections); for (i = 0; i < num_tests; i++) { if (i - oldest >= max_connections) @@ -1664,18 +1770,18 @@ run_schedule(const char *schedule, test_start_function startfunc, wait_for_tests(pids + oldest, statuses + oldest, stoptimes + oldest, tests + oldest, i - oldest); - status_end(); + note_end(); } else { - status(_("parallel group (%d tests): "), num_tests); + note(_("parallel group (%d tests): "), num_tests); for (i = 0; i < num_tests; 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); - status_end(); + note_end(); } /* Check results for all tests */ @@ -1686,8 +1792,7 @@ run_schedule(const char *schedule, test_start_function startfunc, *tl; bool differ = false; - if (num_tests > 1) - status(_(" %-28s ... "), tests[i]); + INSTR_TIME_SUBTRACT(stoptimes[i], starttimes[i]); /* * Advance over all three lists simultaneously. @@ -1707,9 +1812,7 @@ run_schedule(const char *schedule, test_start_function startfunc, (*postfunc) (rl->str); newdiff = results_differ(tests[i], rl->str, el->str); if (newdiff && tl) - { - printf("%s ", tl->str); - } + diag(_("tag: %s\n"), tl->str); differ |= newdiff; } @@ -1726,30 +1829,14 @@ run_schedule(const char *schedule, test_start_function startfunc, break; } } - if (ignore) - { - status(_("failed (ignored)")); - fail_ignore_count++; - } - else - { - status(_("FAILED")); - fail_count++; - } + + test_status_failed(tests[i], ignore, INSTR_TIME_GET_MILLISEC(stoptimes[i]), (num_tests > 1)); } else - { - status(_("ok ")); /* align with FAILED */ - success_count++; - } + test_status_ok(tests[i], INSTR_TIME_GET_MILLISEC(stoptimes[i]), (num_tests > 1)); if (statuses[i] != 0) log_child_failure(statuses[i]); - - INSTR_TIME_SUBTRACT(stoptimes[i], starttimes[i]); - status(_(" %8.0f ms"), INSTR_TIME_GET_MILLISEC(stoptimes[i])); - - status_end(); } for (i = 0; i < num_tests; i++) @@ -1786,7 +1873,6 @@ run_single_test(const char *test, test_start_function startfunc, *tl; bool differ = false; - status(_("test %-28s ... "), test); pid = (startfunc) (test, &resultfiles, &expectfiles, &tags); INSTR_TIME_SET_CURRENT(starttime); wait_for_tests(&pid, &exit_status, &stoptime, NULL, 1); @@ -1809,30 +1895,19 @@ run_single_test(const char *test, test_start_function startfunc, (*postfunc) (rl->str); newdiff = results_differ(test, rl->str, el->str); if (newdiff && tl) - { - printf("%s ", tl->str); - } + diag(_("tag: %s\n"), tl->str); differ |= newdiff; } + INSTR_TIME_SUBTRACT(stoptime, starttime); + if (differ) - { - status(_("FAILED")); - fail_count++; - } + test_status_failed(test, false, INSTR_TIME_GET_MILLISEC(stoptime), false); else - { - status(_("ok ")); /* align with FAILED */ - success_count++; - } + test_status_ok(test, INSTR_TIME_GET_MILLISEC(stoptime), false); if (exit_status != 0) log_child_failure(exit_status); - - INSTR_TIME_SUBTRACT(stoptime, starttime); - status(_(" %8.0f ms"), INSTR_TIME_GET_MILLISEC(stoptime)); - - status_end(); } /* @@ -1854,9 +1929,8 @@ open_result_files(void) logfile = fopen(logfilename, "w"); if (!logfile) { - fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"), - progname, logfilename, strerror(errno)); - exit(2); + bail(_("%s: could not open file \"%s\" for writing: %s"), + progname, logfilename, strerror(errno)); } /* create the diffs file as empty */ @@ -1865,9 +1939,8 @@ open_result_files(void) difffile = fopen(difffilename, "w"); if (!difffile) { - fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"), - progname, difffilename, strerror(errno)); - exit(2); + bail(_("%s: could not open file \"%s\" for writing: %s"), + progname, difffilename, strerror(errno)); } /* we don't keep the diffs file open continuously */ fclose(difffile); @@ -1883,7 +1956,6 @@ drop_database_if_exists(const char *dbname) { StringInfo buf = psql_start_command(); - header(_("dropping database \"%s\""), dbname); /* Set warning level so we don't see chatter about nonexistent DB */ psql_add_command(buf, "SET client_min_messages = warning"); psql_add_command(buf, "DROP DATABASE IF EXISTS \"%s\"", dbname); @@ -1900,7 +1972,6 @@ create_database(const char *dbname) * We use template0 so that any installation-local cruft in template1 will * not mess up the tests. */ - header(_("creating database \"%s\""), dbname); if (encoding) psql_add_command(buf, "CREATE DATABASE \"%s\" TEMPLATE=template0 ENCODING='%s'%s", dbname, encoding, (nolocale) ? " LC_COLLATE='C' LC_CTYPE='C'" : ""); @@ -1922,10 +1993,7 @@ create_database(const char *dbname) * this will work whether or not the extension is preinstalled. */ for (sl = loadextension; sl != NULL; sl = sl->next) - { - header(_("installing %s"), sl->str); psql_command(dbname, "CREATE EXTENSION IF NOT EXISTS \"%s\"", sl->str); - } } static void @@ -1933,7 +2001,6 @@ drop_role_if_exists(const char *rolename) { StringInfo buf = psql_start_command(); - header(_("dropping role \"%s\""), rolename); /* Set warning level so we don't see chatter about nonexistent role */ psql_add_command(buf, "SET client_min_messages = warning"); psql_add_command(buf, "DROP ROLE IF EXISTS \"%s\"", rolename); @@ -1945,7 +2012,6 @@ create_role(const char *rolename, const _stringlist *granted_dbs) { StringInfo buf = psql_start_command(); - header(_("creating role \"%s\""), rolename); psql_add_command(buf, "CREATE ROLE \"%s\" WITH LOGIN", rolename); for (; granted_dbs != NULL; granted_dbs = granted_dbs->next) { @@ -2167,8 +2233,8 @@ regression_main(int argc, char *argv[], break; default: /* getopt_long already emitted a complaint */ - fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"), - progname); + pg_log_error_hint("Try \"%s --help\" for more information.", + progname); exit(2); } } @@ -2241,17 +2307,13 @@ regression_main(int argc, char *argv[], if (directory_exists(temp_instance)) { - header(_("removing existing temp instance")); if (!rmtree(temp_instance, true)) { - fprintf(stderr, _("\n%s: could not remove temp instance \"%s\"\n"), - progname, temp_instance); - exit(2); + bail(_("%s: could not remove temp instance \"%s\""), + progname, temp_instance); } } - header(_("creating temporary instance")); - /* make the temp instance top directory */ make_directory(temp_instance); @@ -2261,7 +2323,6 @@ regression_main(int argc, char *argv[], make_directory(buf); /* initdb */ - header(_("initializing database system")); snprintf(buf, sizeof(buf), "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync%s%s > \"%s/log/initdb.log\" 2>&1", bindir ? bindir : "", @@ -2273,8 +2334,8 @@ regression_main(int argc, char *argv[], fflush(NULL); if (system(buf)) { - fprintf(stderr, _("\n%s: initdb failed\nExamine %s/log/initdb.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf); - exit(2); + bail(_("%s: initdb failed\nExamine %s/log/initdb.log for the reason.\nCommand was: %s"), + progname, outputdir, buf); } /* @@ -2289,8 +2350,8 @@ regression_main(int argc, char *argv[], pg_conf = fopen(buf, "a"); if (pg_conf == NULL) { - fprintf(stderr, _("\n%s: could not open \"%s\" for adding extra config: %s\n"), progname, buf, strerror(errno)); - exit(2); + bail(_("%s: could not open \"%s\" for adding extra config: %s"), + progname, buf, strerror(errno)); } fputs("\n# Configuration added by pg_regress\n\n", pg_conf); fputs("log_autovacuum_min_duration = 0\n", pg_conf); @@ -2309,8 +2370,8 @@ regression_main(int argc, char *argv[], extra_conf = fopen(temp_config, "r"); if (extra_conf == NULL) { - fprintf(stderr, _("\n%s: could not open \"%s\" to read extra config: %s\n"), progname, temp_config, strerror(errno)); - exit(2); + bail(_("%s: could not open \"%s\" to read extra config: %s"), + progname, temp_config, strerror(errno)); } while (fgets(line_buf, sizeof(line_buf), extra_conf) != NULL) fputs(line_buf, pg_conf); @@ -2349,14 +2410,14 @@ regression_main(int argc, char *argv[], if (port_specified_by_user || i == 15) { - fprintf(stderr, _("port %d apparently in use\n"), port); + note(_("port %d apparently in use\n"), port); if (!port_specified_by_user) - fprintf(stderr, _("%s: could not determine an available port\n"), progname); - fprintf(stderr, _("Specify an unused port using the --port option or shut down any conflicting PostgreSQL servers.\n")); - exit(2); + note(_("could not determine an available port\n")); + bail(_("Specify an unused port using the --port option or shut down any conflicting PostgreSQL servers.")); } - fprintf(stderr, _("port %d apparently in use, trying %d\n"), port, port + 1); + note(_("port %d apparently in use, trying %d\n"), + port, port + 1); port++; sprintf(s, "%d", port); setenv("PGPORT", s, 1); @@ -2368,7 +2429,6 @@ regression_main(int argc, char *argv[], /* * Start the temp postmaster */ - header(_("starting postmaster")); snprintf(buf, sizeof(buf), "\"%s%spostgres\" -D \"%s/data\" -F%s " "-c \"listen_addresses=%s\" -k \"%s\" " @@ -2380,11 +2440,7 @@ regression_main(int argc, char *argv[], outputdir); postmaster_pid = spawn_process(buf); if (postmaster_pid == INVALID_PID) - { - fprintf(stderr, _("\n%s: could not spawn postmaster: %s\n"), - progname, strerror(errno)); - exit(2); - } + bail(_("could not spawn postmaster: %s\n"), strerror(errno)); /* * Wait till postmaster is able to accept connections; normally this @@ -2419,16 +2475,16 @@ regression_main(int argc, char *argv[], if (WaitForSingleObject(postmaster_pid, 0) == WAIT_OBJECT_0) #endif { - fprintf(stderr, _("\n%s: postmaster failed\nExamine %s/log/postmaster.log for the reason\n"), progname, outputdir); - exit(2); + bail(_("postmaster failed, examine %s/log/postmaster.log for the reason\n"), + outputdir); } pg_usleep(1000000L); } if (i >= wait_seconds) { - fprintf(stderr, _("\n%s: postmaster did not respond within %d seconds\nExamine %s/log/postmaster.log for the reason\n"), - progname, wait_seconds, outputdir); + diag(_("postmaster did not respond within %d seconds, examine %s/log/postmaster.log for the reason\n"), + wait_seconds, outputdir); /* * If we get here, the postmaster is probably wedged somewhere in @@ -2437,17 +2493,14 @@ regression_main(int argc, char *argv[], * attempts. */ #ifndef WIN32 - if (kill(postmaster_pid, SIGKILL) != 0 && - errno != ESRCH) - fprintf(stderr, _("\n%s: could not kill failed postmaster: %s\n"), - progname, strerror(errno)); + if (kill(postmaster_pid, SIGKILL) != 0 && errno != ESRCH) + bail(_("could not kill failed postmaster: %s"), strerror(errno)); #else if (TerminateProcess(postmaster_pid, 255) == 0) - fprintf(stderr, _("\n%s: could not kill failed postmaster: error code %lu\n"), - progname, GetLastError()); + bail(_("could not kill failed postmaster: error code %lu"), + GetLastError()); #endif - - exit(2); + bail(_("postmaster failed")); } postmaster_running = true; @@ -2458,8 +2511,8 @@ regression_main(int argc, char *argv[], #else #define ULONGPID(x) (unsigned long) (x) #endif - printf(_("running on port %d with PID %lu\n"), - port, ULONGPID(postmaster_pid)); + note(_("running on port %d with PID %lu\n"), + port, ULONGPID(postmaster_pid)); } else { @@ -2490,8 +2543,6 @@ regression_main(int argc, char *argv[], /* * Ready to run the tests */ - header(_("running regression test queries")); - for (sl = schedulelist; sl != NULL; sl = sl->next) { run_schedule(sl->str, startfunc, postfunc); @@ -2507,7 +2558,6 @@ regression_main(int argc, char *argv[], */ if (temp_instance) { - header(_("shutting down postmaster")); stop_postmaster(); } @@ -2518,62 +2568,63 @@ regression_main(int argc, char *argv[], */ if (temp_instance && fail_count == 0 && fail_ignore_count == 0) { - header(_("removing temporary instance")); if (!rmtree(temp_instance, true)) - fprintf(stderr, _("\n%s: could not remove temp instance \"%s\"\n"), - progname, temp_instance); + diag("could not remove temp instance \"%s\"\n", + temp_instance); } - fclose(logfile); + /* + * Emit a TAP compliant Plan + */ + plan((fail_count + fail_ignore_count + success_count)); /* * Emit nice-looking summary message */ if (fail_count == 0 && fail_ignore_count == 0) - snprintf(buf, sizeof(buf), - _(" All %d tests passed. "), - success_count); - else if (fail_count == 0) /* fail_count=0, fail_ignore_count>0 */ - snprintf(buf, sizeof(buf), - _(" %d of %d tests passed, %d failed test(s) ignored. "), - success_count, - success_count + fail_ignore_count, - fail_ignore_count); - else if (fail_ignore_count == 0) /* fail_count>0 && fail_ignore_count=0 */ - snprintf(buf, sizeof(buf), - _(" %d of %d tests failed. "), - fail_count, - success_count + fail_count); + note(_("All %d tests passed.\n"), success_count); + /* fail_count=0, fail_ignore_count>0 */ + else if (fail_count == 0) + note(_("%d of %d tests passed, %d failed test(s) ignored.\n"), + success_count, + success_count + fail_ignore_count, + fail_ignore_count); + /* fail_count>0 && fail_ignore_count=0 */ + else if (fail_ignore_count == 0) + diag(_("%d of %d tests failed.\n"), + fail_count, + success_count + fail_count); + /* fail_count>0 && fail_ignore_count>0 */ else - /* fail_count>0 && fail_ignore_count>0 */ - snprintf(buf, sizeof(buf), - _(" %d of %d tests failed, %d of these failures ignored. "), - fail_count + fail_ignore_count, - success_count + fail_count + fail_ignore_count, - fail_ignore_count); - - putchar('\n'); - for (i = strlen(buf); i > 0; i--) - putchar('='); - printf("\n%s\n", buf); - for (i = strlen(buf); i > 0; i--) - putchar('='); - putchar('\n'); - putchar('\n'); + diag(_("%d of %d tests failed, %d of these failures ignored.\n"), + fail_count + fail_ignore_count, + success_count + fail_count + fail_ignore_count, + fail_ignore_count); + + if (fail_count > 0 || fail_ignore_count > 0) + diag(_("Failed and ignored tests:%s\n"), failed_tests->data); if (file_size(difffilename) > 0) { - printf(_("The differences that caused some tests to fail can be viewed in the\n" - "file \"%s\". A copy of the test summary that you see\n" - "above is saved in the file \"%s\".\n\n"), - difffilename, logfilename); + diag(_("The differences that caused some tests to fail can be viewed in the file \"%s\".\n"), + difffilename); + diag(_("A copy of the test summary that you see above is saved in the file \"%s\".\n"), + logfilename); } else { unlink(difffilename); unlink(logfilename); + + free(difffilename); + difffilename = NULL; + free(logfilename); + logfilename = NULL; } + fclose(logfile); + logfile = NULL; + if (fail_count != 0) exit(1); -- 2.34.1
