Re: pgbench tap tests & minor fixes

From: Nikolay Shaplov <dhyan(at)nataraj(dot)su>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pgbench tap tests & minor fixes
Date: 2017-04-20 12:48:16
Message-ID: 1709400.qunD4fS6on@x200m
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

В письме от 17 апреля 2017 14:51:45 пользователь Fabien COELHO написал:

> When developping new features for pgbench, I usually write some tests
> which are lost when the feature is committed. Given that I have submitted
> some more features and that part of pgbench code may be considered for
> sharing with pgsql, I think that improving the abysmal state of tests
> would be a good move.
Hi! Since I used to be good at perl, I like tests, and I've dealt with
postgres TAP tests before, I think I can review you patch. If it is OK for

For now I've just gave this patch a quick look through... But nevertheless I
have something to comment:

> The attached patch:
> (1) extends the existing perl tap test infrastructure with methods to test
> pgbench, i.e. "pgbench" which runs a pgbench test and "pgbench_likes"
> which allows to check for expectations.
I do not think it is good idea adding this functions to the
They are pgbench specific. I do not think anybody will need them outside of
pgbench tests.

Generally, these functions as they are now, should be placed is separate .pm
file. like it was done in src/test/ssl/

May be you should move some code into some kind of helpers and keep them in

Or write universal functions that can be used for testing any postgres console
tool. Then they can be placed in

> (3) add a lot of new very small tests so that coverage jumps from very low
> to over 90% for source files. I think that derived files (exprparse.c,
> exprscan.c) should be removed from coverage analysis.
> Previous coverage status:
> exprparse.y 0.0 % 0 / 77 0.0 % 0 / 8
> exprscan.l 0.0 % 0 / 102 0.0 % 0 / 8
> pgbench.c 28.3 % 485 / 1716 43.1 % 28 / 65
> New status:
> exprparse.y 96.1 % 73 / 76 100.0 % 8 / 8
> exprscan.l 92.8 % 90 / 97 100.0 % 8 / 8
> pgbench.c 90.4 % 1542 / 1705 96.9 % 63 / 65
> The test runtime is about doubled on my laptop, which is not too bad given
> the coverage achieved.
What tool did you use to calculate the coverage?

Lots of small test looks good at first glance, except the point that adding
lots of small files with pgbench scripts is not great idea. This makes code
tree too overloaded with no practical reason. I am speaking of
src/bin/pgbench/t/scripts/001_0* files.

I think all the data from this file should be somehow imported into and these files should be created only when tests is running,
and then removed when testing is done.

I think that job for creating and removing these files should be moved to
pgbench and pgbench_likes. But there is more then one way to do it. ;-)

That's what I've noticed so far... I will give this patch more attentive look

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

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Stark 2017-04-20 12:51:34 Re: Highly Variable Planning Times
Previous Message Michael Paquier 2017-04-20 12:41:25 Re: DROP SUBSCRIPTION, query cancellations and slot handling