You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2020/01/25 16:55:35 UTC

[commons-compress] 05/06: Fix some tests that leak resources when failing. There is a lot more to do in this area.

This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-compress.git

commit 11b803476f4776c463f0a62516ed3b688be91260
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sat Jan 25 11:55:05 2020 -0500

    Fix some tests that leak resources when failing. There is a lot more to
    do in this area.
    
    Remove unused imports.
---
 .../archivers/ar/ArArchiveInputStreamTest.java     | 23 ++++----
 .../archivers/arj/ArjArchiveInputStreamTest.java   | 61 ++++++++++-----------
 .../archivers/cpio/CpioArchiveInputStreamTest.java | 51 +++++++++--------
 .../compress/archivers/examples/ExpanderTest.java  |  4 --
 .../compress/archivers/sevenz/SevenZFileTest.java  | 64 +++++++++++-----------
 .../archivers/sevenz/SevenZNativeHeapTest.java     | 29 +++++-----
 .../archivers/sevenz/SevenZOutputFileTest.java     | 14 ++---
 7 files changed, 118 insertions(+), 128 deletions(-)

diff --git a/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java b/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java
index 14c4315..f620a6b 100644
--- a/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java
@@ -97,17 +97,18 @@ public class ArArchiveInputStreamTest extends AbstractTestCase {
                 }
             };
 
-            ArArchiveInputStream archiveInputStream = new ArArchiveInputStream(simpleInputStream);
-            ArArchiveEntry entry1 = archiveInputStream.getNextArEntry();
-            assertThat(entry1, not(nullValue()));
-            assertThat(entry1.getName(), equalTo("test1.xml"));
-            assertThat(entry1.getLength(), equalTo(610L));
-
-            ArArchiveEntry entry2 = archiveInputStream.getNextArEntry();
-            assertThat(entry2.getName(), equalTo("test2.xml"));
-            assertThat(entry2.getLength(), equalTo(82L));
-
-            assertThat(archiveInputStream.getNextArEntry(), nullValue());
+            try (ArArchiveInputStream archiveInputStream = new ArArchiveInputStream(simpleInputStream)) {
+                ArArchiveEntry entry1 = archiveInputStream.getNextArEntry();
+                assertThat(entry1, not(nullValue()));
+                assertThat(entry1.getName(), equalTo("test1.xml"));
+                assertThat(entry1.getLength(), equalTo(610L));
+
+                ArArchiveEntry entry2 = archiveInputStream.getNextArEntry();
+                assertThat(entry2.getName(), equalTo("test2.xml"));
+                assertThat(entry2.getLength(), equalTo(82L));
+
+                assertThat(archiveInputStream.getNextArEntry(), nullValue());
+            }
         }
     }
 
