-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change XML file from the original person data to calendar event data, Fix SelectCommandSystemTest #112
Conversation
# Conflicts: # src/test/java/seedu/address/ui/TaskListPanelTest.java # src/test/java/seedu/address/ui/ToDoListEventCardTest.java # src/test/java/systemtests/AddEventCommandSystemTest.java # src/test/java/systemtests/ClearCalendarCommandSystemTest.java # src/test/java/systemtests/DeleteEventCommandSystemTest.java # src/test/java/systemtests/EditEventCommandSystemTest.java # src/test/java/systemtests/FindEventCommandSystemTest.java # src/test/java/systemtests/HelpCommandSystemTest.java # src/test/java/systemtests/SelectCommandSystemTest.java
# Conflicts: # src/test/java/seedu/address/ui/TaskListPanelTest.java # src/test/java/seedu/address/ui/ToDoListEventCardTest.java # src/test/java/systemtests/AddEventCommandSystemTest.java # src/test/java/systemtests/ClearCalendarCommandSystemTest.java # src/test/java/systemtests/DeleteEventCommandSystemTest.java # src/test/java/systemtests/EditEventCommandSystemTest.java # src/test/java/systemtests/FindEventCommandSystemTest.java # src/test/java/systemtests/HelpCommandSystemTest.java # src/test/java/systemtests/SelectCommandSystemTest.java
Pull Request Test Coverage Report for Build 261
💛 - Coveralls |
# Conflicts: # src/main/java/seedu/address/logic/commands/Command.java # src/main/java/seedu/address/model/calendarevent/DateTime.java # src/main/resources/view/DarkTheme.css # src/main/resources/view/MainWindow.fxml # src/main/resources/view/ToDoListEventCard.fxml
TODO: refactor MainWindow (it has too many responsibilities now)
seems good to merge |
throw new AssertionError(e); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a random qn, why are so many edits just adding and deleting the same things?
+ DESCRIPTION_DESC_TUTORIAL + START_DESC_TUTORIAL + END_DESC_TUTORIAL | ||
+ VENUE_DESC_TUTORIAL + TAG_DESC_HUSBAND + TAG_DESC_FRIEND; | ||
+ DESCRIPTION_DESC_TUTORIAL + START_DESC_TUTORIAL + END_DESC_TUTORIAL | ||
+ VENUE_DESC_TUTORIAL + TAG_DESC_HUSBAND + TAG_DESC_FRIEND; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably (eventually) replace the TAG_DESC_HUSBAND and TAG_DESC_FRIEND to something more relevant, like PROJECT or something.
TagsPredicate tagsPredicate = prepareTagsPredicate(""); | ||
FindEventCommand command = new FindEventCommand(predicate, comparator, tagsPredicate); | ||
expectedModel.updateFilteredCalendarEventList(predicate); | ||
expectedModel.sortFilteredCalendarEventList(comparator); // added because FindEventCommand sorts as well | ||
assertCommandSuccess(command, model, commandHistory, expectedMessage, expectedModel); | ||
assertEquals(Arrays.asList(CARL, ELLE, FIONA), model.getFilteredCalendarEventList()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 3 should also eventually be renamed as events. Maybe can put as a TODO?
Looks great! |
Fix regressions caused by changing xml as well