On Fri, 26 May 2023 at 04:09, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, May 25, 2023 at 06:34:20PM +0100, Dagfinn Ilmari Mannsåker wrote:
> > However, none of the other functions in ::Utils know anything about node
> > objects, which makes me think it should be a method on the node itself
> > (i.e. in PostgreSQL::Test::Cluster) instead. Also, I think log_contains
> > would be a better name, since it just returns a boolean. The name
> > find_in_log makes me think it would return the log lines matching the
> > pattern, or the position of the match in the file.
> >
> > In that case, the slurp_file() call would have to be fully qualified,
> > since ::Cluster uses an empty import list to avoid polluting the method
> > namespace with imported functions.
>
> Hmm. connect_ok() and connect_fails() in Cluster.pm have a similar
> log comparison logic, feeding from the offset of a log file. Couldn't
> you use the same code across the board for everything? Note that this
> stuff is parameterized so as it is possible to check if patterns match
> or do not match, for multiple patterns. It seems to me that we could
> use the new log finding routine there as well, so how about extending
> it a bit more? You would need, at least:
> - One parameter for log entries matching.
> - One parameter for log entries not matching.
I felt adding these to log_contains was making the function slightly
complex with multiple checks. I was not able to make it simple with
the approach I tried. How about having a common function
check_connect_log_contents which has the common log contents check for
connect_ok and connect_fails function like the v2-0002 patch attached.
Regards,
Vignesh