Add unit tests for ViewAllCommand and ViewCommand classes #111 #131

zzzzwen · 2016-12-29T16:04:43Z

code ideas and format follow DeleteCommandTest

Adding header comments later

For ViewAll command, should I do it in a separate file or together with view command

Fixes #111

damithc · 2016-12-30T01:17:50Z

@zzzzwen

For ViewAll command, should I do it in a separate file or together with view command

Usually, we have one test class for each functional class

zzzzwen · 2016-12-30T03:14:19Z

Ready for review,

two tests are roughly the same

Here are some of my questions:

in ViewCommandTest:
assertViewSuccess, assertViewErrorInvalidIndex, assertViewErrorPersonNotInAddressBook are actually the same except the expected message is different, should I inline all three?

assertCommandResult this method can be quite general by just asserting the result message, if modify a bit, can move it to TestUtil class?

Same things apply to ViewAllCommandTest

damithc

Added comments to one test class. Can revise the other based on the same comments.

damithc · 2016-12-30T04:17:17Z

test/java/seedu/addressbook/commands/ViewAllCommandTest.java

+                                       new Phone("1234567", true),
+                                       new Email("[email protected]", false),
+                                       new Address("Newgate Prison", true),
+                                       new UniqueTagList(new Tag("friend"), new Tag("criminal")));


Two persons is a bit a special case because the list doesn't have persons in the middle. A 3-person list is more general.

damithc · 2016-12-30T04:18:15Z

test/java/seedu/addressbook/commands/ViewAllCommandTest.java

+        emptyDisplayList = TestUtil.createList();
+        listWithAll = TestUtil.createList(johnDoe, betsyCrowe);
+        listWithSome = TestUtil.createList(betsyCrowe);
+    }


Extract into a TypicalPersons utility class?

damithc · 2016-12-30T04:22:39Z

test/java/seedu/addressbook/commands/ViewAllCommandTest.java

+        // non-empty addressbook
+        assertViewAllErrorInvalidIndex(addressBook, listWithAll, -1);
+        assertViewAllErrorInvalidIndex(addressBook, listWithAll, 0);
+        assertViewAllErrorInvalidIndex(addressBook, listWithAll, 3);


if you use size()+1 of the list instead of magic number 3, the test will be less brittle (less likely to break in future).

damithc · 2016-12-30T04:26:03Z

test/java/seedu/addressbook/commands/ViewAllCommandTest.java

+     * Executes the command, and asserts the result message is as expected.
+     */
+    private void assertCommandResult(Command command, String expectedMessage,
+                                        AddressBook addressBook) {


Do you need this line break?

damithc · 2016-12-30T04:26:40Z

test/java/seedu/addressbook/commands/ViewAllCommandTest.java

+     * and displayed.
+     */
+    private void assertViewAllSuccess(AddressBook addressBook, List<ReadOnlyPerson> list, int index) {
+        String expectedMessage = String.format(ViewAllCommand.MESSAGE_VIEW_PERSON_DETAILS, list.get(index - 1).getAsTextShowAll());


Line too long?

damithc · 2016-12-30T04:35:39Z

@zzzzwen Yes, you can move common methods to TestUtil class.

damithc · 2016-12-30T06:54:17Z

test/java/seedu/addressbook/util/TypicalPersons.java

+            e.printStackTrace();
+            assert false : "not possible";
+        }
+    }


There is another similar class in #120 that might be merged before this. May be you can follow the same? Note how the person names follow a pattern. e.g. First person has initials A B, second one B C, and so on. Even the address follows a pattern and not random. Such patterns help us detect when things go out of place.

@damithc So I wait for the other pr to merge first then use and edit his TypicalPersons for the test?

Since we both have same class created

That class is mostly stable now, so you can copy-paste-modify that code into your and continue with your PR. If that PR is merged first, you'll have to update your branch and de-conflict your class a bit at that point.

zzzzwen · 2016-12-30T08:23:27Z

Updated, I will leave it here first, after merging #120 I will resolve the conflict

into 111-unit-test-view-viewall # Conflicts: # test/java/seedu/addressbook/util/TypicalPersons.java

zzzzwen · 2017-01-03T14:21:43Z

Updated, ready for review

damithc

Some comments added.

damithc · 2017-01-03T15:04:21Z

test/java/seedu/addressbook/commands/ViewAllCommandTest.java

