> On 4 Jul 2022, at 22:13, Andres Freund <andres@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/