Re: pgbench tap tests & minor fixes

В письме от 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
everyone.

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 PostgresNode.pm.
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
PostgresNode.pm.

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

> (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
001_pgbench.pl 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
soon.

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