On Fri, 2021-03-19 at 17:21 +0900, Michael Paquier wrote:
> On Thu, Mar 18, 2021 at 05:14:24PM +0900, Michael Paquier wrote:
> > Looking at 0001, I am not much a fan of relying on the position of the
> > matching pattern in the log file. Instead of relying on the logging
> > collector and one single file, why not just changing the generation of
> > the logfile and rely on the output of stderr by restarting the server?
For getting rid of the logging collector logic, this is definitely an
improvement. It was briefly discussed in [1] but I never got around to
trying it; thanks!
One additional improvement I would suggest, now that the rotation logic
is simpler than it was in my original patch, is to rotate the logfile
regardless of whether the test is checking the logs or not. (Similarly,
we can manually rotate after the block of test_query() calls.) That way
it's harder to match the last test's output.
> While looking at 0003, I have noticed that the new kerberos tests
> actually switch from a logic where one message pattern matches, to a
> logic where multiple message patterns match, but I don't see a problem
> with what I sent previously, as long as one consume once a log file
> and matches all the patterns once, say like the following in
> test_access():
The tradeoff is that if you need to check for log message order, or for
multiple instances of overlapping patterns, you still need some sort of
search-forward functionality. But looking over the tests, I don't see
any that truly *need* that yet. It's nice that the current patchset
enforces an "authenticated" line before an "authorized" line, but I
think it's nicer to not have the extra code.
I'll incorporate this approach into the patchset. Thanks!
--Jacob
[1] https://www.postgresql.org/message-id/f1fd9ccaf7ffb2327bf3c06120afeadd50c1db97.camel%40vmware.com
