On Thu, 8 Sep 2022 06:32:59 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> The correctness of the actually written data is already checked in >> `checkTransferredContents`. I do not see what you win by replacing >> `ByteArrayInputStream` by `FileInputStream`. What additional risks do you >> cover, and what is so special about `FileInputStream` compared to any other >> `InputStream`? `FileInputStream::transferTo` is already checked by its own >> unit test. > >> Given that the implementation in this PR now calls the wrapped >> `InputStream`'s `transferTo` method, should we also include a test where the >> wrapped `InputStream` is a `FileInputStream` and the target `OutputStream` >> is a `FileOutputStream`, so that it exercises the `FileChannel#transferTo` >> code path and verifies the correct data is transferred? > > Right, the implementation change means that the transferTo implementation of > the wrapped input stream is used, or the base InputStream.transferTo is used. > We should expect there is good test coverage of these implementations already > but checking into that might find opportunities to expand those tests (not > this PR of course). > > For the new test to exercise the BIS.transferTo then I think it will need to > test with a mark set, test when there are buffered bytes (read before > transferTo), or when BIS has been extended. Agreed that testing the non-empty-buffer (read-before-transferTo) and the mark-set cases (mark-before-transferTo), but still I do not see any need to take particularly `FileInputStream` into the boat; files only make the test run slower. ------------- PR: https://git.openjdk.org/jdk/pull/6935
