Re: psql - improve test coverage from 41% to 88%

Hello Alvaro,

>>> I think the TestLib.pm changes should be done separately, not together
>>> with the rest of the hacking in this patch.
>>>
>>> Mostly, because I think they're going to cause trouble. Adding a
>>> parameter in the middle of the list may cause trouble for third-party
>>> users of TestLib.
>>
>> That is also what I thought, however, see below.
>
> I see. But you seem to have skipped my suggestion without considering
> it.

I did understand it, but as Tom did not want simple hocus-pocus, ISTM that
dynamically checking the argument type would not be considered a very good
idea either.

> I think the current API of these functions where they just receive a
> plain array of arguments, and all callers have to be patched in unison,
> is not very convenient.

I agree, but the no diff solution was rejected. I can bring one back, but
going against Tom's views has not proven a good move in the past.

> Also, I *think* your new icommand_checks method is the same as
> command_checks_all, except that you also have the "init" part.

Nope, it is an interactive version based on Expect, which sends input and
waits for output, the process is quite different from a simple one shot no
timeout exec version.

> So you're duplicating code because the original doesn't have
> functionality you need?

Yes, I'm creating a interactive validation variant.

> But why do that, if you could have *one* function that does both things?
> If some callers don't have the "init" part, just omit it from the
> parameters.

Although it could be abstracted somehow, I do not think that having one
function behaving so differently under the hood is a good idea. It is not
just the question of the init part.

> (Whether it's implemented using Expect or not should not matter. Either
> Expect works everywhere, and we can use it, or it doesn't and we can't.)

For me the question is not about Expect dependency, it is more about how
the test behaves.

--
Fabien.