+    public void setUp() throws Exception {
+        TypicalPersons td = new TypicalPersons();
+
+        emptyAddressBook = TestUtil.createAddressBook();


This can be initialized at the same place it's declared. Junit creates a new instance for each test.

@damithc This TestUtil.createAddressBook() is throwing an excpetion in TestUtil Class so cannot put outside right?

In that case can keep.

damithc · 2017-01-03T15:04:34Z

test/java/seedu/addressbook/commands/ViewAllCommandTest.java

+        TypicalPersons td = new TypicalPersons();
+
+        emptyAddressBook = TestUtil.createAddressBook();
+        addressBook = td.getTypicalAddressBook();


Same for this

This comment was intended for the line below. Sorry.

Also note #134

damithc · 2017-01-03T15:05:51Z

test/java/seedu/addressbook/commands/ViewAllCommandTest.java

+
+        emptyDisplayList = TestUtil.createList();
+        listWithAll = td.getListWithAllPersons();
+        listWithSome = td.getListWithSomePersons();


On a related note, lines 32, 35, 38, 39, should have been put together as they are connected. i.e. declaring a variable and then using it.

damithc · 2017-01-03T15:08:51Z

test/java/seedu/addressbook/commands/ViewAllCommandTest.java

+    }
+
+    /**
+     * Asserts that the details person at specific index can be successfully retrieved


details of the?

damithc · 2017-01-03T15:11:15Z

test/java/seedu/addressbook/commands/ViewAllCommandTest.java

+
+        Command command = generateViewAllCommand(addressBook, list, index);
+        assertCommandResult(command, expectedMessage, addressBook, TestUtil.clone(addressBook));
+    }


Can the common part of the above two methods can be extracted as a method?

damithc · 2017-01-03T15:15:01Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+        Command command = generateViewCommand(addressBook, list, index);
+        assertCommandResult(command, expectedMessage, addressBook, TestUtil.clone(addressBook));
+    }
+}


If this class is almost the same as the other test class, we can text both commands in one of the class and make the other class empty with just a comment to explain why it is empty.

damithc · 2017-01-03T15:16:18Z

test/java/seedu/addressbook/util/TestUtil.java

+     * Executes the command, and asserts the result message is as expected.
+     */
+    public static void assertCommandResult(Command command, String expectedMessage, AddressBook addressBook, 
+            AddressBook expectedAddressBook) {


More readable indentation:

public static void assertCommandResult(Command command, String expectedMessage, AddressBook addressBook, AddressBook expectedAddressBook) {

damithc · 2017-01-03T15:17:41Z

test/java/seedu/addressbook/util/TypicalPersons.java

+         return list;
+     }
+ }
+


Add a line feed at the end of the file

damithc · 2017-01-03T15:19:12Z

test/java/seedu/addressbook/util/TypicalPersons.java

-        return ab;
-    }
-
-}


Not sure why this is shown as a diff. Most of it looks identical.

oh, this is because there is an extra whitespace for each line, sorry

zzzzwen · 2017-01-04T03:16:55Z

Updated

zzzzwen · 2017-01-04T03:18:52Z

Removed use of TestUtil.createList()

damithc

Much better. Some comments added.

damithc · 2017-01-04T03:36:17Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+        assertViewSuccess(addressBook, listWithAll, 1);
+
+        // person with some private information
+        assertViewSuccess(addressBook, listWithAll, 2);


You need a person with all info private, to confirm that 'privacy' works for each relevant field.

@damithc In TypicalPersons I have a person called dan, all the information which has the private/public option for him is private.

I meant such a person must be included as one of the test cases in this method. Your comment in line 74 says some private information so I guess that's not dan.

oh, I get it

