On 25/03/2019 20:13, Fabien COELHO wrote:
Attached is the remainder of the patch rebased to current head.
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 - 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. So instead of this:
my $elapsed = pgbench(
"-T 2 -P 1 -l --aggregate-interval=1"
. ' -S -b se@2 --rate=20 --latency-limit=1000 -j ' . $nthreads
. ' -c 3 -r',
0,
[ qr{type: multiple},
qr{clients: 3},
qr{threads: $nthreads},
# the shown duration is really -T argument value
qr{duration: 2 s},
qr{script 1: .* select only},
qr{script 2: .* select only},
qr{statement latencies in milliseconds},
qr{FROM pgbench_accounts} ],
[ qr{vacuum},
qr{progress: \d\b} ],
'pgbench progress', undef,
"--log-prefix=$bdir/001_pgbench_log_1");
You would have something like this:
my $elapsed = pgbench(
test_name => 'pgbench progress',
opts => '-T 2 -P 1 -l --aggregate-interval=1'
. ' -S -b se@2 --rate=20 --latency-limit=1000 -j ' . $nthreads
. ' -c 3 -r'
. '--log-prefix=$bdir/001_pgbench_log_1'
expected_stdout =>
[ qr{type: multiple},
qr{clients: 3},
qr{threads: $nthreads},
# the shown duration is really -T argument value
qr{duration: 2 s},
qr{script 1: .* select only},
qr{script 2: .* select only},
qr{statement latencies in milliseconds},
qr{FROM pgbench_accounts} ],
expected_stderr =>
[ qr{vacuum},
qr{progress: \d\b} ]
);
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? 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.
- Heikki
