bug/oversight in TestLib.pm and PostgresNode.pm

Started by Erik Rijkersalmost 9 years ago4 messages
#1Erik Rijkers
er@xs4all.nl
2 attachment(s)

I am trying to re-create pgbench-over-logical-replication as a TAP-test.
(the wisdom of that might be doubted, and I appreciate comments on it
too, but it's really another subject).

While trying to test pgbench's stderr (looking for 'creating tables' in
output of the initialisation step) I ran into these two bugs (or
perhaps better 'oversights').

But especially the omission of command_fails_like() in PostgresNode.pm
feels like an bug.

In the end it was necessary to change TestLib.pm's command_like()
because command_fails_like() also checks for a non-zero return value
(which seems to make sense, but in this case not possible: pgbench
returns 0 on init with output on stderr).

make check-world passes without error

Thanks,

Erik Rijkers

Attachments:

0001-testlib-like-stderr.difftext/x-diff; charset=us-ascii; name=0001-testlib-like-stderr.diffDownload
--- src/test/perl/TestLib.pm.orig	2017-03-22 11:34:36.948857255 +0100
+++ src/test/perl/TestLib.pm	2017-03-22 14:36:56.793267113 +0100
@@ -289,13 +290,18 @@
 
 sub command_like
 {
-	my ($cmd, $expected_stdout, $test_name) = @_;
+	my ($cmd, $expected_stdout, $test_name, $expected_stderr) = @_;
 	my ($stdout, $stderr);
 	print("# Running: " . join(" ", @{$cmd}) . "\n");
 	my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
 	ok($result, "$test_name: exit code 0");
+	if (defined $expected_stderr) {
+		like($stderr, $expected_stderr, "$test_name: stderr matches");
+	}
+	else {
 	is($stderr, '', "$test_name: no stderr");
-	like($stdout, $expected_stdout, "$test_name: matches");
+	}
+	like($stdout, $expected_stdout, "$test_name: stdout matches");
 }
 
 sub command_fails_like
PostgresNode.pm.difftext/x-diff; charset=us-ascii; name=PostgresNode.pm.diffDownload
--- src/test/perl/PostgresNode.pm.orig	2017-03-22 15:58:58.690052999 +0100
+++ src/test/perl/PostgresNode.pm	2017-03-22 15:49:38.422777312 +0100
@@ -1283,6 +1283,23 @@
 
 =pod
 
+=item $node->command_fails_like(...) - TestLib::command_fails_like with our PGPORT
+
+See command_ok(...)
+
+=cut
+
+sub command_fails_like
+{
+	my $self = shift;
+
+	local $ENV{PGPORT} = $self->port;
+
+	TestLib::command_fails_like(@_);
+}
+
+=pod
+
 =item $node->issues_sql_like(cmd, expected_sql, test_name)
 
 Run a command on the node, then verify that $expected_sql appears in the
#2Michael Paquier
michael.paquier@gmail.com
In reply to: Erik Rijkers (#1)
Re: bug/oversight in TestLib.pm and PostgresNode.pm

On Thu, Mar 23, 2017 at 12:51 AM, Erik Rijkers <er@xs4all.nl> wrote:

While trying to test pgbench's stderr (looking for 'creating tables' in
output of the initialisation step) I ran into these two bugs (or perhaps
better 'oversights').

+   if (defined $expected_stderr) {
+       like($stderr, $expected_stderr, "$test_name: stderr matches");
+   }
+   else {
    is($stderr, '', "$test_name: no stderr");
-   like($stdout, $expected_stdout, "$test_name: matches");
+   }
To simplify that you could as well set expected_output to be an empty
string, and just use like() instead of is(), saving this if/else.

But especially the omission of command_fails_like() in PostgresNode.pm feels
like an bug.

