Refactor all TAP test suites doing connection checks

michaelpq · michaelpq · commit c50624cdd248 · 2021-04-05T10:13:57.000+09:00
This commit refactors more TAP tests to adapt with the recent introduction of connect_ok() and connect_fails() in PostgresNode, introduced by 0d1a334. This changes the following test suites to use the same code paths for connection checks: - Kerberos - LDAP - SSL - Authentication Those routines are extended to be able to handle optional parameters that are set depending on each suite's needs, as of: - custom SQL query. - expected stderr matching pattern. - expected stdout matching pattern. The new design is extensible with more parameters, and there are some plans for those routines in the future with checks based on the contents of the backend logs. Author: Jacob Champion, Michael Paquier Discussion: https://postgr.es/m/d17b919e27474abfa55d97786cb9cfadfe2b59e9.camel@vmware.com
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
@@ -46,12 +46,19 @@ sub test_role
 
 	$status_string = 'success' if ($expected_res eq 0);
 
-	local $Test::Builder::Level = $Test::Builder::Level + 1;
-
-	my $res = $node->psql('postgres', undef, extra_params => [ '-U', $role, '-w' ]);
-	is($res, $expected_res,
-		"authentication $status_string for method $method, role $role");
-	return;
+	my $connstr = "user=$role";
+	my $testname =
+	  "authentication $status_string for method $method, role $role";
+
+	if ($expected_res eq 0)
+	{
+		$node->connect_ok($connstr, $testname);
+	}
+	else
+	{
+		# No checks of the error message, only the status code.
+		$node->connect_fails($connstr, $testname);
+	}
 }
 
 # Initialize primary node
diff --git a/src/test/authentication/t/002_saslprep.pl b/src/test/authentication/t/002_saslprep.pl
@@ -41,12 +41,20 @@ sub test_login
 
 	$status_string = 'success' if ($expected_res eq 0);
 
+	my $connstr = "user=$role";
+	my $testname =
+	  "authentication $status_string for role $role with password $password";
+
 	$ENV{"PGPASSWORD"} = $password;
