From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | pgsql-committers(at)lists(dot)postgresql(dot)org, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pgsql: Fix pattern matching logic for logs in TAP tests of pgbench |
Date: | 2021-06-26 15:01:07 |
Message-ID: | 9f70b570-9aa2-29cc-8732-34cc6d16d798@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On 6/26/21 2:47 AM, Fabien COELHO wrote:
>
> Hello Andrew & Michaël,
>
> My 0.02€:
>
>> There's a whole lot wrong with this code. To start with, why is that
>> unchecked eval there.
>
> Yep. The idea was that other tests would go on being collected eg if
> the file is not found, but it should have been checked anyway.
>
>> And why is it reading in log files on its own instead of using
>> TestLib::slurp_file, which, among other things, normalizes line endings?
>
> Indeed.
>
> However, if slurp_file fails it raises an exception and aborts the
> whole TAP unexpectedly, which is pretty unclean. So I'd suggest to
> keep the eval, as attached. I tested it by changing the file name so
> that the slurp fails.
Seem quite unnecessary. We haven't found that to be an issue elsewhere
in the code where slurp_file is used. And in the present case we know
the file exists because we got its name from list_files().
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Fabien COELHO | 2021-06-26 16:08:19 | Re: pgsql: Fix pattern matching logic for logs in TAP tests of pgbench |
Previous Message | Fabien COELHO | 2021-06-26 06:47:34 | Re: pgsql: Fix pattern matching logic for logs in TAP tests of pgbench |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2021-06-26 15:42:03 | Re: Composite types as parameters |
Previous Message | Richard Guo | 2021-06-26 10:48:43 | Re: Using each rel as both outer and inner for JOIN_ANTI |