Merge c77742a into 3a8f0b7

se-edu · Feb 24, 2019 · d4f9fee · d4f9fee
2 parents 3a8f0b7 + c77742a
commit d4f9fee
Show file tree

Hide file tree

Showing 88 changed files with 668 additions and 842 deletions.
diff --git a/build.gradle b/build.gradle
@@ -42,15 +42,15 @@ test {
 }
 
 dependencies {
-    String testFxVersion = '4.0.12-alpha'
-    String jUnitVersion = '5.1.0'
+    String testFxVersion = '4.0.15-alpha'
+    String jUnitVersion = '5.4.0'
 
     implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: '2.7.0'
     implementation group: 'com.fasterxml.jackson.datatype', name: 'jackson-datatype-jsr310', version: '2.7.4'
 
     testImplementation group: 'junit', name: 'junit', version: '4.12'
     testImplementation group: 'org.testfx', name: 'testfx-core', version: testFxVersion
-    testImplementation group: 'org.testfx', name: 'testfx-junit', version: testFxVersion
+    testImplementation group: 'org.testfx', name: 'testfx-junit5', version: testFxVersion
 
     testImplementation group: 'org.junit.jupiter', name: 'junit-jupiter-api', version: jUnitVersion
 

diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml
@@ -229,6 +229,7 @@
     <module name="VisibilityModifier">
       <property name="protectedAllowed" value="true"/>
       <property name="allowPublicFinalFields" value="true"/>
+      <property name="ignoreAnnotationCanonicalNames" value="Rule, ClassRule, VisibleForTesting, TempDir, RegisterExtension"/>
     </module>
 
     <!--
@@ -390,7 +391,7 @@
         Single line of code methods are often due to refactor for readability.
       -->
       <property name="minLineCount" value="1"/>
-      <property name="allowedAnnotations" value="Override, Test, Before, BeforeClass, After, AfterClass, Subscribe"/>
+      <property name="allowedAnnotations" value="Override, Test, Before, BeforeAll, BeforeClass, BeforeEach, After, AfterAll, AfterClass, AfterEach, Subscribe"/>
       <property name="scope" value="public"/>
       <property name="allowUndeclaredRTE" value="true"/>
       <property name="allowThrowsTagsForSubclasses" value="true"/>
@@ -407,7 +408,7 @@
         We define methods that have more than 3 lines of code as non-trivial.
       -->
       <property name="minLineCount" value="3"/>
-      <property name="allowedAnnotations" value="Override, Test, Before, BeforeClass, After, AfterClass, Subscribe"/>
+      <property name="allowedAnnotations" value="Override, Test, Before, BeforeAll, BeforeClass, BeforeEach, After, AfterAll, AfterClass, AfterEach, Subscribe"/>
       <property name="scope" value="private"/>
       <property name="allowUndeclaredRTE" value="true"/>
       <property name="allowThrowsTagsForSubclasses" value="true"/>

diff --git a/src/test/java/guitests/guihandles/StageHandle.java b/src/test/java/guitests/guihandles/StageHandle.java
@@ -1,7 +1,7 @@
 package guitests.guihandles;
 
 import static java.util.Objects.requireNonNull;
-import static org.junit.Assert.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 
 import java.util.Optional;
 import java.util.logging.Logger;

diff --git a/src/test/java/seedu/address/AppParametersTest.java b/src/test/java/seedu/address/AppParametersTest.java
@@ -1,14 +1,14 @@
 package seedu.address;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import java.nio.file.Paths;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 import javafx.application.Application;
 

diff --git a/src/test/java/seedu/address/TestApp.java b/src/test/java/seedu/address/TestApp.java
@@ -16,7 +16,6 @@
 import seedu.address.model.UserPrefs;
 import seedu.address.storage.JsonAddressBookStorage;
 import seedu.address.storage.UserPrefsStorage;
-import seedu.address.testutil.TestUtil;
 import systemtests.ModelHelper;
 
 /**
@@ -25,20 +24,15 @@
  */
 public class TestApp extends MainApp {
 
-    public static final Path SAVE_LOCATION_FOR_TESTING = TestUtil.getFilePathInSandboxFolder("sampleData.json");
-
-    protected static final Path DEFAULT_PREF_FILE_LOCATION_FOR_TESTING =
-            TestUtil.getFilePathInSandboxFolder("pref_testing.json");
     protected Supplier<ReadOnlyAddressBook> initialDataSupplier = () -> null;
-    protected Path saveFileLocation = SAVE_LOCATION_FOR_TESTING;
-
-    public TestApp() {
-    }
+    protected Path saveFileLocation = null;
+    protected Path configFileLocation = null;
 
-    public TestApp(Supplier<ReadOnlyAddressBook> initialDataSupplier, Path saveFileLocation) {
+    public TestApp(Supplier<ReadOnlyAddressBook> initialDataSupplier, Path saveFileLocation, Path configFileLocation) {
         super();
         this.initialDataSupplier = initialDataSupplier;
         this.saveFileLocation = saveFileLocation;
+        this.configFileLocation = configFileLocation;
 
         // If some initial local data has been provided, write those to the file
         if (initialDataSupplier.get() != null) {
@@ -54,7 +48,7 @@ public TestApp(Supplier<ReadOnlyAddressBook> initialDataSupplier, Path saveFileL
     @Override
     protected Config initConfig(Path configFilePath) {
         Config config = super.initConfig(configFilePath);
-        config.setUserPrefsFilePath(DEFAULT_PREF_FILE_LOCATION_FOR_TESTING);
+        config.setUserPrefsFilePath(configFileLocation);
         return config;
     }
 

diff --git a/src/test/java/seedu/address/commons/core/ConfigTest.java b/src/test/java/seedu/address/commons/core/ConfigTest.java
@@ -1,16 +1,12 @@
 package seedu.address.commons.core;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
+import org.junit.jupiter.api.Test;
 
 public class ConfigTest {
-    @Rule
-    public ExpectedException thrown = ExpectedException.none();
 
     @Test
     public void toString_defaultObject_stringReturned() {

diff --git a/src/test/java/seedu/address/commons/core/VersionTest.java b/src/test/java/seedu/address/commons/core/VersionTest.java
@@ -1,15 +1,12 @@
 package seedu.address.commons.core;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static seedu.address.testutil.Assert.assertThrows;
 
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
+import org.junit.jupiter.api.Test;
 
 public class VersionTest {
-    @Rule
-    public ExpectedException thrown = ExpectedException.none();
 
     @Test
     public void versionParsing_acceptableVersionString_parsedVersionCorrectly() {
@@ -20,8 +17,9 @@ public void versionParsing_acceptableVersionString_parsedVersionCorrectly() {
 
     @Test
     public void versionParsing_wrongVersionString_throwIllegalArgumentException() {
-        thrown.expect(IllegalArgumentException.class);
-        Version.fromString("This is not a version string");
+        assertThrows(IllegalArgumentException.class, () ->
+            Version.fromString("This is not a version string")
+        );
     }
 
     @Test
@@ -31,7 +29,7 @@ public void versionConstructor_correctParameter_valueAsExpected() {
         assertEquals(19, version.getMajor());
         assertEquals(10, version.getMinor());
         assertEquals(20, version.getPatch());
-        assertEquals(true, version.isEarlyAccess());
+        assertTrue(version.isEarlyAccess());
     }
 
     @Test

diff --git a/src/test/java/seedu/address/commons/core/index/IndexTest.java b/src/test/java/seedu/address/commons/core/index/IndexTest.java
@@ -1,17 +1,20 @@
 package seedu.address.commons.core.index;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static seedu.address.testutil.Assert.assertThrows;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 public class IndexTest {
 
     @Test
     public void createOneBasedIndex() {
         // invalid index
-        assertCreateOneBasedFailure(0);
+        assertThrows(IndexOutOfBoundsException.class, () ->
+                Index.fromOneBased(0)
+        );
 
         // check equality using the same base
         assertEquals(1, Index.fromOneBased(1).getOneBased());
@@ -25,7 +28,9 @@ public void createOneBasedIndex() {
     @Test
     public void createZeroBasedIndex() {
         // invalid index
-        assertCreateZeroBasedFailure(-1);
+        assertThrows(IndexOutOfBoundsException.class, () ->
+                Index.fromZeroBased(-1)
+        );
 
         // check equality using the same base
         assertEquals(0, Index.fromZeroBased(0).getZeroBased());
@@ -36,39 +41,6 @@ public void createZeroBasedIndex() {
         assertEquals(6, Index.fromZeroBased(5).getOneBased());
     }
 
-    /**
-     * Executes {@code Index#fromZeroBased(int)} with {@code invalidZeroBasedIndex}, confirms that an
-     * {@code IndexOutOfBoundsException} is thrown.
-     */
-    private void assertCreateZeroBasedFailure(int invalidZeroBasedIndex) {
-        assertCreateFailure(invalidZeroBasedIndex, true);
-    }
-
-    /**
-     * Executes {@code Index#fromOneBased(int)} with {@code invalidOneBasedIndex}, confirms that an
-     * {@code IndexOutOfBoundsException} is thrown.
-     */
-    private void assertCreateOneBasedFailure(int invalidOneBasedIndex) {
-        assertCreateFailure(invalidOneBasedIndex, false);
-    }
-
-    /**
-     * Executes either {@code Index#fromZeroBased(int)} (if it is zero based), or {@code Index#fromOneBased(int)}
-     * (if it is one based), and confirms that an {@code IndexOutOfBoundsException} is thrown.
-     */
-    private void assertCreateFailure(int invalidIndex, boolean isZeroBased) {
-        try {
-            if (isZeroBased) {
-                Index.fromZeroBased(invalidIndex);
-            } else {
-                Index.fromOneBased(invalidIndex);
-            }
-            throw new AssertionError("The expected IndexOutOfBoundsException was not thrown.");
-        } catch (IndexOutOfBoundsException ie) {
-            // expected behaviour
-        }
-    }
-
     @Test
     public void equals() {
         final Index fifthPersonIndex = Index.fromOneBased(5);

diff --git a/src/test/java/seedu/address/commons/util/AppUtilTest.java b/src/test/java/seedu/address/commons/util/AppUtilTest.java
@@ -1,28 +1,20 @@
 package seedu.address.commons.util;
 
-import static org.junit.Assert.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static seedu.address.testutil.Assert.assertThrows;
 
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
+import org.junit.jupiter.api.Test;
 
 public class AppUtilTest {
 
-    @Rule
-    public ExpectedException thrown = ExpectedException.none();
-
-
-
     @Test
     public void getImage_exitingImage() {
         assertNotNull(AppUtil.getImage("/images/address_book_32.png"));
     }
 
-
     @Test
     public void getImage_nullGiven_throwsNullPointerException() {
-        thrown.expect(NullPointerException.class);
-        AppUtil.getImage(null);
+        assertThrows(NullPointerException.class, () -> AppUtil.getImage(null));
     }
 
     @Test
@@ -33,15 +25,12 @@ public void checkArgument_true_nothingHappens() {
 
     @Test
     public void checkArgument_falseWithoutErrorMessage_throwsIllegalArgumentException() {
-        thrown.expect(IllegalArgumentException.class);
-        AppUtil.checkArgument(false);
+        assertThrows(IllegalArgumentException.class, () -> AppUtil.checkArgument(false));
     }
 
     @Test
     public void checkArgument_falseWithErrorMessage_throwsIllegalArgumentException() {
         String errorMessage = "error message";
-        thrown.expect(IllegalArgumentException.class);
-        thrown.expectMessage(errorMessage);
-        AppUtil.checkArgument(false, errorMessage);
+        assertThrows(IllegalArgumentException.class, errorMessage, () -> AppUtil.checkArgument(false, errorMessage));
     }
 }
diff --git a/src/test/java/seedu/address/commons/util/CollectionUtilTest.java b/src/test/java/seedu/address/commons/util/CollectionUtilTest.java
@@ -1,15 +1,16 @@
 package seedu.address.commons.util;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static seedu.address.commons.util.CollectionUtil.requireAllNonNull;
+import static seedu.address.testutil.Assert.assertThrows;
 
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 public class CollectionUtilTest {
     @Test
@@ -86,25 +87,15 @@ public void isAnyNonNull() {
      * if {@code objects} or any element of {@code objects} is null.
      */
     private void assertNullPointerExceptionThrown(Object... objects) {
-        try {
-            requireAllNonNull(objects);
-            throw new AssertionError("The expected NullPointerException was not thrown.");
-        } catch (NullPointerException npe) {
-            // expected behavior
-        }
+        assertThrows(NullPointerException.class, () -> requireAllNonNull(objects));
     }
 
     /**
      * Asserts that {@code CollectionUtil#requireAllNonNull(Collection<?>)} throw {@code NullPointerException}
      * if {@code collection} or any element of {@code collection} is null.
      */
     private void assertNullPointerExceptionThrown(Collection<?> collection) {
-        try {
-            requireAllNonNull(collection);
-            throw new AssertionError("The expected NullPointerException was not thrown.");
-        } catch (NullPointerException npe) {
-            // expected behavior
-        }
+        assertThrows(NullPointerException.class, () -> requireAllNonNull(collection));
     }
 
     private void assertNullPointerExceptionNotThrown(Object... objects) {