diff --git a/src/test/java/org/apache/commons/compress/archivers/arj/ArjArchiveInputStreamTest.java b/src/test/java/org/apache/commons/compress/archivers/arj/ArjArchiveInputStreamTest.java
index 70a8b1f..6c0950c 100644
--- a/src/test/java/org/apache/commons/compress/archivers/arj/ArjArchiveInputStreamTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/arj/ArjArchiveInputStreamTest.java
@@ -39,49 +39,48 @@ public class ArjArchiveInputStreamTest extends AbstractTestCase {
         expected.append("<empty/>test2.xml<?xml version=\"1.0\"?>\n");
         expected.append("<empty/>\n");
 
-
-        final ArjArchiveInputStream in = new ArjArchiveInputStream(new FileInputStream(getFile("bla.arj")));
-        ArjArchiveEntry entry;
-
         final StringBuilder result = new StringBuilder();
-        while ((entry = in.getNextEntry()) != null) {
-            result.append(entry.getName());
-            int tmp;
-            while ((tmp = in.read()) != -1) {
-                result.append((char) tmp);
+        try (final ArjArchiveInputStream in = new ArjArchiveInputStream(new FileInputStream(getFile("bla.arj")))) {
+            ArjArchiveEntry entry;
+
+            while ((entry = in.getNextEntry()) != null) {
+                result.append(entry.getName());
+                int tmp;
+                while ((tmp = in.read()) != -1) {
+                    result.append((char) tmp);
+                }
+                assertFalse(entry.isDirectory());
             }
-            assertFalse(entry.isDirectory());
         }
-        in.close();
         assertEquals(result.toString(), expected.toString());
     }
 
     @Test
     public void testReadingOfAttributesDosVersion() throws Exception {
-        final ArjArchiveInputStream in = new ArjArchiveInputStream(new FileInputStream(getFile("bla.arj")));
-        final ArjArchiveEntry entry = in.getNextEntry();
-        assertEquals("test1.xml", entry.getName());
-        assertEquals(30, entry.getSize());
-        assertEquals(0, entry.getUnixMode());
-        final Calendar cal = Calendar.getInstance();
-        cal.set(2008, 9, 6, 23, 50, 52);
-        cal.set(Calendar.MILLISECOND, 0);
-        assertEquals(cal.getTime(), entry.getLastModifiedDate());
-        in.close();
+        try (final ArjArchiveInputStream in = new ArjArchiveInputStream(new FileInputStream(getFile("bla.arj")))) {
+            final ArjArchiveEntry entry = in.getNextEntry();
+            assertEquals("test1.xml", entry.getName());
+            assertEquals(30, entry.getSize());
+            assertEquals(0, entry.getUnixMode());
+            final Calendar cal = Calendar.getInstance();
+            cal.set(2008, 9, 6, 23, 50, 52);
+            cal.set(Calendar.MILLISECOND, 0);
+            assertEquals(cal.getTime(), entry.getLastModifiedDate());
+        }
     }
 
     @Test
     public void testReadingOfAttributesUnixVersion() throws Exception {
-        final ArjArchiveInputStream in = new ArjArchiveInputStream(new FileInputStream(getFile("bla.unix.arj")));
-        final ArjArchiveEntry entry = in.getNextEntry();
-        assertEquals("test1.xml", entry.getName());
-        assertEquals(30, entry.getSize());
-        assertEquals(0664, entry.getUnixMode() & 07777 /* UnixStat.PERM_MASK */);
-        final Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("GMT+0000"));
-        cal.set(2008, 9, 6, 21, 50, 52);
-        cal.set(Calendar.MILLISECOND, 0);
-        assertEquals(cal.getTime(), entry.getLastModifiedDate());
-        in.close();
+        try (final ArjArchiveInputStream in = new ArjArchiveInputStream(new FileInputStream(getFile("bla.unix.arj")))) {
+            final ArjArchiveEntry entry = in.getNextEntry();
+            assertEquals("test1.xml", entry.getName());
+            assertEquals(30, entry.getSize());
+            assertEquals(0664, entry.getUnixMode() & 07777 /* UnixStat.PERM_MASK */);
+            final Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("GMT+0000"));
+            cal.set(2008, 9, 6, 21, 50, 52);
+            cal.set(Calendar.MILLISECOND, 0);
+            assertEquals(cal.getTime(), entry.getLastModifiedDate());
+        }
     }
 
     @Test
