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