On Thu, 8 Sep 2022 06:39:58 GMT, Markus KARG <d...@openjdk.org> wrote:
>>> 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. I have just added testing `transferTo` with non-empty buffer and mark set as part of the already existing *randomized* test steps. I think that should be sufficient to detect problems if they really would exist, so there should not be a real need to run *explicit and separate* test cases for the combinations of "buffered with mark" / "buffered without mark" / "unbuffered with mark" / "unbuffered without mark" as that would not provide any improved coverage or detection rate IMHO. ------------- PR: https://git.openjdk.org/jdk/pull/6935