diff --git a/src/test/java/org/apache/commons/compress/archivers/cpio/CpioArchiveInputStreamTest.java b/src/test/java/org/apache/commons/compress/archivers/cpio/CpioArchiveInputStreamTest.java
index 55a9f75..efb5394 100644
--- a/src/test/java/org/apache/commons/compress/archivers/cpio/CpioArchiveInputStreamTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/cpio/CpioArchiveInputStreamTest.java
@@ -36,50 +36,49 @@ public class CpioArchiveInputStreamTest extends AbstractTestCase {
         expected.append("<empty/>./test2.xml<?xml version=\"1.0\"?>\n");
         expected.append("<empty/>\n");
 
-
-        final CpioArchiveInputStream in = new CpioArchiveInputStream(new FileInputStream(getFile("bla.cpio")));
-        CpioArchiveEntry entry;
-
         final StringBuilder result = new StringBuilder();
-        while ((entry = (CpioArchiveEntry) in.getNextEntry()) != null) {
-            result.append(entry.getName());
-            int tmp;
-            while ((tmp = in.read()) != -1) {
-                result.append((char) tmp);
+        try (final CpioArchiveInputStream in = new CpioArchiveInputStream(new FileInputStream(getFile("bla.cpio")))) {
+            CpioArchiveEntry entry;
+
+            while ((entry = (CpioArchiveEntry) in.getNextEntry()) != null) {
+                result.append(entry.getName());
+                int tmp;
+                while ((tmp = in.read()) != -1) {
+                    result.append((char) tmp);
+                }
             }
         }
-        in.close();
         assertEquals(result.toString(), expected.toString());
     }
 
     @Test
     public void testCpioUnarchiveCreatedByRedlineRpm() throws Exception {
-        final CpioArchiveInputStream in =
-            new CpioArchiveInputStream(new FileInputStream(getFile("redline.cpio")));
-        CpioArchiveEntry entry= null;
-
         int count = 0;
-        while ((entry = (CpioArchiveEntry) in.getNextEntry()) != null) {
-            count++;
-            assertNotNull(entry);
+        try (final CpioArchiveInputStream in = new CpioArchiveInputStream(
+            new FileInputStream(getFile("redline.cpio")))) {
+            CpioArchiveEntry entry = null;
+
+            while ((entry = (CpioArchiveEntry) in.getNextEntry()) != null) {
+                count++;
+                assertNotNull(entry);
+            }
         }
-        in.close();
 
         assertEquals(count, 1);
     }
 
     @Test
     public void testCpioUnarchiveMultibyteCharName() throws Exception {
-        final CpioArchiveInputStream in =
-            new CpioArchiveInputStream(new FileInputStream(getFile("COMPRESS-459.cpio")), "UTF-8");
-        CpioArchiveEntry entry= null;
-
         int count = 0;
-        while ((entry = (CpioArchiveEntry) in.getNextEntry()) != null) {
-            count++;
-            assertNotNull(entry);
+        try (final CpioArchiveInputStream in = new CpioArchiveInputStream(
+            new FileInputStream(getFile("COMPRESS-459.cpio")), "UTF-8")) {
+            CpioArchiveEntry entry = null;
+
+            while ((entry = (CpioArchiveEntry) in.getNextEntry()) != null) {
+                count++;
+                assertNotNull(entry);
+            }
         }
-        in.close();
 
         assertEquals(2, count);
     }
diff --git a/src/test/java/org/apache/commons/compress/archivers/examples/ExpanderTest.java b/src/test/java/org/apache/commons/compress/archivers/examples/ExpanderTest.java
index 15791b6..304777c 100644
--- a/src/test/java/org/apache/commons/compress/archivers/examples/ExpanderTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/examples/ExpanderTest.java
@@ -28,13 +28,9 @@ import java.nio.channels.SeekableByteChannel;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.StandardOpenOption;
-import java.util.Arrays;
-import java.util.Collection;
 
 import org.apache.commons.compress.AbstractTestCase;
-import org.apache.commons.compress.archivers.ArchiveEntry;
 import org.apache.commons.compress.archivers.ArchiveException;
-import org.apache.commons.compress.archivers.ArchiveInputStream;
 import org.apache.commons.compress.archivers.ArchiveOutputStream;
 import org.apache.commons.compress.archivers.ArchiveStreamFactory;
 import org.apache.commons.compress.archivers.StreamingNotSupportedException;
diff --git a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java
index b37184b..3fb99ce 100644
--- a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java
@@ -56,14 +56,14 @@ public class SevenZFileTest extends AbstractTestCase {
     public void testRandomlySkippingEntries() throws Exception {
         // Read sequential reference.
         final Map<String, byte[]> entriesByName = new HashMap<>();
-        SevenZFile archive = new SevenZFile(getFile("COMPRESS-320/Copy.7z"));
-        SevenZArchiveEntry entry;
-        while ((entry = archive.getNextEntry()) != null) {
-            if (entry.hasStream()) {
-                entriesByName.put(entry.getName(), readFully(archive));
+        try (SevenZFile archive = new SevenZFile(getFile("COMPRESS-320/Copy.7z"))) {
+            SevenZArchiveEntry entry;
+            while ((entry = archive.getNextEntry()) != null) {
+                if (entry.hasStream()) {
+                    entriesByName.put(entry.getName(), readFully(archive));
+                }
             }
         }
-        archive.close();
 
         final String[] variants = {
             "BZip2-solid.7z",
@@ -81,26 +81,27 @@ public class SevenZFileTest extends AbstractTestCase {
             // "PPMd.7z",
         };
 
-        // TODO: use randomizedtesting for predictable, but different, randomness.
+        // TODO: use randomized testing for predictable, but different, randomness.
         final Random rnd = new Random(0xdeadbeef);
         for (final String fileName : variants) {
-            archive = new SevenZFile(getFile("COMPRESS-320/" + fileName));
+            try (SevenZFile archive = new SevenZFile(getFile("COMPRESS-320/" + fileName))) {
 
-            while ((entry = archive.getNextEntry()) != null) {
-                // Sometimes skip reading entries.
-                if (rnd.nextBoolean()) {
-                    continue;
-                }
+                SevenZArchiveEntry entry;
+                while ((entry = archive.getNextEntry()) != null) {
+                    // Sometimes skip reading entries.
+                    if (rnd.nextBoolean()) {
+                        continue;
+                    }
 
-                if (entry.hasStream()) {
-                    assertTrue(entriesByName.containsKey(entry.getName()));
-                    final byte [] content = readFully(archive);
-                    assertTrue("Content mismatch on: " + fileName + "!" + entry.getName(),
-                               Arrays.equals(content, entriesByName.get(entry.getName())));
+                    if (entry.hasStream()) {
+                        assertTrue(entriesByName.containsKey(entry.getName()));
+                        final byte[] content = readFully(archive);
+                        assertTrue("Content mismatch on: " + fileName + "!" + entry.getName(),
+                            Arrays.equals(content, entriesByName.get(entry.getName())));
+                    }
                 }
-            }
 
-            archive.close();
+            }
         }
     }
 
@@ -534,23 +535,24 @@ public class SevenZFileTest extends AbstractTestCase {
             sevenZFile.getNextEntry();
             // skip all the entries and jump backwards
             byte[] contents = new byte[(int) testTxtEntry.getSize()];
-            InputStream inputStream = sevenZFile.getInputStream(testTxtEntry);
-            int off = 0;
-            while (off < contents.length) {
-                final int bytesRead = inputStream.read(contents, off, contents.length - off);
-                assert (bytesRead >= 0);
-                off += bytesRead;
+            try (InputStream inputStream = sevenZFile.getInputStream(testTxtEntry)) {
+                int off = 0;
+                while (off < contents.length) {
+                    final int bytesRead = inputStream.read(contents, off, contents.length - off);
+                    assert (bytesRead >= 0);
+                    off += bytesRead;
+                }
+                assertEquals(SevenZMethod.LZMA2, testTxtEntry.getContentMethods().iterator().next().getMethod());
+                assertEquals(testTxtContents, new String(contents, "UTF-8"));
             }
-            assertEquals(SevenZMethod.LZMA2, testTxtEntry.getContentMethods().iterator().next().getMethod());
-            assertEquals(testTxtContents, new String(contents, "UTF-8"));
         }
     }
 
     @Test
     public void extractNonExistSpecifiedFile() throws Exception {
-        try (SevenZFile sevenZFile = new SevenZFile(getFile("COMPRESS-256.7z"))) {
-            SevenZFile anotherSevenZFile = new SevenZFile(getFile("bla.7z"));
-            for(SevenZArchiveEntry nonExistEntry : anotherSevenZFile.getEntries()) {
+        try (SevenZFile sevenZFile = new SevenZFile(getFile("COMPRESS-256.7z"));
+            SevenZFile anotherSevenZFile = new SevenZFile(getFile("bla.7z"))) {
+            for (SevenZArchiveEntry nonExistEntry : anotherSevenZFile.getEntries()) {
                 thrown.expect(IllegalArgumentException.class);
                 sevenZFile.getInputStream(nonExistEntry);
             }
diff --git a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZNativeHeapTest.java b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZNativeHeapTest.java
index 70adec0..04065c0 100644
--- a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZNativeHeapTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZNativeHeapTest.java
@@ -33,29 +33,28 @@ import org.junit.Test;
 
 public class SevenZNativeHeapTest extends AbstractTestCase {
 
-
     @Test
     public void testEndDeflaterOnCloseStream() throws Exception {
-        Coders.DeflateDecoder deflateDecoder = new DeflateDecoder();
-
-        final DeflateDecoderOutputStream outputStream =
-            (DeflateDecoderOutputStream) deflateDecoder.encode(new ByteArrayOutputStream(), 9);
-        DelegatingDeflater delegatingDeflater = new DelegatingDeflater(outputStream.deflater);
-        outputStream.deflater = delegatingDeflater;
-        outputStream.close();
+        final Coders.DeflateDecoder deflateDecoder = new DeflateDecoder();
+        final DelegatingDeflater delegatingDeflater;
+        try (final DeflateDecoderOutputStream outputStream = (DeflateDecoderOutputStream) deflateDecoder
+            .encode(new ByteArrayOutputStream(), 9)) {
+            delegatingDeflater = new DelegatingDeflater(outputStream.deflater);
+            outputStream.deflater = delegatingDeflater;
+        }
         assertTrue(delegatingDeflater.isEnded.get());
 
     }
 
     @Test
     public void testEndInflaterOnCloseStream() throws Exception {
-        Coders.DeflateDecoder deflateDecoder = new DeflateDecoder();
-        final DeflateDecoderInputStream inputStream =
-                (DeflateDecoderInputStream) deflateDecoder
-                        .decode("dummy", new ByteArrayInputStream(new byte[0]), 0, null, null, Integer.MAX_VALUE);
-        DelegatingInflater delegatingInflater = new DelegatingInflater(inputStream.inflater);
-        inputStream.inflater = delegatingInflater;
-        inputStream.close();
+        final Coders.DeflateDecoder deflateDecoder = new DeflateDecoder();
+        final DelegatingInflater delegatingInflater;
+        try (final DeflateDecoderInputStream inputStream = (DeflateDecoderInputStream) deflateDecoder.decode("dummy",
+            new ByteArrayInputStream(new byte[0]), 0, null, null, Integer.MAX_VALUE)) {
+            delegatingInflater = new DelegatingInflater(inputStream.inflater);
+            inputStream.inflater = delegatingInflater;
+        }
 
         assertTrue(delegatingInflater.isEnded.get());
     }
diff --git a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFileTest.java b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFileTest.java
index 7139ec3..63836ae 100644
--- a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFileTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFileTest.java
@@ -479,12 +479,9 @@ public class SevenZOutputFileTest extends AbstractTestCase {
     }
 
     private void createAndReadBack(final File output, final Iterable<SevenZMethodConfiguration> methods) throws Exception {
-        final SevenZOutputFile outArchive = new SevenZOutputFile(output);
-        outArchive.setContentMethods(methods);
-        try {
+        try (final SevenZOutputFile outArchive = new SevenZOutputFile(output)) {
+            outArchive.setContentMethods(methods);
             addFile(outArchive, 0, true);
-        } finally {
-            outArchive.close();
         }
 
         try (SevenZFile archive = new SevenZFile(output)) {
@@ -493,12 +490,9 @@ public class SevenZOutputFileTest extends AbstractTestCase {
     }
 
     private void createAndReadBack(final SeekableInMemoryByteChannel output, final Iterable<SevenZMethodConfiguration> methods) throws Exception {
-        final SevenZOutputFile outArchive = new SevenZOutputFile(output);
-        outArchive.setContentMethods(methods);
-        try {
+        try (final SevenZOutputFile outArchive = new SevenZOutputFile(output)) {
+            outArchive.setContentMethods(methods);
             addFile(outArchive, 0, true);
-        } finally {
-            outArchive.close();
         }
         try (SevenZFile archive =
              new SevenZFile(new SeekableInMemoryByteChannel(output.array()), "in memory")) {