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.
