Re: [HACKERS] pgbench tap tests & minor fixes

Hello Nikolay,

>>> - I agree to add a generic command TestLib & a wrapper in PostgresNode,
>>>
>>>   instead of having pgbench specific things in the later, then call
>>>   them from pgbench test script.
>>>
>>> - I still think that moving the pgbench scripts inside the test script
>>>   is a bad idea (tm).
>
> My sum up is the following:
>
> I see my job as a reviewer is to tell "I've read the code, and I am sure 
> it is good".

I'm ok with that. Does not mean I cannot argue if I happen to disagree on 
a point.

> I can tell this about this code, if:
>
> - There is no pgbench specific staff in src/test/perl. Or there should be
> _really_big_ reason for it.

ISTM that v2 I sent does not contain any pgbench specific code. However It 
contains new generic code to check for status and list of re on stdout & 
stderr.

> - All the testing is done via calls of TestLib.pm functions. May be these
> functions should be improved somehow. May be there should be some warper
> around them. But not direct IPC::Run::run call.

There is no call to IPC out of TestLib.pm in v2 I sent.

> - All the pgbench scripts are stored in one file. 36 files are not acceptable.
> I would include them in the test script itself. May be it can be done in other
> ways. But not 36 less then 100 byte files in source code tree...

I will write an ugly stuff if it is require to pass this patch, but I'm 
unconvinced that this is a good idea.

What it is issue with having 36 small files in a directory?

Pg source tree currently contains about 79 under 100 bytes files related 
to the insufficient test it is running, so this did not seem to be an 
issue in the past.

> May be I am wrong. I am not a guru. But then somebody else should say "I've
> read the code, and I am sure it is good" instead of me. And it would be his
> responsibility then. But if you ask me, issues listed above are very
> important, and until they are solved I can not tell "the code is good", and I
> see no way to persuade me. May be just ask somebody else...

Of all the issues you list, 2/3 are already fixed in the v2 I sent 
attached to the mail you are responding to, and I actually think that the 
last point is a bad idea, which I can implement and be sad about.

-- 
Fabien.