Tighten up Windows CRLF conversion in our TAP test scripts.

tglsfdc · tglsfdc · commit 91bdf499b37b · 2020-07-08T20:25:52.000-04:00
The previous approach was to search-and-destroy all \r occurrences no matter what. That seems more likely to hide bugs than anything else; indeed it seems to be hiding one now. Fix things so that we only transform \r\n to \n. Side effects: must do this before, not after, chomp'ing if we're going to chomp, else we'd fail to clean up a trailing \r\n. Also, remove safe_psql's redundant repetition of what psql already did; else it might reduce \r\r\n to \n, which is exactly the scenario I'm hoping to expose. Perhaps this should be back-patched, but for now I'm content to see what happens in HEAD. Discussion: https://postgr.es/m/412ae8da-76bb-640f-039a-f3513499e53d@gmx.net
diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm
@@ -112,7 +112,7 @@ sub check_query
 	}
 	else
 	{
-		$stdout =~ s/\r//g if $Config{osname} eq 'msys';
+		$stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
 		is($stdout, $expected_stdout, "$test_name: query result matches");
 	}
 	return;
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
@@ -1324,7 +1324,6 @@ sub safe_psql
 		print "\n#### End standard error\n";
 	}
 
-	$stdout =~ s/\r//g if $TestLib::windows_os;
 	return $stdout;
 }
 
@@ -1515,14 +1514,14 @@ sub psql
 
 	if (defined $$stdout)
 	{
+		$$stdout =~ s/\r\n/\n/g if $TestLib::windows_os;
 		chomp $$stdout;
-		$$stdout =~ s/\r//g if $TestLib::windows_os;
 	}
 
 	if (defined $$stderr)
 	{
+		$$stderr =~ s/\r\n/\n/g if $TestLib::windows_os;
 		chomp $$stderr;
-		$$stderr =~ s/\r//g if $TestLib::windows_os;
 	}
 
 	# See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR
@@ -1652,8 +1651,8 @@ sub poll_query_until
 	{
 		my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
 
+		$stdout =~ s/\r\n/\n/g if $TestLib::windows_os;
 		chomp($stdout);
-		$stdout =~ s/\r//g if $TestLib::windows_os;
 
 		if ($stdout eq $expected)
 		{
@@ -1668,8 +1667,8 @@ sub poll_query_until
 
 	# The query result didn't change in 180 seconds. Give up. Print the
 	# output from the last attempt, hopefully that's useful for debugging.
+	$stderr =~ s/\r\n/\n/g if $TestLib::windows_os;
 	chomp($stderr);
-	$stderr =~ s/\r//g if $TestLib::windows_os;
 	diag qq(poll_query_until timed out executing this query:
 $query
 expecting this output:
@@ -2113,8 +2112,8 @@ sub pg_recvlogical_upto
 		}
 	};
 
-	$stdout =~ s/\r//g if $TestLib::windows_os;
-	$stderr =~ s/\r//g if $TestLib::windows_os;
+	$stdout =~ s/\r\n/\n/g if $TestLib::windows_os;
+	$stderr =~ s/\r\n/\n/g if $TestLib::windows_os;
 
 	if (wantarray)
 	{
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
@@ -430,7 +430,7 @@ sub slurp_file
 		CloseHandle($fHandle)
 		  or die "could not close \"$filename\": $^E\n";
 	}
-	$contents =~ s/\r//g if $Config{osname} eq 'msys';
+	$contents =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
 	return $contents;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -112,7 +112,7 @@ sub check_query`
`112`	`112`	`}`
`113`	`113`	`else`
`114`	`114`	`{`
`115`		`- $stdout =~ s/\r//g if $Config{osname} eq 'msys';`
	`115`	`+ $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';`
`116`	`116`	`is($stdout, $expected_stdout, "$test_name: query result matches");`
`117`	`117`	`}`
`118`	`118`	`return;`
Original file line number	Diff line number	Diff line change
`@@ -1324,7 +1324,6 @@ sub safe_psql`
`1324`	`1324`	`print "\n#### End standard error\n";`
`1325`	`1325`	`}`
`1326`	`1326`
`1327`		`- $stdout =~ s/\r//g if $TestLib::windows_os;`
`1328`	`1327`	`return $stdout;`
`1329`	`1328`	`}`
`1330`	`1329`
`@@ -1515,14 +1514,14 @@ sub psql`
`1515`	`1514`
`1516`	`1515`	`if (defined $$stdout)`
`1517`	`1516`	`{`
	`1517`	`+ $$stdout =~ s/\r\n/\n/g if $TestLib::windows_os;`
`1518`	`1518`	`chomp $$stdout;`
`1519`		`- $$stdout =~ s/\r//g if $TestLib::windows_os;`
`1520`	`1519`	`}`
`1521`	`1520`
`1522`	`1521`	`if (defined $$stderr)`
`1523`	`1522`	`{`
	`1523`	`+ $$stderr =~ s/\r\n/\n/g if $TestLib::windows_os;`
`1524`	`1524`	`chomp $$stderr;`
`1525`		`- $$stderr =~ s/\r//g if $TestLib::windows_os;`
`1526`	`1525`	`}`
`1527`	`1526`
`1528`	`1527`	`# See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR`
`@@ -1652,8 +1651,8 @@ sub poll_query_until`
`1652`	`1651`	`{`
`1653`	`1652`	`my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;`
`1654`	`1653`
	`1654`	`+ $stdout =~ s/\r\n/\n/g if $TestLib::windows_os;`
`1655`	`1655`	`chomp($stdout);`
`1656`		`- $stdout =~ s/\r//g if $TestLib::windows_os;`
`1657`	`1656`
`1658`	`1657`	`if ($stdout eq $expected)`
`1659`	`1658`	`{`
`@@ -1668,8 +1667,8 @@ sub poll_query_until`
`1668`	`1667`
`1669`	`1668`	`# The query result didn't change in 180 seconds. Give up. Print the`
`1670`	`1669`	`# output from the last attempt, hopefully that's useful for debugging.`
	`1670`	`+ $stderr =~ s/\r\n/\n/g if $TestLib::windows_os;`
`1671`	`1671`	`chomp($stderr);`
`1672`		`- $stderr =~ s/\r//g if $TestLib::windows_os;`
`1673`	`1672`	`diag qq(poll_query_until timed out executing this query:`
`1674`	`1673`	`$query`
`1675`	`1674`	`expecting this output:`
`@@ -2113,8 +2112,8 @@ sub pg_recvlogical_upto`
`2113`	`2112`	`}`
`2114`	`2113`	`};`
`2115`	`2114`
`2116`		`- $stdout =~ s/\r//g if $TestLib::windows_os;`
`2117`		`- $stderr =~ s/\r//g if $TestLib::windows_os;`
	`2115`	`+ $stdout =~ s/\r\n/\n/g if $TestLib::windows_os;`
	`2116`	`+ $stderr =~ s/\r\n/\n/g if $TestLib::windows_os;`
`2118`	`2117`
`2119`	`2118`	`if (wantarray)`
`2120`	`2119`	`{`
Original file line number	Diff line number	Diff line change
`@@ -430,7 +430,7 @@ sub slurp_file`
`430`	`430`	`CloseHandle($fHandle)`
`431`	`431`	`or die "could not close \"$filename\": $^E\n";`
`432`	`432`	`}`
`433`		`- $contents =~ s/\r//g if $Config{osname} eq 'msys';`
	`433`	`+ $contents =~ s/\r\n/\n/g if $Config{osname} eq 'msys';`
`434`	`434`	`return $contents;`
`435`	`435`	`}`
`436`	`436`