+=item $node->command_fails_like(...) - TestLib::command_fails_like
with our PGPORT
+
+See command_ok(...)
+
+=cut
+
+sub command_fails_like
+{
+   my $self = shift;
+
+   local $ENV{PGPORT} = $self->port;
+
+   TestLib::command_fails_like(@_);
+}
Most likely a case where this is needed has not showed up, so +1 to
remove this inconsistency across the modules.
-- 
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Erik Rijkers
er@xs4all.nl
In reply to: Michael Paquier (#2)
2 attachment(s)
Re: bug/oversight in TestLib.pm and PostgresNode.pm

On 2017-03-23 03:28, Michael Paquier wrote:

On Thu, Mar 23, 2017 at 12:51 AM, Erik Rijkers <er@xs4all.nl> wrote:

While trying to test pgbench's stderr (looking for 'creating tables'
in
output of the initialisation step) I ran into these two bugs (or
perhaps
better 'oversights').

+   if (defined $expected_stderr) {
+       like($stderr, $expected_stderr, "$test_name: stderr matches");
+   }
+   else {
is($stderr, '', "$test_name: no stderr");
-   like($stdout, $expected_stdout, "$test_name: matches");
+   }
To simplify that you could as well set expected_output to be an empty
string, and just use like() instead of is(), saving this if/else.

(I'll assume you meant '$expected_stderr' (not 'expected_output'))

That would be nice but with that, other tests start complaining:
"doesn't look like a regex to me"

To avoid that, I uglified your version back to:

+       like($stderr, (defined $expected_stderr ? $expected_stderr : 
qr{}),
+                                       "$test_name: stderr matches");

I did it like that in the attached patch
(0001-testlib-like-stderr.diff).

The other (PostgresNode.pm.diff) is unchanged.

make check-world without error.

Thanks,

Erik Rijkers

Attachments:

0001-testlib-like-stderr.difftext/x-diff; name=0001-testlib-like-stderr.diffDownload
--- src/test/perl/TestLib.pm.orig	2017-03-23 08:11:16.034410936 +0100
+++ src/test/perl/TestLib.pm	2017-03-23 08:12:33.154132124 +0100
@@ -289,13 +289,14 @@
 
 sub command_like
 {
-	my ($cmd, $expected_stdout, $test_name) = @_;
+	my ($cmd, $expected_stdout, $test_name, $expected_stderr) = @_;
 	my ($stdout, $stderr);
 	print("# Running: " . join(" ", @{$cmd}) . "\n");
 	my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
 	ok($result, "$test_name: exit code 0");
-	is($stderr, '', "$test_name: no stderr");
-	like($stdout, $expected_stdout, "$test_name: matches");
+	like($stderr, (defined $expected_stderr ? $expected_stderr : qr{}),
+					"$test_name: stderr matches");
+	like($stdout, $expected_stdout, "$test_name: stdout matches");
 }
 
 sub command_fails_like
PostgresNode.pm.difftext/x-diff; name=PostgresNode.pm.diffDownload
--- src/test/perl/PostgresNode.pm.orig	2017-03-22 15:58:58.690052999 +0100
+++ src/test/perl/PostgresNode.pm	2017-03-22 15:49:38.422777312 +0100
@@ -1283,6 +1283,23 @@
 
 =pod
 
+=item $node->command_fails_like(...) - TestLib::command_fails_like with our PGPORT
+
+See command_ok(...)
+
+=cut
+
+sub command_fails_like
+{
+	my $self = shift;
+
+	local $ENV{PGPORT} = $self->port;
+
+	TestLib::command_fails_like(@_);
+}
+
+=pod
+
 =item $node->issues_sql_like(cmd, expected_sql, test_name)
 
 Run a command on the node, then verify that $expected_sql appears in the
#4Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Erik Rijkers (#1)
Re: bug/oversight in TestLib.pm and PostgresNode.pm

On 3/22/17 11:51, Erik Rijkers wrote:

While trying to test pgbench's stderr (looking for 'creating tables' in
output of the initialisation step) I ran into these two bugs (or
perhaps better 'oversights').

Perhaps pgbench should be printing progress messages to stdout instead?

But especially the omission of command_fails_like() in PostgresNode.pm
feels like an bug.

Yeah, that's just because no one has needed it yet.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers