Skip to content

Commit e2f0a4d

Browse files
mureinikgarydgregory
mureinik
authored and
garydgregory
committed
CSV-252: Migration to JUnit Jupiter (#49)
* CSV-252 Stop using junit.framework.TestCase junit.framework.TestCase is a class from JUnit 3, and while it is not officially deprecated, it's discouraged to use it. This patch removes the single use of junit.framework.TestCase#assertNull, and replaces it with the standard, recommended, org.junit.Assert#assertNull. * CSV-252 Standardize org.junit.Assert imports Code in the project uses org.junit.Assert's methods in two ways: 1. By statically importing them 2. By importing the class and using its methods Option 1 seems to be the de-facto standard, with just a handful of cases using Option 2. This patch standardizes these cases to also use static imports thus making the code look more uniform, and easier to maintain. * CSV-252 Upgrade Mockito to 3.1.0 Upgrade the Mockito dependency to the latest available version, 3.1.0, in order to facilitate an upgrade to JUnit Jupiter. * CSV-252 JUnit Jupiter upgrade This patch upgrades the project's testing framework from JUnit 4.12 to the modern JUnit Jupiter 5.5.4. Since JUnit 5 Jupiter is not backwards compatible to JUnit 4.x (or even JUnit Vintage), this patch is a bit large, even though a lot of the changes are merely cosmetic (such as changing the argument order, see details below). In order to make the reviewer's task as easy as possible, this PR does not presume to use JUnit Jupiter's best practices and all its new functionality, but only to migrate the existing tests with as little change as possible. Following patches may want to improve the tests by using some of JUnit Jupiter's new features. This patch includes the following changes: 1. Maven dependency changes: a. junit:junit was replaced with org.junit.jupiter:junit-jupiter. b. org.hamcrest:hamcrest was introduced as an explicit dependency, since the project uses Hamcrest, and JUnit Jupiter does not bundle Hamcrest, unlike JUnit 4.x. 2. Annotations: a. org.junit.jupiter.api.Test was used as a drop in replacement for org.juit.Test without arguments. See 3.ii. for handling of @test annotations with an "expected" argument. b. org.junit.jupiter.api.BeforeEach was used as an drop in replacement for org.junit.Before. c. org.junit.jupiter.api.BeforeAll was used as an drop in replacement for org.junit.BeforeClass. d. org.junit.jupiter.api.Disabled was used as a drop in replacement for org.junit.Ignore. 3. Assertions: a. org.junit.jupiter.api.Assertions' methods were used as drop in replacements for org.junit.Assert's methods with the same name in the simple case of an assertion without a message. In the case of an assertion with a message, org.junit.jupiter.api.Assertions' methods were used, but the argument order was changed - Assert's methods take the message as the first argument, while Assertions' methods take the message as the last argument. b. org.junit.jupiter.api.Assertions#assertThrows was used to assert that a specific exception was throws instead of an org.junit.Test annotation with an expected argument. This technique has a couple of side bonuses. First, it makes the tests slightly stricter, as now they can assert the exception was thrown from a specific line and prevent false positives where the test's "set-up" code accidentally threw that exception. Second, it clarifies that some of the test code is unreachable (as a previous line already throws an exception), and can safely be removed in order to clean up the test. The throws clauses of these methods were cleaned up from exceptions that can no longer be thrown in order to avoid compilation warnings. c. org.hamcrest.MatcherAssert#assertThat was used as a drop in replacement for org.junit.Assert#assertThat. 4. Specific Changes: a. CSVFileParserTest was rewritten with JUnit Jupiter's org.junit.jupiter.api.ParameterizedTest. Unlike JUnit 4's org.junit.runners.Parameterized, it cannot be used to inject arguments to a test's construct, and so the test can't be stateful. Instead, it was rewritten so every test receives the file as a parameter, and opens a reader on it itself. As a side bonus, this design makes it easier to close the reader and avoid leaving open file descriptors like the original test did.
1 parent 6aa1756 commit e2f0a4d

19 files changed

Lines changed: 362 additions & 370 deletions

‎pom.xml‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,21 @@ CSV files of various types.
3333

3434
<dependencies>
3535
<dependency>
36-
<groupId>junit</groupId>
37-
<artifactId>junit</artifactId>
38-
<version>4.12</version>
36+
<groupId>org.junit.jupiter</groupId>
37+
<artifactId>junit-jupiter</artifactId>
38+
<version>5.5.2</version>
39+
<scope>test</scope>
40+
</dependency>
41+
<dependency>
42+
<groupId>org.hamcrest</groupId>
43+
<artifactId>hamcrest</artifactId>
44+
<version>2.1</version>
3945
<scope>test</scope>
4046
</dependency>
4147
<dependency>
4248
<groupId>org.mockito</groupId>
43-
<artifactId>mockito-all</artifactId>
44-
<version>1.10.19</version>
49+
<artifactId>mockito-core</artifactId>
50+
<version>3.1.0</version>
4551
<scope>test</scope>
4652
</dependency>
4753
<dependency>

‎src/test/java/org/apache/commons/csv/AssertionsTest.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717

1818
package org.apache.commons.csv;
1919

20-
import org.junit.Test;
20+
import static org.junit.jupiter.api.Assertions.assertThrows;
21+
22+
import org.junit.jupiter.api.Test;
2123

2224
/**
2325
*/
@@ -28,8 +30,8 @@ public void testNotNull() throws Exception {
2830
Assertions.notNull(new Object(), "object");
2931
}
3032

31-
@Test(expected = IllegalArgumentException.class)
32-
public void testNotNullNull() throws Exception {
33-
Assertions.notNull(null, "object");
33+
@Test
34+
public void testNotNullNull() {
35+
assertThrows(IllegalArgumentException.class, () -> Assertions.notNull(null, "object"));
3436
}
3537
}

‎src/test/java/org/apache/commons/csv/CSVFileParserTest.java‎

Lines changed: 92 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -17,152 +17,138 @@
1717

1818
package org.apache.commons.csv;
1919

20-
import static org.junit.Assert.assertEquals;
21-
import static org.junit.Assert.assertNotNull;
22-
import static org.junit.Assert.assertTrue;
23-
import static org.junit.Assert.fail;
20+
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertNotNull;
22+
import static org.junit.jupiter.api.Assertions.assertTrue;
23+
import static org.junit.jupiter.api.Assertions.fail;
2424

2525
import java.io.BufferedReader;
2626
import java.io.File;
27-
import java.io.FileNotFoundException;
2827
import java.io.FileReader;
2928
import java.io.FilenameFilter;
3029
import java.io.IOException;
3130
import java.net.URL;
3231
import java.nio.charset.Charset;
33-
import java.util.ArrayList;
3432
import java.util.Arrays;
35-
import java.util.Collection;
36-
import java.util.List;
33+
import java.util.stream.Stream;
3734

38-
import org.junit.Test;
39-
import org.junit.runner.RunWith;
40-
import org.junit.runners.Parameterized;
41-
import org.junit.runners.Parameterized.Parameters;
35+
import org.junit.jupiter.params.ParameterizedTest;
36+
import org.junit.jupiter.params.provider.MethodSource;
4237

4338
/**
4439
* Parse tests using test files
4540
*/
46-
@RunWith(Parameterized.class)
4741
public class CSVFileParserTest {
4842

4943
private static final File BASE = new File("src/test/resources/CSVFileParser");
5044

51-
private final BufferedReader testData;
52-
53-
private final String testName;
54-
55-
public CSVFileParserTest(final File file) throws FileNotFoundException {
56-
this.testName = file.getName();
57-
this.testData = new BufferedReader(new FileReader(file));
58-
}
59-
60-
private String readTestData() throws IOException {
45+
private String readTestData(BufferedReader reader) throws IOException {
6146
String line;
6247
do {
63-
line = testData.readLine();
48+
line = reader.readLine();
6449
} while (line != null && line.startsWith("#"));
6550
return line;
6651
}
6752

68-
@Parameters
69-
public static Collection<Object[]> generateData() {
70-
final List<Object[]> list = new ArrayList<>();
71-
53+
public static Stream<File> generateData() {
7254
final FilenameFilter fileNameFilter = (dir, name) -> name.startsWith("test") && name.endsWith(".txt");
7355
final File[] files = BASE.listFiles(fileNameFilter);
7456
if (files != null) {
75-
for (final File f : files) {
76-
list.add(new Object[] { f });
77-
}
57+
return Arrays.stream(files);
7858
}
79-
return list;
59+
return Stream.empty();
8060
}
8161

82-
@Test
83-
public void testCSVFile() throws Exception {
84-
String line = readTestData();
85-
assertNotNull("file must contain config line", line);
86-
final String[] split = line.split(" ");
87-
assertTrue(testName + " require 1 param", split.length >= 1);
88-
// first line starts with csv data file name
89-
CSVFormat format = CSVFormat.newFormat(',').withQuote('"');
90-
boolean checkComments = false;
91-
for (int i = 1; i < split.length; i++) {
92-
final String option = split[i];
93-
final String[] option_parts = option.split("=", 2);
94-
if ("IgnoreEmpty".equalsIgnoreCase(option_parts[0])) {
95-
format = format.withIgnoreEmptyLines(Boolean.parseBoolean(option_parts[1]));
96-
} else if ("IgnoreSpaces".equalsIgnoreCase(option_parts[0])) {
97-
format = format.withIgnoreSurroundingSpaces(Boolean.parseBoolean(option_parts[1]));
98-
} else if ("CommentStart".equalsIgnoreCase(option_parts[0])) {
99-
format = format.withCommentMarker(option_parts[1].charAt(0));
100-
} else if ("CheckComments".equalsIgnoreCase(option_parts[0])) {
101-
checkComments = true;
102-
} else {
103-
fail(testName + " unexpected option: " + option);
62+
@ParameterizedTest
63+
@MethodSource("generateData")
64+
public void testCSVFile(File testFile) throws Exception {
65+
try (FileReader fr = new FileReader(testFile); BufferedReader testData = new BufferedReader(fr)) {
66+
String line = readTestData(testData);
67+
assertNotNull("file must contain config line", line);
68+
final String[] split = line.split(" ");
69+
assertTrue(split.length >= 1, testFile.getName() + " require 1 param");
70+
// first line starts with csv data file name
71+
CSVFormat format = CSVFormat.newFormat(',').withQuote('"');
72+
boolean checkComments = false;
73+
for (int i = 1; i < split.length; i++) {
74+
final String option = split[i];
75+
final String[] option_parts = option.split("=", 2);
76+
if ("IgnoreEmpty".equalsIgnoreCase(option_parts[0])) {
77+
format = format.withIgnoreEmptyLines(Boolean.parseBoolean(option_parts[1]));
78+
} else if ("IgnoreSpaces".equalsIgnoreCase(option_parts[0])) {
79+
format = format.withIgnoreSurroundingSpaces(Boolean.parseBoolean(option_parts[1]));
80+
} else if ("CommentStart".equalsIgnoreCase(option_parts[0])) {
81+
format = format.withCommentMarker(option_parts[1].charAt(0));
82+
} else if ("CheckComments".equalsIgnoreCase(option_parts[0])) {
83+
checkComments = true;
84+
} else {
85+
fail(testFile.getName() + " unexpected option: " + option);
86+
}
10487
}
105-
}
106-
line = readTestData(); // get string version of format
107-
assertEquals(testName + " Expected format ", line, format.toString());
108-
109-
// Now parse the file and compare against the expected results
110-
// We use a buffered reader internally so no need to create one here.
111-
try (final CSVParser parser = CSVParser.parse(new File(BASE, split[0]), Charset.defaultCharset(), format)) {
112-
for (final CSVRecord record : parser) {
113-
String parsed = Arrays.toString(record.values());
114-
if (checkComments) {
115-
final String comment = record.getComment().replace("\n", "\\n");
116-
if (comment != null) {
117-
parsed += "#" + comment;
88+
line = readTestData(testData); // get string version of format
89+
assertEquals(line, format.toString(), testFile.getName() + " Expected format ");
90+
91+
// Now parse the file and compare against the expected results
92+
// We use a buffered reader internally so no need to create one here.
93+
try (final CSVParser parser = CSVParser.parse(new File(BASE, split[0]), Charset.defaultCharset(), format)) {
94+
for (final CSVRecord record : parser) {
95+
String parsed = Arrays.toString(record.values());
96+
if (checkComments) {
97+
final String comment = record.getComment().replace("\n", "\\n");
98+
if (comment != null) {
99+
parsed += "#" + comment;
100+
}
118101
}
102+
final int count = record.size();
103+
assertEquals(readTestData(testData), count + ":" + parsed, testFile.getName());
119104
}
120-
final int count = record.size();
121-
assertEquals(testName, readTestData(), count + ":" + parsed);
122105
}
123106
}
124107
}
125108

126-
@Test
127-
public void testCSVUrl() throws Exception {
128-
String line = readTestData();
129-
assertNotNull("file must contain config line", line);
130-
final String[] split = line.split(" ");
131-
assertTrue(testName + " require 1 param", split.length >= 1);
132-
// first line starts with csv data file name
133-
CSVFormat format = CSVFormat.newFormat(',').withQuote('"');
134-
boolean checkComments = false;
135-
for (int i = 1; i < split.length; i++) {
136-
final String option = split[i];
137-
final String[] option_parts = option.split("=", 2);
138-
if ("IgnoreEmpty".equalsIgnoreCase(option_parts[0])) {
139-
format = format.withIgnoreEmptyLines(Boolean.parseBoolean(option_parts[1]));
140-
} else if ("IgnoreSpaces".equalsIgnoreCase(option_parts[0])) {
141-
format = format.withIgnoreSurroundingSpaces(Boolean.parseBoolean(option_parts[1]));
142-
} else if ("CommentStart".equalsIgnoreCase(option_parts[0])) {
143-
format = format.withCommentMarker(option_parts[1].charAt(0));
144-
} else if ("CheckComments".equalsIgnoreCase(option_parts[0])) {
145-
checkComments = true;
146-
} else {
147-
fail(testName + " unexpected option: " + option);
109+
@ParameterizedTest
110+
@MethodSource("generateData")
111+
public void testCSVUrl(File testFile) throws Exception {
112+
try (FileReader fr = new FileReader(testFile); BufferedReader testData = new BufferedReader(fr)) {
113+
String line = readTestData(testData);
114+
assertNotNull("file must contain config line", line);
115+
final String[] split = line.split(" ");
116+
assertTrue(split.length >= 1, testFile.getName() + " require 1 param");
117+
// first line starts with csv data file name
118+
CSVFormat format = CSVFormat.newFormat(',').withQuote('"');
119+
boolean checkComments = false;
120+
for (int i = 1; i < split.length; i++) {
121+
final String option = split[i];
122+
final String[] option_parts = option.split("=", 2);
123+
if ("IgnoreEmpty".equalsIgnoreCase(option_parts[0])) {
124+
format = format.withIgnoreEmptyLines(Boolean.parseBoolean(option_parts[1]));
125+
} else if ("IgnoreSpaces".equalsIgnoreCase(option_parts[0])) {
126+
format = format.withIgnoreSurroundingSpaces(Boolean.parseBoolean(option_parts[1]));
127+
} else if ("CommentStart".equalsIgnoreCase(option_parts[0])) {
128+
format = format.withCommentMarker(option_parts[1].charAt(0));
129+
} else if ("CheckComments".equalsIgnoreCase(option_parts[0])) {
130+
checkComments = true;
131+
} else {
132+
fail(testFile.getName() + " unexpected option: " + option);
133+
}
148134
}
149-
}
150-
line = readTestData(); // get string version of format
151-
assertEquals(testName + " Expected format ", line, format.toString());
152-
153-
// Now parse the file and compare against the expected results
154-
final URL resource = ClassLoader.getSystemResource("CSVFileParser/" + split[0]);
155-
try (final CSVParser parser = CSVParser.parse(resource, Charset.forName("UTF-8"), format)) {
156-
for (final CSVRecord record : parser) {
157-
String parsed = Arrays.toString(record.values());
158-
if (checkComments) {
159-
final String comment = record.getComment().replace("\n", "\\n");
160-
if (comment != null) {
161-
parsed += "#" + comment;
135+
line = readTestData(testData); // get string version of format
136+
assertEquals(line, format.toString(), testFile.getName() + " Expected format ");
137+
138+
// Now parse the file and compare against the expected results
139+
final URL resource = ClassLoader.getSystemResource("CSVFileParser/" + split[0]);
140+
try (final CSVParser parser = CSVParser.parse(resource, Charset.forName("UTF-8"), format)) {
141+
for (final CSVRecord record : parser) {
142+
String parsed = Arrays.toString(record.values());
143+
if (checkComments) {
144+
final String comment = record.getComment().replace("\n", "\\n");
145+
if (comment != null) {
146+
parsed += "#" + comment;
147+
}
162148
}
149+
final int count = record.size();
150+
assertEquals(readTestData(testData), count + ":" + parsed, testFile.getName());
163151
}
164-
final int count = record.size();
165-
assertEquals(testName, readTestData(), count + ":" + parsed);
166152
}
167153
}
168154
}

‎src/test/java/org/apache/commons/csv/CSVFormatPredefinedTest.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,18 @@
1717

1818
package org.apache.commons.csv;
1919

20-
import org.junit.Assert;
21-
import org.junit.Test;
20+
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
22+
import org.junit.jupiter.api.Test;
2223

2324
/**
2425
* Tests {@link CSVFormat.Predefined}.
2526
*/
2627
public class CSVFormatPredefinedTest {
2728

2829
private void test(final CSVFormat format, final String enumName) {
29-
Assert.assertEquals(format, CSVFormat.Predefined.valueOf(enumName).getFormat());
30-
Assert.assertEquals(format, CSVFormat.valueOf(enumName));
30+
assertEquals(format, CSVFormat.Predefined.valueOf(enumName).getFormat());
31+
assertEquals(format, CSVFormat.valueOf(enumName));
3132
}
3233

3334
@Test

0 commit comments

Comments
 (0)