Hi Michael, Brandon and Ben, Thank you for looking into this!
On Thu, 4 Dec 2025 at 10:42, Michael Paquier <[email protected]> wrote: > > On Wed, Dec 03, 2025 at 11:01:31PM -0800, Brandon Tat wrote: > > Regarding the function regression_log_helper(), this function reads > > all the lines in the logs at line 219 of > > src/test/recovery/t/027_stream_regress.pl. It seems wasteful to read > > the file again twice in read_file_ends(). Alternatively, we could > > read the file once within regression_log_helper() and index lines to > > emit the lines that we want. > > It seems to me that you are looking at v4-0001 and v4-0002 posted at > [1], which I did not author. So your suggestion would be to call > read_file_ends() once with the file opened once, with three modes > instead of the two presented in the patch: fetch the head, the tail, > or both at the same time. Yes, that would be more efficient. > > While looking at the patch with fresher eyes (didn't look at this > thread for a couple of months, sorry), it looks like there is no point > in having regression_log_helper() at all. We could just return the > tail and the head in a single call of read_file_ends() with two output > variables. Then we could embed in read_file_ends() the knowledge that > if we are dealing with a file that has less lines than twice > PG_TEST_FILE_READ_LINES, we can just print the whole file, returning > only the full contents as in a variable for what would have been the > head content, leaving the tail content empty. > > If somebody would like to send a patch among these lines, feel free.. I applied these feedbacks in v5. I wanted to cover all possible cases so I think 0002 might be a bit more complicated than it needs to be. What do you think about the current implementation? -- Regards, Nazir Bilal Yavuz Microsoft
From f3a4bbd092b545a519a4f28281ab14ffadb95fec Mon Sep 17 00:00:00 2001 From: Nazir Bilal Yavuz <[email protected]> Date: Thu, 4 Dec 2025 14:45:45 +0300 Subject: [PATCH v5 1/2] Add read_file_ends() helper to Utils.pm The read_file_ends() function reads lines from a file, from both head and tail directions. It takes how many lines to read as a 'line_count' argument. If the 'PG_TEST_FILE_READ_LINES' environment variable is set, it overrides the 'line_count'. Suggested-by: Michael Paquier <[email protected]> Discussion: https://postgr.es/m/CAN55FZ1D6KXvjSs7YGsDeadqCxNF3UUhjRAfforzzP0k-cE%3DbA%40mail.gmail.com --- src/test/perl/PostgreSQL/Test/Utils.pm | 52 ++++++++++++++++++++++++++ doc/src/sgml/regress.sgml | 8 ++++ 2 files changed, 60 insertions(+) diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm index 85d36a3171e..0291c43102f 100644 --- a/src/test/perl/PostgreSQL/Test/Utils.pm +++ b/src/test/perl/PostgreSQL/Test/Utils.pm @@ -68,6 +68,7 @@ our @EXPORT = qw( slurp_file append_to_file string_replace_file + read_file_ends check_mode_recursive chmod_recursive check_pg_config @@ -588,6 +589,57 @@ sub string_replace_file return; } +=pod + +=item read_file_ends(filename, line_count) + +Return $line_count lines from the head and tail of a given file. + +If the [2 * $line_count] is more than size of the file, return all lines of +the given file in the head variable. + +If the PG_TEST_FILE_READ_LINES environment variable is set, use it instead of +the line_count variable. + +=cut + +sub read_file_ends +{ + my ($filename, $line_count) = @_; + my (@head, @tail); + + # Use the PG_TEST_FILE_READ_LINES environment variable if it is set + if (defined $ENV{PG_TEST_FILE_READ_LINES}) + { + $line_count = $ENV{PG_TEST_FILE_READ_LINES}; + } + + return ([], 0, [], 0, 0, $line_count) if $line_count <= 0; + + open my $fh, '<', $filename or die "couldn't open file: $filename\n"; + my @lines = <$fh>; + close $fh; + + chomp @lines; + + my $total = scalar @lines; + + # If the file is small, return all lines in head variable + if (2 * $line_count >= $total) + { + @head = @lines; + @tail = (); + return (\@head, scalar(@head), \@tail, scalar(@tail), 1, $line_count); + } + + @head = @lines[ 0 .. $line_count - 1 ]; + @tail = @lines[ $total - $line_count .. $total - 1 ]; + + return (\@head, scalar(@head), \@tail, scalar(@tail), 0, $line_count); +} + + + =pod =item check_mode_recursive(dir, expected_dir_mode, expected_file_mode, ignore_list) diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml index fd1e142d559..5fed320c470 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -546,6 +546,14 @@ make check EXTRA_TESTS=numeric_big can run <command>diff</command> yourself, if you prefer.) </para> + <para> + For certain tests, the environment variable + <envar>PG_TEST_FILE_READ_LINES</envar> can be set to limit the number of + lines read from the output files. This is useful when test output files + are large or contain unnecessary content, allowing the test framework to + read only a specified number of lines for comparison. + </para> + <para> If for some reason a particular platform generates a <quote>failure</quote> for a given test, but inspection of the output convinces you that -- 2.51.0
From e4479f1b4f9c5b8e7d48d727cff2dfed9164fbf6 Mon Sep 17 00:00:00 2001 From: Nazir Bilal Yavuz <[email protected]> Date: Thu, 4 Dec 2025 14:47:25 +0300 Subject: [PATCH v5 2/2] Improve error reporting in 027_stream_regress test Previously, the 027_stream_regress test only reported that the regression test had failed, without showing the actual error. The detailed failure information was hidden in the regression.diffs file. This commit improves the situation by including the head and tail of regression.diffs directly in the failure message. This helps quickly identify the root cause without needing to open extra files. Suggested-by: Andres Freund <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Discussion: https://postgr.es/m/CAN55FZ1D6KXvjSs7YGsDeadqCxNF3UUhjRAfforzzP0k-cE%3DbA%40mail.gmail.com --- src/test/recovery/t/027_stream_regress.pl | 35 +++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/test/recovery/t/027_stream_regress.pl b/src/test/recovery/t/027_stream_regress.pl index 589c79d97d3..60025dd4c86 100644 --- a/src/test/recovery/t/027_stream_regress.pl +++ b/src/test/recovery/t/027_stream_regress.pl @@ -97,6 +97,41 @@ if ($rc != 0 && $primary_alive && $standby_alive) print "=== dumping $diffs ===\n"; print slurp_file($diffs); print "=== EOF ===\n"; + + # Dump out 50 lines from head and tail of regression diffs to the + # failure message. Number of lines may change depending on the + # PG_TEST_FILE_READ_LINES environment variable. + my ($head, $head_size, $tail, $tail_size, $entire_file, $line_count) + = read_file_ends($diffs, 50); + + if ($entire_file) + { + diag("\n=== dumping $diffs ===\n"); + foreach my $line (@$head) + { + diag($line); + } + } + if (!$entire_file && $head_size) + { + diag("\n=== dumping $line_count lines from head of $diffs ===\n"); + foreach my $line (@$head) + { + diag($line); + } + } + if (!$entire_file && $tail_size) + { + diag("\n=== dumping $line_count lines from tail of $diffs ===\n"); + foreach my $line (@$tail) + { + diag($line); + } + } + if ($entire_file || $head_size || $tail_size) + { + diag("=== EOF ===\n\n"); + } } } is($rc, 0, 'regression tests pass'); -- 2.51.0
