From: | Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> |
---|---|
To: | Mark Dilger <hornschnorter(at)gmail(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: TestLib::command_fails_like enhancement |
Date: | 2019-11-25 13:08:56 |
Message-ID: | 2ab1d6d1-1cfa-1e90-9b0c-ae6767fb500f@2ndQuadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 11/11/19 4:28 PM, Mark Dilger wrote:
>
>
>>>>>
>>>>
>>>> On further consideration, I'm wondering why we don't just
>>>> unconditionally use a closed input pty for all these functions (except
>>>> run_log). None of them use any input, and making the client worry
>>>> about
>>>> whether or not to close it seems something of an abstraction break.
>>>> There would be no API change at all involved in this case, just a
>>>> bit of
>>>> extra cleanliness. Would need testing on Windows, I'll go and do that
>>>> now.
>>>>
>>>>
>>>> Thoughts?
>>>
>>> That sounds a lot better than your previous patch.
>>>
>>> PostgresNode.pm and TestLib.pm both invoke IPC::Run::run. If you
>>> change all the invocations in TestLib to close input pty, should you
>>> do the same for PostgresNode? I don't have a strong argument for
>>> doing so, but it seems cleaner to have both libraries invoking
>>> commands under identical conditions, such that if commands were
>>> borrowed from one library and called from the other they would behave
>>> the same.
>>>
>>> PostgresNode already uses TestLib, so perhaps setting up the
>>> environment can be abstracted into something, perhaps TestLib::run,
>>> and then used everywhere that IPC::Run::run currently is used.
>>
>>
>>
>> I don't think we need to do that. In the case of the PostgresNode.pm
>> uses we know what the executable is, unlike the the TestLib.pm cases.
>> They are our own executables and we don't expect them to be doing
>> anything funky with /dev/tty.
>
> Ok. I think your proposal sounds fine.
Here's a patch for that. The pty stuff crashes and burns on my Windows
test box, so there I just set stdin to an empty string via the usual
pipe mechanism.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment | Content-Type | Size |
---|---|---|
testlib-close-stdin.patch | text/x-patch | 4.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Dmitry Dolgov | 2019-11-25 13:37:25 | Re: proposal: new polymorphic types - commontype and commontypearray |
Previous Message | Amit Kapila | 2019-11-25 12:18:11 | Re: dropdb --force |