From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
---|---|
To: | Nikolay Shaplov <dhyan(at)nataraj(dot)su> |
Cc: | PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pgbench tap tests & minor fixes |
Date: | 2017-04-25 16:42:04 |
Message-ID: | alpine.DEB.2.20.1704251823080.10770@lancre |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
From | Date | Subject | |
---|---|---|---|
Next Message | Konstantin Knizhnik | 2017-04-25 16:45:20 | Re: Cached plans and statement generalization |
Previous Message | Claudio Freire | 2017-04-25 16:37:13 | Re: PG 10 release notes |