[PATCH] D99198: [flang] Fix output buffering bug

Tue Mar 23 13:50:49 PDT 2021

klausler added a comment.

In D99198#2645631 <https://reviews.llvm.org/D99198#2645631>, @Leporacanthicus wrote:

> In D99198#2645402 <https://reviews.llvm.org/D99198#2645402>, @klausler wrote:
>
>> In D99198#2645322 <https://reviews.llvm.org/D99198#2645322>, @Leporacanthicus wrote:
>>
>>> I'm wondering if it's possible to construct a test-case for this "shuffle it around" situation, so that some newbie (like me ...) doesn't break it accidentally when making changes?
>>
>> It gets exercised pretty thoroughly by the I/O tests in the FCVS Fortran '77 suite, especially by codes doing direct and sequential I/O, and the flang/unittests/Runtime/external-io.cpp test really shook it out on the read path (and still passes with this change now that it's used on the write path).  But it would be nice to be able to make the code work harder by e.g. using an artificially small buffer size.  I've done that in the past during development (but obviously not for console output, sorry) but I'm not sure how it could be made part of a unit testing framework.
>
> Yes, I am asking without fully understanding the overall infrastructure. A way to test without having to write 64KB+ of data would be nice - but not sure how easy that is to achieve either.
> From what I can tell from a quick look, the buffer functionality itself isn't being tested, it is the IO functionality that is layered on top of it that gets tested - so it wouldn't be very easy to implement a dummy STORE class that tracks the actual output and expects to see certain strings - even if we could make the buffer size smaller.
> Maybe the right thing would be to actually add a small test-suite for the buffer itself, along with some kind of variable setting for the buffer-size?
>
> These are just my thoughts, as a newbie to this project.

That's a fine idea, and I encourage you to take a whack at it.  I am swamped.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99198/new/

https://reviews.llvm.org/D99198