Hello Michaƫl,

I doubt that -V & -? are heavily tested:-) Patch works for me, though.

They are not, and the patch misses this area.

Indeed.

I don't think that it is a bad idea to improve things the way you are

For the record, this is not my patch, I'm merely reviewing it.

doing, however you should extend program_version_ok() and program_help_ok() in src/test/perl/TestLib.pm so as short options are tested for two reasons:

Interesting, I did not notice these functions before. I fully agree that manual testing is a pain for such a simple change.

Do you mean something like the attached?

--
Fabien.
diff --git a/src/bin/pgbench/t/002_pgbench_no_server.pl 
b/src/bin/pgbench/t/002_pgbench_no_server.pl
index a2845a583b..24ecfec33e 100644
--- a/src/bin/pgbench/t/002_pgbench_no_server.pl
+++ b/src/bin/pgbench/t/002_pgbench_no_server.pl
@@ -56,6 +56,10 @@ sub pgbench_scripts
        return;
 }
 
+# help and version sanity checks
+program_help_ok("pgbench");
+program_version_ok("pgbench");
+
 #
 # Option various errors
 #
@@ -205,7 +209,7 @@ pgbench(
        'pgbench help');
 
 # Version
-pgbench('-V', 0, [qr{^pgbench .PostgreSQL. }], [qr{^$}], 'pgbench version');
+pgbench('-V', 0, [qr{^pgbench \(PostgreSQL\) \d+}], [qr{^$}], 'pgbench 
version');
 
 # list of builtins
 pgbench(
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 3a184a4fad..7799157335 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -409,13 +409,25 @@ sub program_help_ok
 {
        local $Test::Builder::Level = $Test::Builder::Level + 1;
        my ($cmd) = @_;
-       my ($stdout, $stderr);
+       my ($stdout, $stdout2, $stderr, $stderr2);
        print("# Running: $cmd --help\n");
        my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
          \$stderr;
+       my $result2 = IPC::Run::run [ $cmd, '-?' ], '>', \$stdout2, '2>',
+         \$stderr2;
        ok($result, "$cmd --help exit code 0");
+       ok($result2, "$cmd -? exit code 0");
        isnt($stdout, '', "$cmd --help goes to stdout");
+       is($stdout, $stdout2, "$cmd --help and -? have same output");
+       like($stdout, qr{Usage:}, "$cmd --help is about usage");
+       like($stdout, qr{\b$cmd\b}, "$cmd --help is about $cmd");
+       like($stdout, qr{--help}, "$cmd --help is about option --help");
+       like($stdout, qr{-\?}, "$cmd --help is about options -?");
+       like($stdout, qr{--version}, "$cmd --help is about options --version");
+       like($stdout, qr{-V}, "$cmd --help is about options -V");
+       # more sanity check on the output? eg Report bug to ..., options:...
        is($stderr, '', "$cmd --help nothing to stderr");
+       is($stderr2, '', "$cmd -? nothing to stderr");
        return;
 }
 
@@ -423,13 +435,19 @@ sub program_version_ok
 {
        local $Test::Builder::Level = $Test::Builder::Level + 1;
        my ($cmd) = @_;
-       my ($stdout, $stderr);
+       my ($stdout, $stdout2, $stderr, $stderr2);
        print("# Running: $cmd --version\n");
        my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
          \$stderr;
+       my $result2 = IPC::Run::run [ $cmd, '-V' ], '>', \$stdout2, '2>',
+         \$stderr2;
        ok($result, "$cmd --version exit code 0");
+       ok($result2, "$cmd -V exit code 0");
        isnt($stdout, '', "$cmd --version goes to stdout");
+       is($stdout, $stdout2, "$cmd --version and -V have same output");
+       like($stdout, qr{^$cmd \(PostgreSQL\) \d+(devel|\.\d+)\b}, "$cmd 
--version looks like a version");
        is($stderr, '', "$cmd --version nothing to stderr");
+       is($stderr2, '', "$cmd -V nothing to stderr");
        return;
 }
 

Reply via email to