-	my $res = $node->psql('postgres', undef, extra_params => [ '-U', $role ]);
-	is($res, $expected_res,
-		"authentication $status_string for role $role with password $password"
-	);
-	return;
+	if ($expected_res eq 0)
+	{
+		$node->connect_ok($connstr, $testname);
+	}
+	else
+	{
+		# No checks of the error message, only the status code.
+		$node->connect_fails($connstr, $testname);
+	}
 }
 
 # Initialize primary node. Force UTF-8 encoding, so that we can use non-ASCII
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
@@ -20,7 +20,7 @@
 
 if ($ENV{with_gssapi} eq 'yes')
 {
-	plan tests => 26;
+	plan tests => 30;
 }
 else
 {
@@ -182,28 +182,25 @@ END
 # Test connection success or failure, and if success, that query returns true.
 sub test_access
 {
-	my ($node, $role, $query, $expected_res, $gssencmode, $test_name, $expect_log_msg) = @_;
+	my ($node, $role, $query, $expected_res, $gssencmode, $test_name,
+		$expect_log_msg)
+	  = @_;
 
 	# need to connect over TCP/IP for Kerberos
-	my ($res, $stdoutres, $stderrres) = $node->psql(
-		'postgres',
-		"$query",
-		extra_params => [
-			'-XAtd',
-			$node->connstr('postgres')
-			  . " host=$host hostaddr=$hostaddr $gssencmode",
-			'-U',
-			$role
-		]);
-
-	# If we get a query result back, it should be true.
-	if ($res == $expected_res and $res eq 0)
+	my $connstr = $node->connstr('postgres')
+	  . " user=$role host=$host hostaddr=$hostaddr $gssencmode";
+
+	if ($expected_res eq 0)
 	{
-		is($stdoutres, "t", $test_name);
+		# The result is assumed to match "true", or "t", here.
+		$node->connect_ok(
+			$connstr, $test_name,
+			sql             => $query,
+			expected_stdout => qr/t/);
 	}
 	else
 	{
-		is($res, $expected_res, $test_name);
+		$node->connect_fails($connstr, $test_name);
 	}
 
 	# Verify specified log message is logged in the log file.
@@ -227,20 +224,12 @@ sub test_query
 	my ($node, $role, $query, $expected, $gssencmode, $test_name) = @_;
 
 	# need to connect over TCP/IP for Kerberos
-	my ($res, $stdoutres, $stderrres) = $node->psql(
-		'postgres',
-		"$query",
-		extra_params => [
-			'-XAtd',
-			$node->connstr('postgres')
-			  . " host=$host hostaddr=$hostaddr $gssencmode",
-			'-U',
-			$role
-		]);
-
-	is($res, 0, $test_name);
-	like($stdoutres, $expected, $test_name);
-	is($stderrres, "", $test_name);
+	my $connstr = $node->connstr('postgres')
+	  . " user=$role host=$host hostaddr=$hostaddr $gssencmode";
+
+	my ($stdoutres, $stderrres);
+
+	$node->connect_ok($connstr, $test_name, $query, $expected);
 	return;
 }
 
diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
@@ -163,12 +163,17 @@ END
 sub test_access
 {
 	my ($node, $role, $expected_res, $test_name) = @_;
-
-	my $res =
-	  $node->psql('postgres', undef,
-				  extra_params => [ '-U', $role, '-c', 'SELECT 1' ]);
-	is($res, $expected_res, $test_name);
-	return;
+	my $connstr = "user=$role";
+
+	if ($expected_res eq 0)
+	{
+		$node->connect_ok($connstr, $test_name);
+	}
+	else
+	{
+		# No checks of the error message, only the status code.
+		$node->connect_fails($connstr, $test_name);
+	}
 }
 
 note "simple bind";
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
@@ -1860,47 +1860,94 @@ sub interactive_psql
 
 =pod
 
-=item $node->connect_ok($connstr, $test_name)
+=item $node->connect_ok($connstr, $test_name, %params)
 
 Attempt a connection with a custom connection string.  This is expected
 to succeed.
 
+=over
+
+=item sql => B<value>
+
+If this parameter is set, this query is used for the connection attempt
+instead of the default.
+
+=item expected_stdout => B<value>
+
+If this regular expression is set, matches it with the output generated.
+
+=back
+
 =cut
 
 sub connect_ok
 {
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
-	my ($self, $connstr, $test_name) = @_;
-	my ($ret,  $stdout,  $stderr)    = $self->psql(
+	my ($self, $connstr, $test_name, %params) = @_;
+
+	my $sql;
+	if (defined($params{sql}))
+	{
+		$sql = $params{sql};
+	}
+	else
+	{
+		$sql = "SELECT \$\$connected with $connstr\$\$";
+	}
+
+	# Never prompt for a password, any callers of this routine should
+	# have set up things properly, and this should not block.
+	my ($ret, $stdout, $stderr) = $self->psql(
 		'postgres',
-		"SELECT \$\$connected with $connstr\$\$",
+		$sql,
+		extra_params  => ['-w'],
 		connstr       => "$connstr",
 		on_error_stop => 0);
 
-	ok($ret == 0, $test_name);
+	is($ret, 0, $test_name);
+
+	if (defined($params{expected_stdout}))
+	{
+		like($stdout, $params{expected_stdout}, "$test_name: matches");
+	}
 }
 
 =pod
 
-=item $node->connect_fails($connstr, $expected_stderr, $test_name)
+=item $node->connect_fails($connstr, $test_name, %params)
 
 Attempt a connection with a custom connection string.  This is expected
-to fail with a message that matches the regular expression
-$expected_stderr.
+to fail.
+
+=over
+
+=item expected_stderr => B<value>
+
+If this regular expression is set, matches it with the output generated.
+
+=back
 
 =cut
 
 sub connect_fails
 {
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
-	my ($self, $connstr, $expected_stderr, $test_name) = @_;
+	my ($self, $connstr, $test_name, %params) = @_;
+
+	# Never prompt for a password, any callers of this routine should
+	# have set up things properly, and this should not block.
 	my ($ret, $stdout, $stderr) = $self->psql(
 		'postgres',
 		"SELECT \$\$connected with $connstr\$\$",
-		connstr => "$connstr");
+		extra_params => ['-w'],
+		connstr      => "$connstr");
 
-	ok($ret != 0, $test_name);
-	like($stderr, $expected_stderr, "$test_name: matches");
+	isnt($ret, 0, $test_name);
+
+	if (defined($params{expected_stderr}))
+	{
+		like($stderr, $params{expected_stderr}, "$test_name: matches");
+	}
 }
 
 =pod
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
