Re: TestLib::command_fails_like enhancement

On 11/8/19 9:22 AM, Andrew Dunstan wrote:
...
> This will need to be rewritten in light of the above, but see
> <https://www.postgresql.org/message-id/87b1e36b-e36a-add5-1a9b-9fa34914a256@2ndQuadrant.com>

Thanks for the reference. Having read your motivating example, this new
review reverses some of what I suggested in the prior review.

In the existing TestLib.pm, there are eight occurrences of nearly
identical usages of IPC::Run scattered through similar functions:

run_command:
my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;

check_pg_config:
my $result = IPC::Run::run [ 'pg_config', '--includedir' ], '>',
\$stdout, '2>', \$stderr
or die "could not execute pg_config";

program_help_ok:
my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
\$stderr;

program_version_ok:
my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
\$stderr;

program_options_handling_ok:
my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
\$stdout,
'2>', \$stderr;

command_like:
my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;

command_like_safe:
my $result = IPC::Run::run $cmd, '>', $stdoutfile, '2>', $stderrfile;

command_fails_like:
my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr,
@extra_ipcrun_opts;

One rational motive for designing TestLib with so much code duplication
is to make the tests that use the library easier to read:

command_like_safe(foo);
command_like(bar);
command_fails_like(baz);

which is easier to understand than:

command_like(foo, failure_mode => safe);
command_like(bar);
command_like(baz, failure => expected);

and so forth.

In line with that thinking, perhaps you should just create:

command_fails_without_tty_like(foo)

and be done, or perhaps:

command_fails_like(foo, tty => 'closed')

and still preserve some of the test readability. Will anyone like the
readability of your tests if you have:

command_fails_like(foo, extra_ipcrun_opts => ['<pty<', \$eof_in]) ?

Admittedly, "foo", "bar", and "baz" above are shorthand notation for
things in practice that are already somewhat hard to read, as in:

command_fails_like(
[ 'pg_dump', 'qqq', 'abc' ],
qr/\Qpg_dump: error: too many command-line arguments (first is
"abc")\E/,
'pg_dump: too many command-line arguments');

but adding more to that cruft just makes it worse. Right?

--
Mark Dilger