damithc · 2017-01-04T03:37:14Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+    }
+
+    /**
+     * Creates a new View command.


ViewCommand

Actually, this comment doesn't add much value. Can be removed.

damithc · 2017-01-04T03:39:07Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+
+        expectedMessage = String.format(ViewCommand.MESSAGE_VIEW_PERSON_DETAILS,
+                                        list.get(index - 1).getAsTextShowAll());
+        assertViewAllCommandBehaviour(addressBook, list, expectedMessage, index);


index - 1 appears twice. Assign to local variable?

I feel that a new local variable, eg, pos = index - 1 doesn't add much value since index-1 just refer to the position in the list and won't be changed. Is it necessary to extract to a local variable?

By local, I meant inside the method. Reasons,

It's not good when a calculation is duplicated.

its meaning is not immediately clear.

Something like this will help clear both problems.

int internalIndex = index - 1; // because index is one-indexed

damithc · 2017-01-04T03:44:40Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+        String expectedMessage = Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX;
+
+        assertViewCommandBehaviour(addressBook, list, expectedMessage, index);
+        assertViewAllCommandBehaviour(addressBook, list, expectedMessage, index);


Can be simplified as this (after extracting common part of this and next method)?
assertCommandError(addressBook, list, index, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX)

damithc · 2017-01-04T03:46:47Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+    /**
+     * Asserts that the View command gives correct feedback information
+     */
+    private void assertViewCommandBehaviour(AddressBook addressBook, List<ReadOnlyPerson> list, String message, int index) {


Reorder parameters so that the three input values come first?
message -> expectedMessage

damithc · 2017-01-04T03:47:49Z

test/java/seedu/addressbook/util/TestUtil.java

+     * Executes the command, and asserts the result message is as expected.
+     */
+    public static void assertCommandResult(Command command, String expectedMessage, AddressBook addressBook, 
+                                                                             AddressBook expectedAddressBook) {


Similarly, can reorder parameters so that input comes first and expected values come next.

damithc · 2017-01-04T03:50:09Z

test/java/seedu/addressbook/util/TypicalPersons.java

 import seedu.addressbook.data.tag.UniqueTagList;
-
+ 


Remove extra space?

damithc · 2017-01-04T03:54:58Z

test/java/seedu/addressbook/util/TypicalPersons.java

+        List<ReadOnlyPerson> list = getListWithAllPersons();
+        list.remove(0); //remove a person in the list
+        return list;
+    }


This method is very specific to your test class and therefore not a good abstraction. Try something like this instead:
public List<ReadOnlyPerson> getList(ReadOnlyPerson... personsToInclude);

Can be used like this:
getList(td.amy, td.candy);

zzzzwen · 2017-01-08T05:33:55Z

Updated, but ViewCommand and ViewAllCommand are two separate commands and my original idea is to split them into two different blocks of methods to show they are different.

pyokagan

Please configure your editor to strip all trailing whitespace and mark out the 110 character boundary.

pyokagan · 2017-01-08T16:21:59Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+     * Asserts that the ViewCommand and ViewAllCommand show given feedback information
+     */
+    private static void assertViewBehavior(Command viewCommand, AddressBook addressBook,
+        List<ReadOnlyPerson> list, String expectedMessage) {


Indent this further to the right.

pyokagan · 2017-01-08T16:22:20Z

test/java/seedu/addressbook/util/TestUtil.java

@@ -75,4 +78,5 @@ public static void assertTextFilesEqual(Path path1, Path path2) throws IOExcepti
        List<String> list2 = Files.readAllLines(path2, Charset.defaultCharset());
        assertEquals(String.join("\n", list1), String.join("\n", list2));
    }
+


Unnecessary change.

pyokagan · 2017-01-08T16:30:22Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+
+public class ViewCommandTest {
+    private TypicalPersons td = new TypicalPersons();
+


Trailing whitespace

pyokagan · 2017-01-08T16:30:34Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+        // non-empty addressbook
+        assertViewErrorInvalidIndex(typicalAddressBook, displayListWithAllTypicalPersons, -1);
+        assertViewErrorInvalidIndex(typicalAddressBook, displayListWithAllTypicalPersons, 0);
+        assertViewErrorInvalidIndex(typicalAddressBook, displayListWithAllTypicalPersons, displayListWithAllTypicalPersons.size() + 1);


Overlong line way above 110 chars.

pyokagan · 2017-01-08T16:30:59Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+                                            new Email("[email protected]", true),
+                                            new Address("nus", false),
+                                            new UniqueTagList(Collections.emptySet()));
+        List <ReadOnlyPerson> displayListWithExtraPerson = new ArrayList<ReadOnlyPerson>(displayListWithAllTypicalPersons);


Overlong line way above 110 chars.

pyokagan · 2017-01-08T16:31:11Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+        assertViewErrorPersonNotInAddressBook(emptyAddressBook, displayListWithExtraPerson, 1);
+
+        // non-empty addressbook
+        assertViewErrorPersonNotInAddressBook(typicalAddressBook, displayListWithExtraPerson, displayListWithExtraPerson.size());


Overlong line way above 110 chars.

pyokagan · 2017-01-08T16:31:30Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+    /**
+     * Asserts that both a ViewCommand and a ViewAllCommand can retrieve from
+     * the {@code addressBook} details of the person at the given {@code index} in the given {@code list}.
+     * 


Trailing whitespace.

pyokagan · 2017-01-08T16:37:06Z

@zzzzwen I notice you are still using the displayList terminology in your code. As I noted previously, this is inconsistent with the terminology already in use (relevantPersons).

Maybe displayList is a better name ( I personally think that relevantPersons is not such a great term, but I'm not so sure that displayList is a good replacement either. @damithc What do you think? ) But if so, you need to commit to changing all instances of relevantPersons in the code base to use your displayList terminology. Are you up to the task? If so, make sure you open up an issue for that.

Otherwise, please change all your naming to use relevantPersons.

zzzzwen · 2017-01-09T00:56:52Z

Configured and updated. Replaced all with parameters in Command

pyokagan

There are still lots of references to displayList in the code. I've suggested some alternatives, or you can think up of your own as long as they don't introduce new terminology not used in the existing code base.

pyokagan · 2017-01-09T07:01:15Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+
+    private AddressBook typicalAddressBook = td.getTypicalAddressBook();
+    private AddressBook emptyAddressBook = TestUtil.createAddressBook();
+    private List<ReadOnlyPerson> emptyDisplayList = Collections.emptyList();


Maybe rename emptyDisplayList to emptyPersonList.

pyokagan · 2017-01-09T07:01:50Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+    private AddressBook typicalAddressBook = td.getTypicalAddressBook();
+    private AddressBook emptyAddressBook = TestUtil.createAddressBook();
+    private List<ReadOnlyPerson> emptyDisplayList = Collections.emptyList();
+    private List<ReadOnlyPerson> displayListWithAllTypicalPersons = Arrays.asList(td.getTypicalPersons());


Maybe rename displayListWithAllTypicalPersons to allTypicalPersonsList or listWithAllTypicalPersons.

pyokagan · 2017-01-09T07:02:06Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+    private AddressBook emptyAddressBook = TestUtil.createAddressBook();
+    private List<ReadOnlyPerson> emptyDisplayList = Collections.emptyList();
+    private List<ReadOnlyPerson> displayListWithAllTypicalPersons = Arrays.asList(td.getTypicalPersons());
+    private List<ReadOnlyPerson> listWithSome = Arrays.asList(td.amy, td.candy, td.dan);


Maybe rename listWithSome to listWithSomeTypicalPersons.

pyokagan · 2017-01-09T07:03:00Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+                                            new Email("[email protected]", true),
+                                            new Address("nus", false),
+                                            new UniqueTagList(Collections.emptySet()));
+        List <ReadOnlyPerson> displayListWithExtraPerson


Can just be listWithExtraPerson. Also, remove the space in List <ReadOnlyPerson>.

pyokagan · 2017-01-09T07:03:35Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+
+    @Test
+    public void execute_personNotInAddressBook_returnsPersonNotInAddressBookMessage() throws Exception {
+        // generate person not in addressbook, add to displayList


More descriptive: generate list with person not in addressbook.

pyokagan · 2017-01-09T07:03:57Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+     * Asserts that the details of person at specific index cannot be retrieved due to
+     * invalid index.
+     */
+    private void assertViewErrorInvalidIndex(AddressBook addressBook, List<ReadOnlyPerson> list,


Rename list to relevantPersons.

pyokagan · 2017-01-09T07:04:06Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+     * Asserts that the details of person at specific index cannot be retrieved due to
+     * person not existing in the addressbook.
+     */
+    private void assertViewErrorPersonNotInAddressBook(AddressBook addressBook, List<ReadOnlyPerson> list,


Rename list to relevantPersons.

pyokagan · 2017-01-09T07:06:37Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+    }
+
+    /**
+     * Asserts that the ViewCommand and ViewAllCommand show given feedback information


It does not just check for feedback information. You may wish to enumerate the list of things it checks for, e.g.

Checks that viewCommand exhibits the correct view command behavior, namely: 1. The feedback message of the CommandResult it returns matches expectedMessage 2. The CommandResult it returns has no relevant persons. 3. ...

zzzzwen · 2017-01-09T11:53:51Z

Updated, the displayList in the assertViewErrorInvalidIndex and assertViewErrorPersonNotInAddressBook are not changed in the previous commits because the terminology relevantPersons is for constructing the Command and those two in the methods still don't touch the Command yet, so I didn't change them in the two methods.

pyokagan

Good, just a few nits. I think everything else looks fine.

pyokagan · 2017-01-12T11:56:56Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+
+    /**
+     * Asserts that both a ViewCommand and a ViewAllCommand can retrieve from
+     * the {@code addressBook} details of the person at the given {@code index} in the given {@code list}.


Change {@code index} to {@code targetVisibleIndex}?

Change {@code list} to {@code relevantPersons} list?

pyokagan · 2017-01-12T12:00:46Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+     */
+    private void assertViewErrorInvalidIndex(AddressBook addressBook, List<ReadOnlyPerson> relevantPersons,
+                                             int targetVisibleIndex) {
+        assertViewError(addressBook, relevantPersons, targetVisibleIndex, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);


This line is much longer than 110 chars.

pyokagan · 2017-01-12T12:01:01Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+     */
+    private void assertViewSuccess(AddressBook addressBook, List<ReadOnlyPerson> relevantPersons,
+                                   int targetVisibleIndex) {
+        ReadOnlyPerson personToBeViewed = relevantPersons.get(targetVisibleIndex - 1); // -1 is because index is one-indexed


This line is much longer than 110 chars.

We could put it like that:

// -1 is because index is one-indexed ReadOnlyPerson personToBeViewed = relevantPersons.get(targetVisibleIndex - 1);

but imo that looks weird because usually a comment at the code block describes what the code below overall does, but in this case all we are doing is just commenting on the use of -1.

So maybe we need to add a description on what the code below does, even though it may be obvious. So something like:

// Get person to be viewed (targetVisibleIndex - 1 because index is one-indexed) ReadOnlyPerson personToBeViewed = relevantPersons.get(targetVisibleIndex - 1);

zzzzwen · 2017-01-13T02:33:20Z

Updated

damithc · 2017-01-13T11:25:20Z

@zzzzwen the Fixes #... info is missing from the PR description.

zzzzwen · 2017-01-13T15:17:13Z

oh, added

pyokagan

Good work.

pyokagan · 2017-01-15T03:57:05Z

@damithc Can reconfirm your approval? PR has changed since your last review.

damithc

Looks good. A few cosmetic nits only.

damithc · 2017-01-15T04:36:50Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+        assertViewErrorInvalidIndex(typicalAddressBook, listWithAllTypicalPersons, -1);
+        assertViewErrorInvalidIndex(typicalAddressBook, listWithAllTypicalPersons, 0);
+        assertViewErrorInvalidIndex(typicalAddressBook, listWithAllTypicalPersons,
+                listWithAllTypicalPersons.size() + 1);


Minor nit: indent for better readability. This way, ( and ) that demarcate the parameter list appear on far left and far right of the parameters, respectively.

assertViewErrorInvalidIndex(typicalAddressBook, listWithAllTypicalPersons, listWithAllTypicalPersons.size() + 1);

This applies to a few other places as well.

damithc · 2017-01-15T04:39:02Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+    @Test
+    public void execute_personNotInAddressBook_returnsPersonNotInAddressBookMessage() throws Exception {
+        // generate list with person not in addressbook, add to list
+        ReadOnlyPerson someone = new Person(new Name("me"),


someone --> stranger?

damithc · 2017-01-15T04:43:57Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+        assertViewSuccess(typicalAddressBook, listWithAllTypicalPersons, 4);
+
+        // addressbook has more people than the list
+        // This can happen when view from filtered list caused by some commands(eg. FindCommand)


This sentence is not very clear.
This can happen when a command causes the list to show only a sub-set of persons (e.g. FindCommand).

Also, full sentences should end with full stop.

damithc · 2017-01-15T04:46:49Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

+     */
+    private void assertViewSuccess(AddressBook addressBook, List<ReadOnlyPerson> relevantPersons,
+                                   int targetVisibleIndex) {
+        // Get person to be viewed (targetVisibleIndex - 1 because index is one-indexed)


get (to be consistent with other comments in this class)?

zzzzwen · 2017-01-15T06:54:59Z

Updated

damithc

One minor nit remaining

damithc · 2017-01-15T09:52:21Z

test/java/seedu/addressbook/commands/ViewCommandTest.java

-        // addressbook has more people than the list
-        // This can happen when view from filtered list caused by some commands(eg. FindCommand)
+        // addressbook has more people than the list.
+        // this can happen when a command causes the list to show only a sub-set of persons(e.g. FindCommand).


Full sentences should start with a upper case letter too. :-) So it should either a full sentence starting with a upper case letter and ending with a full stop, or just a short phrase starting with a lower case letter and no full stop at the end.

zzzzwen · 2017-01-15T10:26:11Z

ok

pyokagan · 2017-01-17T14:14:18Z

@damithc Any more comments for this PR?

add unit test for view command

20e25f1

zzzzwen added 3 commits December 30, 2016 10:47

Add header comment

017b996

Add unit test for ViewAll command

9d049e3

javadoc change

f97c6b1

damithc requested changes Dec 30, 2016

View reviewed changes

zzzzwen added 4 commits December 30, 2016 14:01

Move assertCommandResult method to TestUtil

e942a7e

Extract TypicalPersons

42f0c7f

Conflict

b414b98

Merge branch 'master' into 111-unit-test-view-viewall

7140ae2

damithc reviewed Dec 30, 2016

View reviewed changes

zzzzwen added 2 commits December 30, 2016 14:56

Conflict resolving

5457976

Rework TypicalPersons

ffd7b0f

zzzzwen added 5 commits December 30, 2016 16:30

code quality

8b3d3a3

Merge branch 'master' of https://github.com/se-edu/addressbook-level2

2486c44

into 111-unit-test-view-viewall # Conflicts: # test/java/seedu/addressbook/util/TypicalPersons.java

Update TypicalPersons

2d1260d

Code style issues

73147c3

Code quality

915c4d9

damithc requested changes Jan 3, 2017

View reviewed changes

zzzzwen added 2 commits January 4, 2017 11:14

Code Style and combine two test cases

acfb6a3

Grammar

ea54ab7

Remove use of createList

3a068e6

damithc requested changes Jan 4, 2017

View reviewed changes

Code Style

2d4cf4f

Combine to generic methods

2a11bd5

pyokagan requested changes Jan 8, 2017

View reviewed changes

zzzzwen added 2 commits January 9, 2017 08:52

Code style changes

beef8ab

code style changes

c3af8e8

pyokagan requested changes Jan 9, 2017

View reviewed changes

Code style changes

9c60233

pyokagan requested changes Jan 12, 2017

View reviewed changes

code style changes

b9db7e5

pyokagan approved these changes Jan 15, 2017

View reviewed changes

pyokagan added the s.ToMerge label Jan 15, 2017

damithc requested changes Jan 15, 2017

View reviewed changes

zzzzwen added 2 commits January 15, 2017 14:52

Code style changes

5b654c4

code style changes

b4aba03

damithc requested changes Jan 15, 2017

View reviewed changes

Code style changes

eb7f500

damithc approved these changes Jan 17, 2017

View reviewed changes

damithc added s.MergeApproved and removed s.ToMerge labels Jan 17, 2017

pyokagan merged commit f92b1d0 into se-edu:master Jan 17, 2017


		public class ViewCommandTest {
		private TypicalPersons td = new TypicalPersons();

Add unit tests for ViewAllCommand and ViewCommand classes #111 #131

Add unit tests for ViewAllCommand and ViewCommand classes #111 #131

Conversation

zzzzwen commented Dec 29, 2016 • edited by pyokagan Loading

damithc commented Dec 30, 2016

zzzzwen commented Dec 30, 2016 • edited Loading

damithc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damithc commented Dec 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zzzzwen commented Dec 30, 2016

zzzzwen commented Jan 3, 2017

damithc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zzzzwen commented Jan 4, 2017

zzzzwen commented Jan 4, 2017 • edited Loading

damithc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zzzzwen Jan 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zzzzwen commented Jan 8, 2017

pyokagan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pyokagan commented Jan 8, 2017 • edited Loading

zzzzwen commented Jan 9, 2017

pyokagan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zzzzwen commented Dec 29, 2016 •

edited by pyokagan

Loading

zzzzwen commented Dec 30, 2016 •

edited

Loading

zzzzwen commented Jan 4, 2017 •

edited

Loading

zzzzwen Jan 4, 2017 •

edited

Loading

pyokagan commented Jan 8, 2017 •

edited

Loading