Hello Andrew & Michaël,
My 0.02€:
There's a whole lot wrong with this code. To start with, why is that
unchecked eval there.
Yep. The idea was that other tests would go on being collected eg if the
file is not found, but it should have been checked anyway.
And why is it reading in log files on its own instead of using
TestLib::slurp_file, which, among other things, normalizes line endings?
Indeed.
However, if slurp_file fails it raises an exception and aborts the whole
TAP unexpectedly, which is pretty unclean. So I'd suggest to keep the
eval, as attached. I tested it by changing the file name so that the slurp
fails.
There's a very good chance that this latter is the issue. It only
affects msys which is why you didn't see an issue on MSVC. And also, why
does it carefully unlink the log files so that any trace of what's gone
wrong is deleted?
Based on the little I've seen this file needs a serious code review.
Probably: My very old perl expertise is fading away because I'm not using
it much these days. Cannot say I miss it:-)
--
Fabien.
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 781cc08fb1..6a82a3494d 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -1190,29 +1190,32 @@ sub check_pgbench_logs
ok(@logs == $nb, "number of log files");
ok(grep(/\/$prefix\.\d+(\.\d+)?$/, @logs) == $nb, "file name format");
- my $log_number = 0;
for my $log (sort @logs)
{
- # Check the contents of each log file.
- my $contents_raw = slurp_file($log);
+ eval {
+ # Check the contents of each log file.
+ my $contents_raw = slurp_file($log);
- my @contents = split(/\n/, $contents_raw);
- my $clen = @contents;
- ok( $min <= $clen && $clen <= $max,
- "transaction count for $log ($clen)");
- my $clen_match = grep(/$re/, @contents);
- ok($clen_match == $clen, "transaction format for $prefix");
+ my @contents = split(/\n/, $contents_raw);
+ my $clen = @contents;
+ ok( $min <= $clen && $clen <= $max,
+ "transaction count for $log ($clen)");
+ my $clen_match = grep(/$re/, @contents);
+ ok($clen_match == $clen, "transaction format for $prefix");
- # Show more information if some logs don't match
- # to help with debugging.
- if ($clen_match != $clen)
- {
- foreach my $log (@contents)
+ # Show more information if some logs don't match
+ # to help with debugging.
+ if ($clen_match != $clen)
{
- print "# Log entry not matching: $log\n"
- unless $log =~ /$re/;
+ foreach my $log (@contents)
+ {
+ print "# Log entry not matching: $log\n"
+ unless $log =~ /$re/;
+ }
}
- }
+ };
+ # Tell if an exception was raised
+ ok(0, "Error while checking log file \"$log\": $@") if $@;
}
return;
}
