Skip to content

Commit

Permalink
Assert: Use JUnit 5 assertions
Browse files Browse the repository at this point in the history
Currently, Assert implements our custom assertThrows using try/catch.

This implementation was done when we used JUnit 4 which has no proper
support for assertThrows. As we migrate to JUnit 5[1], we can make use
of JUnit 5's Assertions.assertThrows. However, our current
assertThrows supports checking the correctness of error's message,
whcih JUnit 5 does not support.

Let's
* migrate Assert#assertThrows to use JUnit 5's assertThrows.
* Remove VoidCallable interface within Assert
* create an overloaded assertThrows(Class<? extends Throwable>,
String, Executable) to support checking the correctness of error
messages.
* standardise to use only seedu.address.testutil.Assert#assertThrows
instead of org.junit.jupiter.api.Assertions.assertThrows

By creating an overloaded assertThrows method, we can extract the
common routine of checking error message into a single utility
function within Assert.java. This encourages reusability and follows
the DRY principle.
In addition, by using org.junit.jupiter.api.function.Executable that
already accepts lambda expressions, we no longer need the VoidCallable
interface previously. Hence, we can remove the unused VoidCallable
interface.

[1] se-edu#951
  • Loading branch information
sijie123 committed Feb 24, 2019
1 parent 3a8f0b7 commit 34ee8be
Show file tree
Hide file tree
Showing 26 changed files with 195 additions and 368 deletions.
4 changes: 0 additions & 4 deletions src/test/java/seedu/address/commons/core/ConfigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,9 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

public class ConfigTest {
@Rule
public ExpectedException thrown = ExpectedException.none();

@Test
public void toString_defaultObject_stringReturned() {
Expand Down
14 changes: 6 additions & 8 deletions src/test/java/seedu/address/commons/core/VersionTest.java
Original file line number Diff line number Diff line change
@@ -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;

public class VersionTest {
@Rule
public ExpectedException thrown = ExpectedException.none();

@Test
public void versionParsing_acceptableVersionString_parsedVersionCorrectly() {
Expand All @@ -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
Expand Down
21 changes: 5 additions & 16 deletions src/test/java/seedu/address/commons/util/AppUtilTest.java
Original file line number Diff line number Diff line change
@@ -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;

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
Expand All @@ -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));
}
}
15 changes: 3 additions & 12 deletions src/test/java/seedu/address/commons/util/CollectionUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static seedu.address.commons.util.CollectionUtil.requireAllNonNull;
import static seedu.address.testutil.Assert.assertThrows;

