> On 4 Jul 2022, at 22:13, Andres Freund <and...@anarazel.de> wrote: > > Hi, > > On 2022-06-29 21:50:45 +0200, Daniel Gustafsson wrote: >> @@ -279,8 +648,7 @@ stop_postmaster(void) >> r = system(buf); >> if (r != 0) >> { >> - fprintf(stderr, _("\n%s: could not stop postmaster: >> exit code was %d\n"), >> - progname, r); >> + pg_log_error("could not stop postmaster: exit code was >> %d", r); >> _exit(2); /* not exit(), that >> could be recursive */ >> } > > There's a lot of stuff like this. Perhaps worth doing separately? I'm not sure > I unerstand where you used bail and where not. I assume it's mostly arund use > uf _exit() vs exit()?
Since bail will cause the entire testrun to be considered a failure, the idea was to avoid using bail() for any errors in tearing down the test harness after an otherwise successful test run. Moving to pg_log_error() can for sure be broken out into a separate patch from the rest of the set (if we at all want to do that, but it seemed logical to address when dealing with other output routines). >> + test_status_ok(tests[i]); >> >> 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])); >> + runtime(tests[i], >> INSTR_TIME_GET_MILLISEC(stoptimes[i])); > > Based on the discussion downthread, let's just always compute this and display > it even in the tap format? Sure, it's easy enough to do and include in the test description. The reason I left it out is that the test runners I played around with all hide those details and only show a running total. That of course doesn't mean that all runners will do that (and anyone running TAP output for human consumption will want it), so I agree with putting it in, I'll fix that up in a v6 shortly. -- Daniel Gustafsson https://vmware.com/