Re: [HACKERS] pgbench tap tests & minor fixes - Mailing list pgsql-hackers

В письме от 23 апреля 2017 22:02:25 пользователь Fabien COELHO написал:
> Hello Nikolay,
>
> >> Hmmm. The pre-existing TAP test in pgbench is about concurrent commits,
> >> it
> >> is not to test pgbench itself. Pgbench allows to run some programmable
> >> clients in parallel very easily, which cannot be done simply otherwise. I
> >> think that having it there would encourage such uses.
> >
> > Since none of us is Tom Lane, I think we have no moral right to encourage
> > somebody doing somebody in postgres.
>
> I do not get it.
>
> > People do what they think better to do.
>
> Probably.
>
> >> [...] abstraction. For me, all pg standard executables could have their
> >> methods in PostgresNode.pm.
> >
> > you are speaking about
> > local $ENV{PGPORT} = $self->port;
> > ?
>
> Yes. My point is that this is the internal stuff of PostgresNode and that
> it should stay inside that class. The point of the class is to organize
> postgres instances for testing, including client-server connections. From
> an object oriented point of view, the method to identify the server could
> vary, thus the testing part should not need to know, unless what is tested
> is this connection variations, hence it make sense to have it there.
>
> > Why do you need it here after all? Lots of TAP tests for bin utilities
> > runs
> > them using command_like function from TestLib.pm and need no setting
> > $ENV{PGPORT}.
>
> The test I've seen that do not need it do not connect to the server.
> In order to connect to the server they get through variants from
> PostgresNode which set the variables before calling the TestLib function.
>
> > Is pgbench so special? If it is so, may be it is good reason to
> > fix utilities from TestLib.pm, so they can take port from PostgresNode.
>
> Nope. TestLib does not know about PostgresNode, the idea is rather that
> PostgresNode knows and wraps around TestLib when needed.

Actually as I look at this part, I feeling an urge to rewrite this code, and
change it so, that all command_like calls were called in a context of certain
node, but it is subject for another patch. In this patch it would be good to
use TestLib functions as they are now, or with minimum modifications.
> > If in these tests we can use command_like instead of pgbench_likes it
> > would increase maintainability of the code.
>

> "command_like" variants do not look precisely at all results (status,
> stdout, stderr) and are limited to what they check (eg one regex at a
> time). Another thing I do not like is that they expect commands as a list
> of pieces, which is not very readable.
checking all this things sounds as a good idea.

>
> Now I can add a command_likes which allows to manage status, stdout and
> stderr and add that in TestLib & PostgresNode .
This is good idea too, I still do not understand why do you want to add it to
PostgresNode.

And name command_likes -- a very bad solution, as it can easily be confused
with command_like. If it is possible to do it backward compatible, I would try
to improve command_like, so it can check all the results. If it is not, I
would give this function a name that brings no confusion.

> >>> I think all the data from this file should be somehow imported into
> >>> 001_pgbench.pl and these files should be created only when tests is
> >>> running, and then removed when testing is done.
> >>
> >> Hmmm. I thought of that but was very unconvinced by the approach: pgbench
> >> basically always requires a file, so what the stuff was doing was
> >> writting
> >> the script into a file, checking for possible errors when doing so, then
> >> unlinking the file and checking for errors again after the run.
> >
> > I think I was wrong about deleting file after tests are finished. Better
> > keep them.
>
> Hmmm... Then what is the point not to have them as files to begin with?

Not to have them in source code tree in git.

The rest would be in the answer to another sum up letter.

--
Nikolay Shaplov, independent Perl & C/C++ developer. Available for hire.