public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andrew Dunstan <[email protected]>
To: vignesh C <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Dagfinn Ilmari Mannsåker <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Implement generalized sub routine find_in_log for tap test
Date: Sat, 27 May 2023 08:01:58 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALDaNm1sDD-MCW6zrxHCC7Ka5AzO-L4ejxeeTMtXPPN+F4vz2Q@mail.gmail.com>
References: <CALDaNm0YSiLpjCmajwLfidQrFOrLNKPQir7s__PeVvh9U3uoTQ@mail.gmail.com>
<[email protected]>
<ZG/[email protected]>
<CALDaNm1sDD-MCW6zrxHCC7Ka5AzO-L4ejxeeTMtXPPN+F4vz2Q@mail.gmail.com>
On 2023-05-26 Fr 20:35, vignesh C wrote:
> On Fri, 26 May 2023 at 04:09, Michael Paquier<[email protected]> 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
view thread (15+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Implement generalized sub routine find_in_log for tap test
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox