Re: Implement generalized sub routine find_in_log for tap test

On 2023-05-26 Fr 20:35, vignesh C wrote:
> 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.


+    $offset = 0 unless defined $offset;


This is unnecessary, as slurp_file() handles it appropriately, and in 
fact doing this is slightly inefficient, as it will cause slurp_file to 
do a redundant seek.

FYI there's a simpler way to say it if we wanted to:

     $offset //= 0;


cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com