import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -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) {
Expand Down
33 changes: 11 additions & 22 deletions src/test/java/seedu/address/commons/util/ConfigUtilTest.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package seedu.address.commons.util;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static seedu.address.testutil.Assert.assertThrows;

import java.io.IOException;
import java.nio.file.Path;
Expand All @@ -11,7 +12,6 @@

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.rules.TemporaryFolder;

import seedu.address.commons.core.Config;
Expand All @@ -21,16 +21,13 @@ public class ConfigUtilTest {

private static final Path TEST_DATA_FOLDER = Paths.get("src", "test", "data", "ConfigUtilTest");

@Rule
public ExpectedException thrown = ExpectedException.none();

@Rule
public TemporaryFolder testFolder = new TemporaryFolder();

@Test
public void read_null_throwsNullPointerException() throws DataConversionException {
thrown.expect(NullPointerException.class);
read(null);
public void read_null_throwsNullPointerException() {
assertThrows(NullPointerException.class, () -> read(null));
}

@Test
Expand All @@ -39,14 +36,8 @@ public void read_missingFile_emptyResult() throws DataConversionException {
}

@Test
public void read_notJsonFormat_exceptionThrown() throws DataConversionException {

thrown.expect(DataConversionException.class);
read("NotJsonFormatConfig.json");

/* IMPORTANT: Any code below an exception-throwing line (like the one above) will be ignored.
* That means you should not have more than one exception test in one method
*/
public void read_notJsonFormat_exceptionThrown() {
assertThrows(DataConversionException.class, () -> read("NotJsonFormatConfig.json"));
}

@Test
Expand Down Expand Up @@ -85,15 +76,13 @@ private Optional<Config> read(String configFileInTestDataFolder) throws DataConv
}

@Test
public void save_nullConfig_throwsNullPointerException() throws IOException {
thrown.expect(NullPointerException.class);
save(null, "SomeFile.json");
public void save_nullConfig_throwsNullPointerException() {
assertThrows(NullPointerException.class, () -> save(null, "SomeFile.json"));
}

@Test
public void save_nullFile_throwsNullPointerException() throws IOException {
thrown.expect(NullPointerException.class);
save(new Config(), null);
public void save_nullFile_throwsNullPointerException() {
assertThrows(NullPointerException.class, () -> save(new Config(), null));
}

@Test
Expand Down
9 changes: 4 additions & 5 deletions src/test/java/seedu/address/commons/util/FileUtilTest.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
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.testutil.Assert.assertThrows;

import org.junit.Test;

import seedu.address.testutil.Assert;

public class FileUtilTest {

@Test
Expand All @@ -18,7 +17,7 @@ public void isValidPath() {
assertFalse(FileUtil.isValidPath("a\0"));

// null path -> throws NullPointerException
Assert.assertThrows(NullPointerException.class, () -> FileUtil.isValidPath(null));
assertThrows(NullPointerException.class, () -> FileUtil.isValidPath(null));
}

}
41 changes: 13 additions & 28 deletions src/test/java/seedu/address/commons/util/StringUtilTest.java
Original file line number Diff line number Diff line change
@@ -1,22 +1,15 @@
package seedu.address.commons.util;

import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
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.testutil.Assert.assertThrows;

import java.io.FileNotFoundException;
import java.util.Optional;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

public class StringUtilTest {

@Rule
public ExpectedException thrown = ExpectedException.none();

//---------------- Tests for isUnsignedPositiveInteger --------------------------------------

@Test
Expand Down Expand Up @@ -63,31 +56,25 @@ public void isUnsignedPositiveInteger() {

@Test
public void containsWordIgnoreCase_nullWord_throwsNullPointerException() {
assertExceptionThrown(NullPointerException.class, "typical sentence", null, Optional.empty());
}

private void assertExceptionThrown(Class<? extends Throwable> exceptionClass, String sentence, String word,
Optional<String> errorMessage) {
thrown.expect(exceptionClass);
errorMessage.ifPresent(message -> thrown.expectMessage(message));
StringUtil.containsWordIgnoreCase(sentence, word);
assertThrows(NullPointerException.class, ()
-> StringUtil.containsWordIgnoreCase("typical sentence", null));
}

@Test
public void containsWordIgnoreCase_emptyWord_throwsIllegalArgumentException() {
assertExceptionThrown(IllegalArgumentException.class, "typical sentence", " ",
Optional.of("Word parameter cannot be empty"));
assertThrows(IllegalArgumentException.class, "Word parameter cannot be empty", ()
-> StringUtil.containsWordIgnoreCase("typical scenario", " "));
}

@Test
public void containsWordIgnoreCase_multipleWords_throwsIllegalArgumentException() {
assertExceptionThrown(IllegalArgumentException.class, "typical sentence", "aaa BBB",
Optional.of("Word parameter should be a single word"));
assertThrows(IllegalArgumentException.class, "Word parameter should be a single word", ()
-> StringUtil.containsWordIgnoreCase("typical scenario", "aaa BBB"));
}

@Test
public void containsWordIgnoreCase_nullSentence_throwsNullPointerException() {
assertExceptionThrown(NullPointerException.class, null, "abc", Optional.empty());
assertThrows(NullPointerException.class, () -> StringUtil.containsWordIgnoreCase(null, "abc"));
}

/*
Expand Down Expand Up @@ -145,15 +132,13 @@ public void containsWordIgnoreCase_validInputs_correctResult() {

@Test
public void getDetails_exceptionGiven() {
assertThat(StringUtil.getDetails(new FileNotFoundException("file not found")),
containsString("java.io.FileNotFoundException: file not found"));
assertTrue(StringUtil.getDetails(new FileNotFoundException("file not found"))
.contains("java.io.FileNotFoundException: file not found"));
}

@Test
public void getDetails_nullGiven_throwsNullPointerException() {
thrown.expect(NullPointerException.class);
StringUtil.getDetails(null);
assertThrows(NullPointerException.class, () -> StringUtil.getDetails(null));
}


}

0 comments on commit 34ee8be

Please sign in to comment.