Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is (fwd)

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is (fwd)
Date: 2019-03-27 09:36:48
Message-ID: alpine.DEB.2.21.1903271008140.14554@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hello Heikki,

> I stared at the new test case for a while, and I must say it looks very
> cryptic. It's not exactly this patch's fault - all the pgbench tests are
> cryptic -

Perl is cryptic. Regexprs are cryptic.

> but I think we need to do something about that before adding any more
> tests. I'm not sure what exactly, but I'd like them to be more like
> pg_regress tests, where you have an expected output and you compare it
> with the actual output. I realize that's not easy, because there are a
> lot of varying numbers in the output, but we've got to do something.
> As a good first step, I wish the pgbench() function used named
> arguments. [...]
> You would have something like this:
> my $elapsed = pgbench(
> test_name => 'pgbench progress',
> opts => '-T 2 -P 1 -l --aggregate-interval=1'

I do not like them much in perl because it changes the code significantly,
but why not. That would be another patch anyway.

A lighter but efficient option would be to add a few comments on the
larger calls, see attached.

> My other complaint about the new test is that it does nothing to check
> if the output looks sensible. That's even harder to test, so it's
> probably not worth the trouble to try. But as it is, how much value does
> the test really have?

I do not understand. The list of expected regexpr on stdout and stderr
*are* the checks out the outputs, and there quite a few plenty of them?

> It would fail, if --progress caused pgbench to crash, or if no progress
> reports were printed at all, but I can't get very excited about that.

I cannot help you on this one: tests are never exciting:-)

The point is to improve code coverage, inducing that if a changes breaks
something the test should help notice before field reports.

There is nothing "exciting" about that, indeed.

The current test status is that one could write a dozen abort() in the
code and it would pass the tests.

Current tests coverage is much too low all over the project, see which looks abysmal to me. I would to put
pgbench in the green. I do think that the whole project should be in the
green when all tests are run. Although coverage is not everything there is
about testing, it is a good start.


In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2019-03-27 09:51:42 Re: amcheck verification for GiST
Previous Message Adrien NAYRAT 2019-03-27 09:22:20 Re: Log a sample of transactions