From 485929e626e479079b408501f7253ac4daedd3fc Mon Sep 17 00:00:00 2001 From: Allon Murienik Date: Sun, 6 Oct 2019 15:16:08 +0300 Subject: [PATCH] CSV-252: Clean up exception handling (#50) * CSV-252: Clean up assertions using assertThrows As a followup to commit e2f0a4d8a83a41eaa984086636a3712c682307ea that introduced JUnit Jupiter to the project, this patch leverages the new Assertions#assertThrows method to clean up tests for expected exceptions. Instead of the somewhat clunky structure common in JUnit 4 tests: ``` try { someMethod(); fail("SomeException should be thrown"); } catch (SomeException e) { // Expected... // Possibly some assertion on e } ``` JUnit Jupiter allows the following elegant syntax: ``` SomeException e = assertThrows(SomeException.class, () -> someMethod()); // Possibly some assertions on e ``` * CSV-252: Remove redundant throws clauses from tests --- .../apache/commons/csv/AssertionsTest.java | 2 +- .../org/apache/commons/csv/CSVBenchmark.java | 2 +- .../org/apache/commons/csv/CSVFormatTest.java | 43 ++++++++----------- .../org/apache/commons/csv/CSVParserTest.java | 22 ++-------- 4 files changed, 24 insertions(+), 45 deletions(-) diff --git a/src/test/java/org/apache/commons/csv/AssertionsTest.java b/src/test/java/org/apache/commons/csv/AssertionsTest.java index 9d956a29..7d90d9b7 100644 --- a/src/test/java/org/apache/commons/csv/AssertionsTest.java +++ b/src/test/java/org/apache/commons/csv/AssertionsTest.java @@ -26,7 +26,7 @@ import org.junit.jupiter.api.Test; public class AssertionsTest { @Test - public void testNotNull() throws Exception { + public void testNotNull() { Assertions.notNull(new Object(), "object"); } diff --git a/src/test/java/org/apache/commons/csv/CSVBenchmark.java b/src/test/java/org/apache/commons/csv/CSVBenchmark.java index b2ecd528..d055e697 100644 --- a/src/test/java/org/apache/commons/csv/CSVBenchmark.java +++ b/src/test/java/org/apache/commons/csv/CSVBenchmark.java @@ -67,7 +67,7 @@ public class CSVBenchmark { in.close(); } - private BufferedReader getReader() throws IOException { + private BufferedReader getReader() { return new BufferedReader(new StringReader(data)); } diff --git a/src/test/java/org/apache/commons/csv/CSVFormatTest.java b/src/test/java/org/apache/commons/csv/CSVFormatTest.java index 1c95fd66..01478672 100644 --- a/src/test/java/org/apache/commons/csv/CSVFormatTest.java +++ b/src/test/java/org/apache/commons/csv/CSVFormatTest.java @@ -550,17 +550,12 @@ public class CSVFormatTest { final CSVFormat csvFormat = CSVFormat.MYSQL; - try { - csvFormat.format((Object[]) null); - fail("Expecting exception: NullPointerException"); - } catch(final NullPointerException e) { - assertEquals(CSVFormat.class.getName(), e.getStackTrace()[0].getClassName()); - } - + NullPointerException e = assertThrows(NullPointerException.class, () -> csvFormat.format((Object[]) null)); + assertEquals(CSVFormat.class.getName(), e.getStackTrace()[0].getClassName()); } @Test - public void testGetHeader() throws Exception { + public void testGetHeader() { final String[] header = new String[]{"one", "two", "three"}; final CSVFormat formatWithHeader = CSVFormat.DEFAULT.withHeader(header); // getHeader() makes a copy of the header array. @@ -893,7 +888,7 @@ public class CSVFormatTest { } @Test - public void testWithCommentStart() throws Exception { + public void testWithCommentStart() { final CSVFormat formatWithCommentStart = CSVFormat.DEFAULT.withCommentMarker('#'); assertEquals( Character.valueOf('#'), formatWithCommentStart.getCommentMarker()); } @@ -904,7 +899,7 @@ public class CSVFormatTest { } @Test - public void testWithDelimiter() throws Exception { + public void testWithDelimiter() { final CSVFormat formatWithDelimiter = CSVFormat.DEFAULT.withDelimiter('!'); assertEquals('!', formatWithDelimiter.getDelimiter()); } @@ -915,13 +910,13 @@ public class CSVFormatTest { } @Test - public void testWithEmptyEnum() throws Exception { + public void testWithEmptyEnum() { final CSVFormat formatWithHeader = CSVFormat.DEFAULT.withHeader(EmptyEnum.class); assertTrue(formatWithHeader.getHeader().length == 0); } @Test - public void testWithEscape() throws Exception { + public void testWithEscape() { final CSVFormat formatWithEscape = CSVFormat.DEFAULT.withEscape('&'); assertEquals(Character.valueOf('&'), formatWithEscape.getEscapeCharacter()); } @@ -932,14 +927,14 @@ public class CSVFormatTest { } @Test - public void testWithFirstRecordAsHeader() throws Exception { + public void testWithFirstRecordAsHeader() { final CSVFormat formatWithFirstRecordAsHeader = CSVFormat.DEFAULT.withFirstRecordAsHeader(); assertTrue(formatWithFirstRecordAsHeader.getSkipHeaderRecord()); assertTrue(formatWithFirstRecordAsHeader.getHeader().length == 0); } @Test - public void testWithHeader() throws Exception { + public void testWithHeader() { final String[] header = new String[]{"one", "two", "three"}; // withHeader() makes a copy of the header array. final CSVFormat formatWithHeader = CSVFormat.DEFAULT.withHeader(header); @@ -1111,35 +1106,35 @@ public class CSVFormatTest { @Test - public void testWithHeaderEnum() throws Exception { + public void testWithHeaderEnum() { final CSVFormat formatWithHeader = CSVFormat.DEFAULT.withHeader(Header.class); assertArrayEquals(new String[]{ "Name", "Email", "Phone" }, formatWithHeader.getHeader()); } @Test - public void testWithIgnoreEmptyLines() throws Exception { + public void testWithIgnoreEmptyLines() { assertFalse(CSVFormat.DEFAULT.withIgnoreEmptyLines(false).getIgnoreEmptyLines()); assertTrue(CSVFormat.DEFAULT.withIgnoreEmptyLines().getIgnoreEmptyLines()); } @Test - public void testWithIgnoreSurround() throws Exception { + public void testWithIgnoreSurround() { assertFalse(CSVFormat.DEFAULT.withIgnoreSurroundingSpaces(false).getIgnoreSurroundingSpaces()); assertTrue(CSVFormat.DEFAULT.withIgnoreSurroundingSpaces().getIgnoreSurroundingSpaces()); } @Test - public void testWithNullString() throws Exception { + public void testWithNullString() { final CSVFormat formatWithNullString = CSVFormat.DEFAULT.withNullString("null"); assertEquals("null", formatWithNullString.getNullString()); } @Test - public void testWithQuoteChar() throws Exception { + public void testWithQuoteChar() { final CSVFormat formatWithQuoteChar = CSVFormat.DEFAULT.withQuote('"'); assertEquals(Character.valueOf('"'), formatWithQuoteChar.getQuoteCharacter()); } @@ -1151,31 +1146,31 @@ public class CSVFormatTest { } @Test - public void testWithQuotePolicy() throws Exception { + public void testWithQuotePolicy() { final CSVFormat formatWithQuotePolicy = CSVFormat.DEFAULT.withQuoteMode(QuoteMode.ALL); assertEquals(QuoteMode.ALL, formatWithQuotePolicy.getQuoteMode()); } @Test - public void testWithRecordSeparatorCR() throws Exception { + public void testWithRecordSeparatorCR() { final CSVFormat formatWithRecordSeparator = CSVFormat.DEFAULT.withRecordSeparator(CR); assertEquals(String.valueOf(CR), formatWithRecordSeparator.getRecordSeparator()); } @Test - public void testWithRecordSeparatorCRLF() throws Exception { + public void testWithRecordSeparatorCRLF() { final CSVFormat formatWithRecordSeparator = CSVFormat.DEFAULT.withRecordSeparator(CRLF); assertEquals(CRLF, formatWithRecordSeparator.getRecordSeparator()); } @Test - public void testWithRecordSeparatorLF() throws Exception { + public void testWithRecordSeparatorLF() { final CSVFormat formatWithRecordSeparator = CSVFormat.DEFAULT.withRecordSeparator(LF); assertEquals(String.valueOf(LF), formatWithRecordSeparator.getRecordSeparator()); } @Test - public void testWithSystemRecordSeparator() throws Exception { + public void testWithSystemRecordSeparator() { final CSVFormat formatWithRecordSeparator = CSVFormat.DEFAULT.withSystemRecordSeparator(); assertEquals(System.getProperty("line.separator"), formatWithRecordSeparator.getRecordSeparator()); } diff --git a/src/test/java/org/apache/commons/csv/CSVParserTest.java b/src/test/java/org/apache/commons/csv/CSVParserTest.java index d23a8707..eeb35261 100644 --- a/src/test/java/org/apache/commons/csv/CSVParserTest.java +++ b/src/test/java/org/apache/commons/csv/CSVParserTest.java @@ -27,7 +27,6 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; import java.io.File; import java.io.IOException; @@ -529,12 +528,7 @@ public class CSVParserTest { CSVFormat.DEFAULT.withHeader("A", "B", "C"))) { final List headerNames = parser.getHeaderNames(); assertNotNull(headerNames); - try { - headerNames.add("This is a read-only list."); - fail(); - } catch (final UnsupportedOperationException e) { - // Yes. - } + assertThrows(UnsupportedOperationException.class, () -> headerNames.add("This is a read-only list.")); } } @@ -758,12 +752,7 @@ public class CSVParserTest { final Iterator iterator = CSVFormat.DEFAULT.parse(in).iterator(); assertTrue(iterator.hasNext()); - try { - iterator.remove(); - fail("expected UnsupportedOperationException"); - } catch (final UnsupportedOperationException expected) { - // expected - } + assertThrows(UnsupportedOperationException.class, iterator::remove); assertArrayEquals(new String[] { "a", "b", "c" }, iterator.next().values()); assertArrayEquals(new String[] { "1", "2", "3" }, iterator.next().values()); assertTrue(iterator.hasNext()); @@ -772,12 +761,7 @@ public class CSVParserTest { assertArrayEquals(new String[] { "x", "y", "z" }, iterator.next().values()); assertFalse(iterator.hasNext()); - try { - iterator.next(); - fail("NoSuchElementException expected"); - } catch (final NoSuchElementException e) { - // expected - } + assertThrows(NoSuchElementException.class, iterator::next); } @Test