[#559] Clean up GUI tests related classes (#617)

GUI tests related classes are plagued with many issues. For example, GuiRobot's access modifiers are wrong, and AddressBookGuiTest is doing more than it ought to do (a violation of Single Responsibility Principle). Let's fix them.
se-edu · Aug 7, 2017 · a60d16b · a60d16b
2 parents e38f155 + bfa08bd
commit a60d16b
Show file tree

Hide file tree

Showing 16 changed files with 86 additions and 69 deletions.
diff --git a/src/test/java/guitests/AddCommandTest.java b/src/test/java/guitests/AddCommandTest.java
@@ -6,6 +6,7 @@
 import static seedu.address.testutil.TypicalPersons.getTypicalPersons;
 import static seedu.address.ui.testutil.GuiTestAssert.assertCardDisplaysPerson;
 import static seedu.address.ui.testutil.GuiTestAssert.assertListMatching;
+import static seedu.address.ui.testutil.GuiTestAssert.assertResultMessage;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -37,7 +38,7 @@ public void add() throws Exception {
 
         //add duplicate person
         runCommand(PersonUtil.getAddCommand(HOON));
-        assertResultMessage(AddCommand.MESSAGE_DUPLICATE_PERSON);
+        assertResultMessage(getResultDisplay(), AddCommand.MESSAGE_DUPLICATE_PERSON);
         assertListMatching(getPersonListPanel(), expectedList.toArray(new Person[expectedList.size()]));
 
         //add to empty list
@@ -49,7 +50,7 @@ public void add() throws Exception {
 
         //invalid command
         runCommand("adds Johnny");
-        assertResultMessage(Messages.MESSAGE_UNKNOWN_COMMAND);
+        assertResultMessage(getResultDisplay(), Messages.MESSAGE_UNKNOWN_COMMAND);
     }
 
     private void assertAddSuccess(ReadOnlyPerson personToAdd, ArrayList<ReadOnlyPerson> expectedList) throws Exception {

diff --git a/src/test/java/guitests/AddressBookGuiTest.java b/src/test/java/guitests/AddressBookGuiTest.java
@@ -1,7 +1,5 @@
 package guitests;
 
-import static org.junit.Assert.assertEquals;
-
 import java.util.concurrent.TimeoutException;
 
 import org.junit.After;
@@ -18,11 +16,9 @@
 import guitests.guihandles.PersonListPanelHandle;
 import guitests.guihandles.ResultDisplayHandle;
 import guitests.guihandles.StatusBarFooterHandle;
-import javafx.application.Platform;
 import javafx.stage.Stage;
 import seedu.address.TestApp;
 import seedu.address.commons.core.EventsCenter;
-import seedu.address.commons.events.BaseEvent;
 import seedu.address.model.AddressBook;
 import seedu.address.testutil.TypicalPersons;
 
@@ -118,23 +114,4 @@ public void cleanup() throws Exception {
         FxToolkit.cleanupStages();
     }
 
-    /**
-     * Asserts the size of the person list is equal to the given number.
-     */
-    protected void assertListSize(int size) {
-        int numberOfPeople = getPersonListPanel().getListSize();
-        assertEquals(size, numberOfPeople);
-    }
-
-    /**
-     * Asserts the message shown in the Result Display area is same as the given string.
-     */
-    protected void assertResultMessage(String expected) {
-        assertEquals(expected, getResultDisplay().getText());
-    }
-
-    protected void raise(BaseEvent event) {
-        //JUnit doesn't run its test cases on the UI thread. Platform.runLater is used to post event on the UI thread.
-        Platform.runLater(() -> EventsCenter.getInstance().post(event));
-    }
 }
diff --git a/src/test/java/guitests/ClearCommandTest.java b/src/test/java/guitests/ClearCommandTest.java
@@ -3,6 +3,8 @@
 import static seedu.address.testutil.TypicalPersons.HOON;
 import static seedu.address.testutil.TypicalPersons.getTypicalPersons;
 import static seedu.address.ui.testutil.GuiTestAssert.assertListMatching;
+import static seedu.address.ui.testutil.GuiTestAssert.assertListSize;
+import static seedu.address.ui.testutil.GuiTestAssert.assertResultMessage;
 
 import org.junit.Test;
 
@@ -23,15 +25,15 @@ public void clear() throws Exception {
         runCommand(PersonUtil.getAddCommand(HOON));
         assertListMatching(getPersonListPanel(), HOON);
         runCommand(DeleteCommand.COMMAND_WORD + " 1");
-        assertListSize(0);
+        assertListSize(getPersonListPanel(), 0);
 
         //verify clear command works when the list is empty
         assertClearCommandSuccess();
     }
 
     private void assertClearCommandSuccess() {
         runCommand(ClearCommand.COMMAND_WORD);
-        assertListSize(0);
-        assertResultMessage("Address book has been cleared!");
+        assertListSize(getPersonListPanel(), 0);
+        assertResultMessage(getResultDisplay(), "Address book has been cleared!");
     }
 }
diff --git a/src/test/java/guitests/EditCommandTest.java b/src/test/java/guitests/EditCommandTest.java
@@ -12,6 +12,7 @@
 import static seedu.address.testutil.TypicalPersons.getTypicalPersons;
 import static seedu.address.ui.testutil.GuiTestAssert.assertCardDisplaysPerson;
 import static seedu.address.ui.testutil.GuiTestAssert.assertListMatching;
+import static seedu.address.ui.testutil.GuiTestAssert.assertResultMessage;
 
 import org.junit.Test;
 
@@ -85,37 +86,38 @@ public void edit_findThenEdit_success() throws Exception {
     @Test
     public void edit_missingPersonIndex_failure() {
         runCommand(EditCommand.COMMAND_WORD + " " + PREFIX_NAME + "Bobby");
-        assertResultMessage(String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditCommand.MESSAGE_USAGE));
+        assertResultMessage(getResultDisplay(),
+                String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditCommand.MESSAGE_USAGE));
     }
 
     @Test
     public void edit_invalidPersonIndex_failure() {
         runCommand(EditCommand.COMMAND_WORD + " 8 " + PREFIX_NAME + "Bobby");
-        assertResultMessage(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
+        assertResultMessage(getResultDisplay(), Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
     }
 
     @Test
     public void edit_noFieldsSpecified_failure() {
         runCommand(EditCommand.COMMAND_WORD + " 1");
-        assertResultMessage(EditCommand.MESSAGE_NOT_EDITED);
+        assertResultMessage(getResultDisplay(), EditCommand.MESSAGE_NOT_EDITED);
     }
 
     @Test
     public void edit_invalidValues_failure() {
         runCommand(EditCommand.COMMAND_WORD + " 1 " + PREFIX_NAME + "*&");
-        assertResultMessage(Name.MESSAGE_NAME_CONSTRAINTS);
+        assertResultMessage(getResultDisplay(), Name.MESSAGE_NAME_CONSTRAINTS);
 
         runCommand(EditCommand.COMMAND_WORD + " 1 " + PREFIX_PHONE + "abcd");
-        assertResultMessage(Phone.MESSAGE_PHONE_CONSTRAINTS);
+        assertResultMessage(getResultDisplay(), Phone.MESSAGE_PHONE_CONSTRAINTS);
 
         runCommand(EditCommand.COMMAND_WORD + " 1 " + PREFIX_EMAIL + "yahoo!!!");
-        assertResultMessage(Email.MESSAGE_EMAIL_CONSTRAINTS);
+        assertResultMessage(getResultDisplay(), Email.MESSAGE_EMAIL_CONSTRAINTS);
 
         runCommand(EditCommand.COMMAND_WORD + " 1 " + PREFIX_ADDRESS.getPrefix());
-        assertResultMessage(Address.MESSAGE_ADDRESS_CONSTRAINTS);
+        assertResultMessage(getResultDisplay(), Address.MESSAGE_ADDRESS_CONSTRAINTS);
 
         runCommand(EditCommand.COMMAND_WORD + " 1 " + PREFIX_TAG + "*&");
-        assertResultMessage(Tag.MESSAGE_TAG_CONSTRAINTS);
+        assertResultMessage(getResultDisplay(), Tag.MESSAGE_TAG_CONSTRAINTS);
     }
 
     @Test
@@ -126,7 +128,7 @@ public void edit_duplicatePerson_failure() {
                 + PREFIX_NAME + "Alice Pauline "
                 + PREFIX_ADDRESS + "123, Jurong West Ave 6, #08-111 "
                 + PREFIX_TAG + "friends");
-        assertResultMessage(EditCommand.MESSAGE_DUPLICATE_PERSON);
+        assertResultMessage(getResultDisplay(), EditCommand.MESSAGE_DUPLICATE_PERSON);
     }
 
     /**
@@ -152,6 +154,6 @@ private void assertEditSuccess(Index filteredPersonListIndex, Index addressBookI
         ReadOnlyPerson[] expectedPersons = getTypicalPersons();
         expectedPersons[addressBookIndex.getZeroBased()] = editedPerson;
         assertListMatching(getPersonListPanel(), expectedPersons);
-        assertResultMessage(String.format(EditCommand.MESSAGE_EDIT_PERSON_SUCCESS, editedPerson));
+        assertResultMessage(getResultDisplay(), String.format(EditCommand.MESSAGE_EDIT_PERSON_SUCCESS, editedPerson));
     }
 }
diff --git a/src/test/java/guitests/ErrorDialogGuiTest.java b/src/test/java/guitests/ErrorDialogGuiTest.java
@@ -1,6 +1,7 @@
 package guitests;
 
 import static org.junit.Assert.assertEquals;
+import static seedu.address.testutil.EventsUtil.postLater;
 import static seedu.address.ui.UiManager.FILE_OPS_ERROR_DIALOG_CONTENT_MESSAGE;
 import static seedu.address.ui.UiManager.FILE_OPS_ERROR_DIALOG_HEADER_MESSAGE;
 import static seedu.address.ui.UiManager.FILE_OPS_ERROR_DIALOG_STAGE_TITLE;
@@ -18,7 +19,7 @@ public class ErrorDialogGuiTest extends AddressBookGuiTest {
 
     @Test
     public void showErrorDialogs() {
-        raise(new DataSavingExceptionEvent(IO_EXCEPTION_STUB));
+        postLater(new DataSavingExceptionEvent(IO_EXCEPTION_STUB));
 
         guiRobot.waitForEvent(() -> guiRobot.isWindowShown(FILE_OPS_ERROR_DIALOG_STAGE_TITLE));
 

diff --git a/src/test/java/guitests/FindCommandTest.java b/src/test/java/guitests/FindCommandTest.java
@@ -3,6 +3,8 @@
 import static seedu.address.testutil.TypicalPersons.BENSON;
 import static seedu.address.testutil.TypicalPersons.DANIEL;
 import static seedu.address.ui.testutil.GuiTestAssert.assertListMatching;
+import static seedu.address.ui.testutil.GuiTestAssert.assertListSize;
+import static seedu.address.ui.testutil.GuiTestAssert.assertResultMessage;
 
 import org.junit.Test;
 
@@ -33,13 +35,13 @@ public void find_emptyList() throws Exception {
     @Test
     public void find_invalidCommand_fail() {
         runCommand(FindCommand.COMMAND_WORD + "george");
-        assertResultMessage(Messages.MESSAGE_UNKNOWN_COMMAND);
+        assertResultMessage(getResultDisplay(), Messages.MESSAGE_UNKNOWN_COMMAND);
     }
 
     private void assertFindResult(String command, ReadOnlyPerson... expectedHits) throws Exception {
         runCommand(command);
-        assertListSize(expectedHits.length);
-        assertResultMessage(expectedHits.length + " persons listed!");
+        assertListSize(getPersonListPanel(), expectedHits.length);
+        assertResultMessage(getResultDisplay(), expectedHits.length + " persons listed!");
         assertListMatching(getPersonListPanel(), expectedHits);
     }
 }
diff --git a/src/test/java/guitests/GuiRobot.java b/src/test/java/guitests/GuiRobot.java
@@ -14,8 +14,8 @@
  */
 public class GuiRobot extends FxRobot {
 
-    public static final int PAUSE_FOR_HUMAN_DELAY_MILLISECONDS = 250;
-    public static final int DEFAULT_WAIT_FOR_EVENT_TIMEOUT_MILLISECONDS = 5000;
+    private static final int PAUSE_FOR_HUMAN_DELAY_MILLISECONDS = 250;
+    private static final int DEFAULT_WAIT_FOR_EVENT_TIMEOUT_MILLISECONDS = 5000;
 
     private static final String PROPERTY_TESTFX_HEADLESS = "testfx.headless";
 

diff --git a/src/test/java/guitests/SelectCommandTest.java b/src/test/java/guitests/SelectCommandTest.java
@@ -4,6 +4,8 @@
 import static seedu.address.testutil.TypicalPersons.INDEX_FIRST_PERSON;
 import static seedu.address.testutil.TypicalPersons.getTypicalPersons;
 import static seedu.address.ui.testutil.GuiTestAssert.assertCardEquals;
+import static seedu.address.ui.testutil.GuiTestAssert.assertListSize;
+import static seedu.address.ui.testutil.GuiTestAssert.assertResultMessage;
 
 import org.junit.Test;
 
@@ -36,30 +38,30 @@ public void selectPerson_nonEmptyList() throws Exception {
     @Test
     public void selectPerson_emptyList() {
         runCommand(ClearCommand.COMMAND_WORD);
-        assertListSize(0);
+        assertListSize(getPersonListPanel(), 0);
         assertSelectionInvalid(INDEX_FIRST_PERSON); //invalid index
     }
 
     private void assertSelectionInvalid(Index index) {
         runCommand(SelectCommand.COMMAND_WORD + " " + index.getOneBased());
-        assertResultMessage("The person index provided is invalid");
+        assertResultMessage(getResultDisplay(), "The person index provided is invalid");
     }
 
     private void assertSelectionSuccess(Index index) throws Exception {
         runCommand(SelectCommand.COMMAND_WORD + " " + index.getOneBased());
-        assertResultMessage("Selected Person: " + index.getOneBased());
+        assertResultMessage(getResultDisplay(), "Selected Person: " + index.getOneBased());
         assertCardSelected(index);
     }
 
     private void assertCardSelected(Index index) throws Exception {
         PersonCardHandle expectedCard = getPersonListPanel().getPersonCardHandle(index.getZeroBased());
-        PersonCardHandle selectedCard = getPersonListPanel().getHandleToSelectedCard().get();
+        PersonCardHandle selectedCard = getPersonListPanel().getHandleToSelectedCard();
         assertCardEquals(expectedCard, selectedCard);
         //TODO: confirm the correct page is loaded in the Browser Panel
     }
 
     private void assertNoCardSelected() {
-        assertFalse(getPersonListPanel().getHandleToSelectedCard().isPresent());
+        assertFalse(getPersonListPanel().isAnyCardSelected());
     }
 
 }
diff --git a/src/test/java/guitests/guihandles/BrowserPanelHandle.java b/src/test/java/guitests/guihandles/BrowserPanelHandle.java
@@ -1,6 +1,6 @@
 package guitests.guihandles;
 
-import static seedu.address.testutil.EventsUtil.post;
+import static seedu.address.testutil.EventsUtil.postNow;
 
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -28,7 +28,7 @@ public BrowserPanelHandle(Node browserPanelNode) {
         WebEngine engine = webView.getEngine();
         new GuiRobot().interact(() -> engine.getLoadWorker().stateProperty().addListener((obs, oldState, newState) -> {
             if (newState == Worker.State.SUCCEEDED) {
-                post(new WebViewLoadedEvent());
+                postNow(new WebViewLoadedEvent());
             }
         }));
     }

diff --git a/src/test/java/guitests/guihandles/PersonListPanelHandle.java b/src/test/java/guitests/guihandles/PersonListPanelHandle.java
@@ -4,7 +4,6 @@
 import java.util.Optional;
 
 import javafx.scene.control.ListView;
-import seedu.address.commons.core.index.Index;
 import seedu.address.model.person.ReadOnlyPerson;
 import seedu.address.model.person.exceptions.PersonNotFoundException;
 import seedu.address.ui.PersonCard;
@@ -24,22 +23,29 @@ public PersonListPanelHandle(ListView<PersonCard> personListPanelNode) {
     /**
      * Returns a handle to the selected {@code PersonCardHandle}.
      * A maximum of 1 item can be selected at any time.
+     * @throws AssertionError if no card is selected, or more than 1 card is selected.
      */
-    public Optional<PersonCardHandle> getHandleToSelectedCard() {
+    public PersonCardHandle getHandleToSelectedCard() {
         List<PersonCard> personList = getRootNode().getSelectionModel().getSelectedItems();
 
-        if (personList.size() > 1) {
-            throw new AssertionError("Person list size expected 0 or 1.");
+        if (personList.size() != 1) {
+            throw new AssertionError("Person list size expected 1.");
         }
 
-        return personList.isEmpty() ? Optional.empty() : Optional.of(new PersonCardHandle(personList.get(0).getRoot()));
+        return new PersonCardHandle(personList.get(0).getRoot());
     }
 
     /**
-     * Scrolls the list to the {@code index} given.
+     * Returns true if a card is currently selected.
      */
-    public void scrollTo(Index index) {
-        guiRobot.interact(() -> getRootNode().scrollTo(index.getZeroBased()));
+    public boolean isAnyCardSelected() {
+        List<PersonCard> selectedCardsList = getRootNode().getSelectionModel().getSelectedItems();
+
+        if (selectedCardsList.size() > 1) {
+            throw new AssertionError("Card list size expected 0 or 1.");
+        }
+
+        return !selectedCardsList.isEmpty();
     }
 
     /**

diff --git a/src/test/java/seedu/address/testutil/EventsUtil.java b/src/test/java/seedu/address/testutil/EventsUtil.java
@@ -1,6 +1,7 @@
 package seedu.address.testutil;
 
 import guitests.GuiRobot;
+import javafx.application.Platform;
 import seedu.address.commons.core.EventsCenter;
 import seedu.address.commons.events.BaseEvent;
 
@@ -12,7 +13,14 @@ public class EventsUtil {
      * Posts {@code event} to all registered subscribers. This method will return successfully after the {@code event}
      * has been posted to all subscribers.
      */
-    public static void post(BaseEvent event) {
+    public static void postNow(BaseEvent event) {
         new GuiRobot().interact(() -> EventsCenter.getInstance().post(event));
     }
+
+    /**
+     * Posts {@code event} to all registered subscribers at some unspecified time in the future.
+     */
+    public static void postLater(BaseEvent event) {
+        Platform.runLater(() -> EventsCenter.getInstance().post(event));
+    }
 }
diff --git a/src/test/java/seedu/address/ui/BrowserPanelTest.java b/src/test/java/seedu/address/ui/BrowserPanelTest.java
@@ -1,7 +1,7 @@
 package seedu.address.ui;
 
 import static org.junit.Assert.assertEquals;
-import static seedu.address.testutil.EventsUtil.post;
+import static seedu.address.testutil.EventsUtil.postNow;
 import static seedu.address.testutil.TypicalPersons.ALICE;
 import static seedu.address.ui.BrowserPanel.DEFAULT_PAGE;
 import static seedu.address.ui.BrowserPanel.GOOGLE_SEARCH_URL_PREFIX;
@@ -47,7 +47,7 @@ public void display() throws Exception {
         assertEquals(expectedDefaultPageUrl, browserPanelHandle.getLoadedUrl());
 
         // associated web page of a person
-        post(selectionChangedEventStub);
+        postNow(selectionChangedEventStub);
         URL expectedPersonUrl = new URL(GOOGLE_SEARCH_URL_PREFIX
                 + ALICE.getName().fullName.replaceAll(" ", "+") + GOOGLE_SEARCH_URL_SUFFIX);
 

diff --git a/src/test/java/seedu/address/ui/PersonListPanelTest.java b/src/test/java/seedu/address/ui/PersonListPanelTest.java
@@ -1,7 +1,7 @@
 package seedu.address.ui;
 
 import static org.junit.Assert.assertEquals;
-import static seedu.address.testutil.EventsUtil.post;
+import static seedu.address.testutil.EventsUtil.postNow;
 import static seedu.address.testutil.TypicalPersons.INDEX_SECOND_PERSON;
 import static seedu.address.testutil.TypicalPersons.getTypicalPersons;
 import static seedu.address.ui.testutil.GuiTestAssert.assertCardDisplaysPerson;
@@ -50,11 +50,11 @@ public void display() throws Exception {
 
     @Test
     public void handleJumpToListRequestEvent() throws Exception {
-        post(JUMP_TO_SECOND_EVENT);
+        postNow(JUMP_TO_SECOND_EVENT);
         guiRobot.pauseForHuman();
 
         PersonCardHandle expectedCard = personListPanelHandle.getPersonCardHandle(INDEX_SECOND_PERSON.getZeroBased());
-        PersonCardHandle selectedCard = personListPanelHandle.getHandleToSelectedCard().get();
+        PersonCardHandle selectedCard = personListPanelHandle.getHandleToSelectedCard();
         assertCardEquals(expectedCard, selectedCard);
     }
 }