IO-628: Migration to JUnit Jupiter (#97)

* Use fail instead of throwing an AssertionFailedError

Use JUnit's Assert#fail in order to fail tests with the proper JUnit
API, instead of explicitly throwing the underlying exception.

This change makes the tests easier to read and maintain, and will help
facilitate the migration to JUnit Jupiter.

* Remove outdate javadoc references to TestCase#setUp()

In JUnit 3, in order to run some code before each test, you would
extend junit.framework.TestCase and override its setUp() method.

In JUnit 4 this is no longer the case, and you simply need to annotate
the method with org.junit.Before, as is done in the classes in
question.

This patch removes old javadoc comments that reference the TestCase
method which are no longer relevant, and are probably outdated remains
from the days the project used JUnit 3.

* Standardize org.junit.Assert imports

The de-facto standard for using org.junit.Assert methods in the
project is to statically import its method.
However, some places use the class directly and call methods on it.
This patch standardizes the approach, and makes sure all the usages of
org.junit.Assert statically import methods in order to have a
consistent coding standard throughout the project's tests.

* Statically import org.junit.Assume methods

Statically import methods from org.junit.Assume in order to make its
usages consistent with org.junit.Assert's usage pattern.

* Migrate test suite to JUnit Jupiter

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 patch does not presume to use JUnit Jupiter's best
practices and all its new functionality, but only to migrate the
existing test 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.junit-pioneer:junit-pioneer was introduced (see details
    below in section 2.g.).

2. Annotations:
 a. org.junit.jupiter.api.Test was used as a drop in replacement for
    org.juit.Test without arguments. See 3.b. and 3.c. for handling
    of @test annotations with "expected" and "timeout" arguments
    respectively.
 b. org.junit.jupiter.params.ParameterizedTest in conjunction with
    org.junit.jupiter.params.provider.MethodSource was used to
    replace org.juit.Test in test classes run with
    org.junit.runners.Parameterized.
 c. org.junit.runners.Parameterized.Parameters annotations were
    removed, and the methods annotated with them were written to
    match the signature expected of a @MethodSource
 d. org.junit.jupiter.api.BeforeEach was used as an drop in
    replacement for org.junit.Before.
 e. org.junit.jupiter.api.AfterEach was used as a drop in replacement
    for org.junit.After.
 f. org.junit.jupiter.api.Disabled was used as a drop in replacement
    for org.junit.Ignore.
 g. org.junitpioneer.jupiter.DefaultLocale was used as a drop in
    replacement for org.apache.commons.io.testtools.SystemDefaults.
    This annotation comes from the JUnit Pioneer project, and while
    it isn't part of the actual JUnit project, it's widely accepted
    as an extension library for JUnit, and is used in other Apache
    Commons projects, such as Apache Commons Lang.

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 side
    bonus of making 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.
    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.junit.jupiter.api.Assertions#assertTimeout was used to assert
    a specific block of code completes before a timeout is reach
    instead of an org.junit.Test annotation with a "timeout"
    argument.

4. Assumptions
 a. org.junit.jupiter.api.Assumptions' methods were used as drop in
    replacements for org.junit.Assume's methods with the same name,
    but the argument order was changed - Assume's methods take the
    message as the first argument, while Assumptions' methods take
    the message as the last argument.

5. Rules
 a. org.junit.jupiter.api.io.TempDir was used instead of the
    org.junit.rules.TemporaryFolder Rule, and the code that used them
    was adapted accordingly.
 b. org.apache.commons.io.testtools.SystemDefaultsSwitch was removed
    from the project, and its functionality replaced by
    org.junitpioneer.jupiter.DefaultLocale.

6. Specific changes:
 a. Parameters in JUnit Jupiter (such as those produced by
    MethodSource) can only be applied to @test methods, not to other
    lifecycle methods, such as @beforeeach. Thus, the design of
    ReversedLinesFileReaderTestParamFile had to be changed - instead
    of receiving constructor arguments and storing a state in data
    members, it was rewritten to be stateless, and manage all its
    resources in the single test method it has,
    testDataIntegrityWithBufferedReader. As a side bonus, this change
    allows to narrow the scope of open resources, and close them
    immediately after they are no longer needed, instead of
    preserving them in data members until the @after method is
    called.
 b. JUnit Jupiter's @tempdir is stricter than JUnit 4's
    @TemporaryFolder, and if it cannot delete any file at the end of
    the test, it will fail the test. FileUtilsCleanDirectoryTestCase's
    methods testThrowsOnNullList and testThrowsOnCannotDeleteFile
    chmod the temporary directory and thus prevent files in it from
    being deleted at the end of the test. A finally block was added to
    these tests in order to chmod the temporary directory back to 755
    and allow it to be deleted. In addition, the restriction of not
    running the test on Windows was changed to use JUnit Jupiter's
    @DisabledOnOs annotation, in order to make the test a tad easier
    to follow.
 c. TailerTest has a similar issue to 6.b., where threads opened
    during the test are only closed in the @AfterEach method (by
    calling Tailer#stop), which is too late, since the @tempdir's
    tear down is called (and fails) before hand. Using a shared temp
    dir (by declaring the member static) causes the directory to be
    deleted only when the entire class is finished.

* Attempt to disable JaCoCo for JDK EA

This is done by passing an environment variable in the .travis.yml
file, and then using it to set the jacoco.skip property in the
pom.xml.

Author: mureinik

.travis.yml

@@ -23,6 +23,14 @@ jdk:
   - openjdk-ea
   - oraclejdk11
 
+matrix:
+  exclude:
+    - jdk: openjdk-ea
+  include:
+    - jdk: openjdk-ea
+      env: JACOCO_SKIP=true
+
+
 script:
   - mvn
 

pom.xml

@@ -227,9 +227,15 @@ file comparators, endian transformation classes, and much more.
 
   <dependencies>
     <dependency>
-      <groupId>junit</groupId>
-      <artifactId>junit</artifactId>
-      <version>4.12</version>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter</artifactId>
+      <version>5.5.2</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.junit-pioneer</groupId>
+      <artifactId>junit-pioneer</artifactId>
+      <version>0.3.0</version>
       <scope>test</scope>
     </dependency>
     <dependency>
@@ -285,6 +291,7 @@ file comparators, endian transformation classes, and much more.
     <commons.surefire.version>2.22.2</commons.surefire.version>
     <commons.japicmp.version>0.14.1</commons.japicmp.version>
     <japicmp.skip>false</japicmp.skip>
+    <jacoco.skip>${env.JACOCO_SKIP}</jacoco.skip>
   </properties>
 
   <build>

src/test/java/org/apache/commons/io/ByteOrderMarkTestCase.java

@@ -17,16 +17,16 @@
 package org.apache.commons.io;
 
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 import java.nio.charset.Charset;
 import java.util.Arrays;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 
 /**
@@ -42,9 +42,9 @@ public class ByteOrderMarkTestCase  {
     /** Test {@link ByteOrderMark#getCharsetName()} */
     @Test
     public void charsetName() {
-        assertEquals("test1 name", "test1", TEST_BOM_1.getCharsetName());
-        assertEquals("test2 name", "test2", TEST_BOM_2.getCharsetName());
-        assertEquals("test3 name", "test3", TEST_BOM_3.getCharsetName());
+        assertEquals("test1", TEST_BOM_1.getCharsetName(), "test1 name");
+        assertEquals("test2", TEST_BOM_2.getCharsetName(), "test2 name");
+        assertEquals("test3", TEST_BOM_3.getCharsetName(), "test3 name");
     }
 
     /** Tests that {@link ByteOrderMark#getCharsetName()} can be loaded as a {@link java.nio.charset.Charset} as advertised. */
@@ -59,52 +59,53 @@ public void constantCharsetNames() {
 
     /** Test {@link ByteOrderMark#length()} */
     @Test
-    public void testLength() {        assertEquals("test1 length", 1, TEST_BOM_1.length());
-        assertEquals("test2 length", 2, TEST_BOM_2.length());
-        assertEquals("test3 length", 3, TEST_BOM_3.length());
+    public void testLength() {
+        assertEquals(1, TEST_BOM_1.length(), "test1 length");
+        assertEquals(2, TEST_BOM_2.length(), "test2 length");
+        assertEquals(3, TEST_BOM_3.length(), "test3 length");
     }
 
     /** Test {@link ByteOrderMark#get(int)} */
     @Test
     public void get() {
-        assertEquals("test1 get(0)", 1, TEST_BOM_1.get(0));
-        assertEquals("test2 get(0)", 1, TEST_BOM_2.get(0));
-        assertEquals("test2 get(1)", 2, TEST_BOM_2.get(1));
-        assertEquals("test3 get(0)", 1, TEST_BOM_3.get(0));
-        assertEquals("test3 get(1)", 2, TEST_BOM_3.get(1));
-        assertEquals("test3 get(2)", 3, TEST_BOM_3.get(2));
+        assertEquals(1, TEST_BOM_1.get(0), "test1 get(0)");
+        assertEquals(1, TEST_BOM_2.get(0), "test2 get(0)");
+        assertEquals(2, TEST_BOM_2.get(1), "test2 get(1)");
+        assertEquals(1, TEST_BOM_3.get(0), "test3 get(0)");
+        assertEquals(2, TEST_BOM_3.get(1), "test3 get(1)");
+        assertEquals(3, TEST_BOM_3.get(2), "test3 get(2)");
     }
 
     /** Test {@link ByteOrderMark#getBytes()} */
     @Test
     public void getBytes() {
-        assertTrue("test1 bytes", Arrays.equals(TEST_BOM_1.getBytes(), new byte[] {(byte)1}));
-        assertTrue("test1 bytes", Arrays.equals(TEST_BOM_2.getBytes(), new byte[] {(byte)1, (byte)2}));
-        assertTrue("test1 bytes", Arrays.equals(TEST_BOM_3.getBytes(), new byte[] {(byte)1, (byte)2, (byte)3}));
+        assertTrue(Arrays.equals(TEST_BOM_1.getBytes(), new byte[] {(byte)1}), "test1 bytes");
+        assertTrue(Arrays.equals(TEST_BOM_2.getBytes(), new byte[] {(byte)1, (byte)2}), "test1 bytes");
+        assertTrue(Arrays.equals(TEST_BOM_3.getBytes(), new byte[] {(byte)1, (byte)2, (byte)3}), "test1 bytes");
     }
 
     /** Test {@link ByteOrderMark#equals(Object)} */
     @SuppressWarnings("EqualsWithItself")
     @Test
     public void testEquals() {
-        assertTrue("test1 equals", TEST_BOM_1.equals(TEST_BOM_1));
-        assertTrue("test2 equals", TEST_BOM_2.equals(TEST_BOM_2));
-        assertTrue("test3 equals", TEST_BOM_3.equals(TEST_BOM_3));
-
-        assertFalse("Object not equal",  TEST_BOM_1.equals(new Object()));
-        assertFalse("test1-1 not equal", TEST_BOM_1.equals(new ByteOrderMark("1a", 2)));
-        assertFalse("test1-2 not test2", TEST_BOM_1.equals(new ByteOrderMark("1b", 1, 2)));
-        assertFalse("test2 not equal", TEST_BOM_2.equals(new ByteOrderMark("2", 1, 1)));
-        assertFalse("test3 not equal", TEST_BOM_3.equals(new ByteOrderMark("3", 1, 2, 4)));
+        assertTrue(TEST_BOM_1.equals(TEST_BOM_1), "test1 equals");
+        assertTrue(TEST_BOM_2.equals(TEST_BOM_2), "test2 equals");
+        assertTrue(TEST_BOM_3.equals(TEST_BOM_3), "test3 equals");
+
+        assertFalse(TEST_BOM_1.equals(new Object()), "Object not equal");
+        assertFalse(TEST_BOM_1.equals(new ByteOrderMark("1a", 2)), "test1-1 not equal");
+        assertFalse(TEST_BOM_1.equals(new ByteOrderMark("1b", 1, 2)), "test1-2 not test2");
+        assertFalse(TEST_BOM_2.equals(new ByteOrderMark("2", 1, 1)), "test2 not equal");
+        assertFalse(TEST_BOM_3.equals(new ByteOrderMark("3", 1, 2, 4)), "test3 not equal");
     }
 
     /** Test {@link ByteOrderMark#hashCode()} */
     @Test
     public void testHashCode() {
         final int bomClassHash = ByteOrderMark.class.hashCode();
-        assertEquals("hash test1 ", bomClassHash + 1,  TEST_BOM_1.hashCode());
-        assertEquals("hash test2 ", bomClassHash + 3,  TEST_BOM_2.hashCode());
-        assertEquals("hash test3 ", bomClassHash + 6,  TEST_BOM_3.hashCode());
+        assertEquals(bomClassHash + 1, TEST_BOM_1.hashCode(), "hash test1 ");
+        assertEquals(bomClassHash + 3, TEST_BOM_2.hashCode(), "hash test2 ");
+        assertEquals(bomClassHash + 6, TEST_BOM_3.hashCode(), "hash test3 ");
     }
 
     /** Test Errors */
@@ -139,8 +140,8 @@ public void errors() {
     /** Test {@link ByteOrderMark#toString()} */
     @Test
     public void testToString() {
-        assertEquals("test1 ", "ByteOrderMark[test1: 0x1]",          TEST_BOM_1.toString());
-        assertEquals("test2 ", "ByteOrderMark[test2: 0x1,0x2]",      TEST_BOM_2.toString());
-        assertEquals("test3 ", "ByteOrderMark[test3: 0x1,0x2,0x3]",  TEST_BOM_3.toString());
+        assertEquals("ByteOrderMark[test1: 0x1]",          TEST_BOM_1.toString(), "test1 ");
+        assertEquals("ByteOrderMark[test2: 0x1,0x2]",      TEST_BOM_2.toString(), "test2 ");
+        assertEquals("ByteOrderMark[test3: 0x1,0x2,0x3]",  TEST_BOM_3.toString(), "test3 ");
     }
 }

src/test/java/org/apache/commons/io/ByteOrderParserTest.java

@@ -16,10 +16,12 @@
  */
 package org.apache.commons.io;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
 import java.nio.ByteOrder;
 
-import org.junit.Assert;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 public class ByteOrderParserTest {
 
@@ -29,16 +31,16 @@ private ByteOrder parseByteOrder(final String value) {
 
     @Test
     public void testParseBig() {
-        Assert.assertEquals(ByteOrder.BIG_ENDIAN, parseByteOrder("BIG_ENDIAN"));
+        assertEquals(ByteOrder.BIG_ENDIAN, parseByteOrder("BIG_ENDIAN"));
     }
 
     @Test
     public void testParseLittle() {
-        Assert.assertEquals(ByteOrder.LITTLE_ENDIAN, parseByteOrder("LITTLE_ENDIAN"));
+        assertEquals(ByteOrder.LITTLE_ENDIAN, parseByteOrder("LITTLE_ENDIAN"));
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void testThrowsException() throws Exception {
-        parseByteOrder("some value");
+    @Test
+    public void testThrowsException() {
+        assertThrows(IllegalArgumentException.class, () -> parseByteOrder("some value"));
     }
 }

src/test/java/org/apache/commons/io/CharsetsTestCase.java

@@ -17,11 +17,12 @@
 
 package org.apache.commons.io;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
 import java.nio.charset.Charset;
 import java.util.SortedMap;
 
-import org.junit.Assert;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 /**
  * Tests {@link Charsets}.
@@ -35,50 +36,50 @@ public void testRequiredCharsets() {
         final SortedMap<String, Charset> requiredCharsets = Charsets.requiredCharsets();
         // test for what we expect to be there as of Java 6
         // Make sure the object at the given key is the right one
-        Assert.assertEquals(requiredCharsets.get("US-ASCII").name(), "US-ASCII");
-        Assert.assertEquals(requiredCharsets.get("ISO-8859-1").name(), "ISO-8859-1");
-        Assert.assertEquals(requiredCharsets.get("UTF-8").name(), "UTF-8");
-        Assert.assertEquals(requiredCharsets.get("UTF-16").name(), "UTF-16");
-        Assert.assertEquals(requiredCharsets.get("UTF-16BE").name(), "UTF-16BE");
-        Assert.assertEquals(requiredCharsets.get("UTF-16LE").name(), "UTF-16LE");
+        assertEquals(requiredCharsets.get("US-ASCII").name(), "US-ASCII");
+        assertEquals(requiredCharsets.get("ISO-8859-1").name(), "ISO-8859-1");
+        assertEquals(requiredCharsets.get("UTF-8").name(), "UTF-8");
+        assertEquals(requiredCharsets.get("UTF-16").name(), "UTF-16");
+        assertEquals(requiredCharsets.get("UTF-16BE").name(), "UTF-16BE");
+        assertEquals(requiredCharsets.get("UTF-16LE").name(), "UTF-16LE");
     }
 
     @Test
     public void testIso8859_1() {
-        Assert.assertEquals("ISO-8859-1", Charsets.ISO_8859_1.name());
+        assertEquals("ISO-8859-1", Charsets.ISO_8859_1.name());
     }
 
     @Test
     public void testToCharset() {
-        Assert.assertEquals(Charset.defaultCharset(), Charsets.toCharset((String) null));
-        Assert.assertEquals(Charset.defaultCharset(), Charsets.toCharset((Charset) null));
-        Assert.assertEquals(Charset.defaultCharset(), Charsets.toCharset(Charset.defaultCharset()));
-        Assert.assertEquals(Charset.forName("UTF-8"), Charsets.toCharset(Charset.forName("UTF-8")));
+        assertEquals(Charset.defaultCharset(), Charsets.toCharset((String) null));
+        assertEquals(Charset.defaultCharset(), Charsets.toCharset((Charset) null));
+        assertEquals(Charset.defaultCharset(), Charsets.toCharset(Charset.defaultCharset()));
+        assertEquals(Charset.forName("UTF-8"), Charsets.toCharset(Charset.forName("UTF-8")));
     }
 
     @Test
     public void testUsAscii() {
-        Assert.assertEquals("US-ASCII", Charsets.US_ASCII.name());
+        assertEquals("US-ASCII", Charsets.US_ASCII.name());
     }
 
     @Test
     public void testUtf16() {
-        Assert.assertEquals("UTF-16", Charsets.UTF_16.name());
+        assertEquals("UTF-16", Charsets.UTF_16.name());
     }
 
     @Test
     public void testUtf16Be() {
-        Assert.assertEquals("UTF-16BE", Charsets.UTF_16BE.name());
+        assertEquals("UTF-16BE", Charsets.UTF_16BE.name());
     }
 
     @Test
     public void testUtf16Le() {
-        Assert.assertEquals("UTF-16LE", Charsets.UTF_16LE.name());
+        assertEquals("UTF-16LE", Charsets.UTF_16LE.name());
     }
 
     @Test
     public void testUtf8() {
-        Assert.assertEquals("UTF-8", Charsets.UTF_8.name());
+        assertEquals("UTF-8", Charsets.UTF_8.name());
     }
 
 }