Re: Improve error reporting in 027_stream_regress test

Hi,

On Thu, 24 Jul 2025 at 13:34, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, 22 Jul 2025 at 03:56, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> >
> > On Mon, Jul 21, 2025 at 11:53:00AM +0300, Nazir Bilal Yavuz wrote:
> > > I realized that we actually don't trim the file, we do the opposite;
> > > read the file from both ends. Sorry for not realizing earlier. I will
> > > update the remaining patches according to that but I think trim_file()
> > > is helpful, too. What do you think about adding both
> >
> > I did not review the contents of the patch yet, as I was rather unsure
> > about the right semantics here.
> >
> > > ```
> > > trim_file() -> trims $n lines from head or tail of the file and
> > > returns the remaining lines.
> > > read_file_ends() -> returns $n lines from head or tail of the file.
> > > ```
> > >
> > > although trim_file() will not be used in these particular patches?
> >
> > And this made me wonder over the weekend if only showing the head
> > and/or tail of a file is always the best set of properties.
> > Then I came up that this could be something close to what git-grep
> > -A/-B is able to do. For example, for a crash, it would be much
> > cheaper to target a log entry that matches with what we see in the
> > usual crash stacks, then print the surroundings. The "trim" behavior
> > you have proposed is a subset of that, where the matching patterns are
> > the end and/or the beginning of the file to show.
> >
> > So the API could com down to a pattern matching, with "head" and
> > "tail" being the optional number of lines we'd want to print around
> > the pattern we are looking for.
>
> That makes sense and could be really useful in more general cases but
> I think that discussion might be better suited for a separate thread.
>
> For this specific case, we are dealing with regression.diffs; which
> already contains the relevant diff output. We don't need to search for
> patterns here; we just want to include a small portion of the file in
> the error message, to spare users from having to open the file
> manually.

I wanted to show what is in my mind, v4 is attached. Summary is:

- 0001 introduces the read_file_ends() function, which reads lines
from either the beginning or end of a given file. It includes a
force_line_count argument that, when set to true, ensures that the
specified number of lines is read from the file regardless of whether
the PG_TEST_FILE_READ_LINES environment variable is set.

- 0002 is the actual patch that improves error reporting in the
027_stream_regress test by using the read_file_ends() function. It
adds a regression_log_helper() function, which reads the
PG_TEST_FILE_READ_LINES environment variable and then calls
read_file_ends() with force_line_count set to true. This approach
avoids any potential race condition where the environment variable
might be modified after being read in the regression_log_helper() and
before used in the read_file_ends().

--
Regards,
Nazir Bilal Yavuz
Microsoft