RFR: 8276994: java/nio/channels/Channels/TransferTo.java leaves multi-GB files in /tmp

[Lance Andersen]

On Sat, 13 Nov 2021 21:56:36 GMT, Markus KARG <duke at openjdk.java.net> wrote:

> This PR proposes to cleanup *all* temp files of the `TransferTo` test, not just the 2GB files. Also it separates the actual test logic from the cleanup logic, so the test cases are simple to understand again as they are not interwoven with cleanup code anymore.

thank you for cleaning up the additional temp files created by this test.  Please see comments below.

test/jdk/java/nio/channels/Channels/TransferTo.java line 78:

> 76: 
> 77:     private static Collection<Path> tempFiles;
> 78: 

Is there a reason you chose a LinkedList over a List here?  As mentioned below, you could initialize  tempFiles here.

Please change Collection -> LinkedList or if you use a List to ArrayList()

test/jdk/java/nio/channels/Channels/TransferTo.java line 147:

> 145:     @Test
> 146:     public void testMoreThanTwoGB() throws IOException {
> 147:         // preparing two temporary files which will be compared at the end of the test

I would prefer to keep the previous changes to this test.  We went through a few iterations off line.  We would also prefer to delete these files at the end of the test, not just the over all test run due to their size.

test/jdk/java/nio/channels/Channels/TransferTo.java line 285:

> 283:     @BeforeClass
> 284:     public static void createTempFileRegistry() {
> 285:         tempFiles = new LinkedList();

This could be initialized as part of the declaration.

test/jdk/java/nio/channels/Channels/TransferTo.java line 303:

> 301:      * Creates a temporary file and registers it for later cleanup after tests.
> 302:      */
> 303:     private static Path newTempFile() throws IOException {

You might consider passing in the prefix to make it easier to differentiate the temp file (InputStreamProvider vs OutputSteamProvider...)

-------------

Changes requested by lancea (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6379