Add Instrumental Test for BookmarksBridge

chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java

Index: chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java
new file mode 100644
index 0000000000000000000000000000000000000000..d7addd3110ab48c1d7d16d007399176ce6b0528c
--- /dev/null
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/BookmarksBridgeTest.java
@@ -0,0 +1,135 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser;
+
+import android.test.UiThreadTest;
+import android.test.suitebuilder.annotation.SmallTest;
+
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.test.util.Feature;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.shell.ChromeShellActivity;
+import org.chromium.chrome.shell.ChromeShellTab;
+import org.chromium.chrome.shell.ChromeShellTestBase;
+import org.chromium.components.bookmarks.BookmarkId;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+
+/**
+ * Tests for bookmark bridge
+ */
+public class BookmarksBridgeTest extends ChromeShellTestBase {
+
+    private ChromeShellActivity mActivity;
+    private Profile mProfile;
+    private BookmarksBridge mBookmarksBridge;
+    private BookmarkId mMobileNode;
+    private BookmarkId mOtherNode;
+    private BookmarkId mDesktopNode;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        mActivity = launchChromeShellWithBlankPage();
+        assertTrue(waitForActiveShellToBeDoneLoading());
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                ChromeShellTab tab = mActivity.getActiveTab();
+                mProfile = tab.getProfile();
+                mBookmarksBridge = new BookmarksBridge(mProfile);
+                mBookmarksBridge.loadEmptyPartnerBookmarkShimForTesting();
+                mMobileNode = mBookmarksBridge.getMobileFolderId();
+                mDesktopNode = mBookmarksBridge.getDesktopFolderId();
+                mOtherNode = mBookmarksBridge.getOtherFolderId();
+            }
+        });
+    }
+
+    @UiThreadTest
+    @SmallTest
+    @Feature({"Bookmark"})
+    public void testAddBookmarksAndFolders() {
+        BookmarkId[] bookmarks = new BookmarkId[] {
+                mBookmarksBridge.addBookmark(mDesktopNode, 0, "a", "http://a.com"),
+                mBookmarksBridge.addBookmark(mOtherNode, 0, "b", "http://b.com"),
+                mBookmarksBridge.addBookmark(mMobileNode, 0, "c", "http://c.com"),
+        };
+        BookmarkId[] folders = new BookmarkId[] {
+                mBookmarksBridge.addFolder(mOtherNode, 0, "fa"),
+                mBookmarksBridge.addFolder(mDesktopNode, 0, "fb"),
+                mBookmarksBridge.addFolder(mMobileNode, 0, "fc"),
+        };
+        for (int i = 0; i < bookmarks.length; i++) {
+            assertNotNull(bookmarks[i]);
+            assertEquals(mBookmarksBridge.getBookmarkById(bookmarks[i]).getTitle(),

[Ted C, 2014/09/17 19:51:31]

I think these verification steps are still a bit more complicated than
necessary.

In my opinion, it would be clearer to have a separate verification function (or
two if you want to have one for bookmarks and one for folders):

verifyBookmarkEntry(BookmarkId id, String title, String url, boolean isFolder,
BookmarkId parentId) {
    BookmarkItem item = mBookmarksBridge.getBookmarkById(id);
    assertEquals(title, item.getTitle());
    ...
}

Then it would just be:
BookmarkId id = mBookmarksBridge.addBookmark(mDesktopNode, 0, "a",
"http://a.com");
verifyBookmarkEntry(id, "a", "http://a.com", false, mDesktopNode);

...
...
...

That should make the test really easy to understand and doesn't require jumping
around.

[Ian Wen, 2014/09/17 20:41:23]

Done.

+                    "" + (char) ('a' + i));
+            assertEquals(mBookmarksBridge.getBookmarkById(bookmarks[i]).getUrl(),
+                    "http://" + (char)('a' + i) + ".com/");
+        }
+        for (int i = 0; i < folders.length; i++) {
+            assertNotNull(folders[i]);
+            assertEquals(mBookmarksBridge.getBookmarkById(folders[i]).getTitle(),
+                    "f" + (char) ('a' + i));
+        }
+    }
+
+    @UiThreadTest
+    @SmallTest
+    @Feature({"Bookmark"})
+    public void testGetAllFoldersWithDepths() {
+        BookmarkId folderA = mBookmarksBridge.addFolder(mMobileNode, 0, "a");
+        BookmarkId folderB = mBookmarksBridge.addFolder(mDesktopNode, 0, "b");
+        BookmarkId folderC = mBookmarksBridge.addFolder(mOtherNode, 0, "c");
+        BookmarkId folderAA = mBookmarksBridge.addFolder(folderA, 0, "aa");
+        BookmarkId folderBA = mBookmarksBridge.addFolder(folderB, 0, "ba");
+        BookmarkId folderAAA = mBookmarksBridge.addFolder(folderAA, 0, "aaa");
+        BookmarkId folderAAAA = mBookmarksBridge.addFolder(folderAAA, 0, "aaaa");
+
+        HashMap<BookmarkId, Integer> idToDepth = new HashMap<BookmarkId, Integer>();
+        idToDepth.put(folderA, 0);
+        idToDepth.put(folderAA, 1);
+        idToDepth.put(folderAAA, 2);
+        idToDepth.put(folderAAAA, 3);
+        idToDepth.put(mDesktopNode, 0);
+        idToDepth.put(folderB, 1);
+        idToDepth.put(folderBA, 2);
+        idToDepth.put(folderC, 0);

[Kibeom Kim (inactive), 2014/09/17 19:28:08]

how about just creating ground-truth expectedFolderList and expectedDepthList,
and compare that by == below. I think that would make the code much simpler.

[Ted C, 2014/09/17 19:51:31]

I think you'd want to use:
http://developer.android.com/reference/android/test/MoreAsserts.html#assertCo...,
java.lang.Object...)
To compare equality, but I think that should work.

[Ian Wen, 2014/09/17 20:41:22]

My major concern about comparing lists directly was that in other languages, the
position of "Desktop Folder" will change, so if we launch a bot in other
languages, this instrumental test is going to fail. Should I care about
multi-language here?

[Kibeom Kim (inactive), 2014/09/18 17:46:37]

Good point! I'm not sure if there is any bot running other than English. But I
guess handling non-English mDesktopNode shouldn't hurt, maybe exclude the node
from the result and expected arrays before you compare?

+
+        BookmarkId[] bookmarks = new BookmarkId[] {
+                mBookmarksBridge.addBookmark(mMobileNode, 0, "ua", "http://www.google.com"),
+                mBookmarksBridge.addBookmark(mDesktopNode, 0, "ua", "http://www.google.com"),
+                mBookmarksBridge.addBookmark(mOtherNode, 0, "ua", "http://www.google.com"),
+                mBookmarksBridge.addBookmark(folderA, 0, "ua", "http://www.medium.com") };
+
+        List<BookmarkId> folderList = new ArrayList<BookmarkId>();
+        List<Integer> depthList = new ArrayList<Integer>();
+        mBookmarksBridge.getAllFoldersWithDepths(folderList, depthList);
+
+        assertEquals(folderList.size(), depthList.size());
+        assertEquals(folderList.size(), idToDepth.size());
+
+        // Every folder in hashmap should exist in folderList
+        for (BookmarkId folder: idToDepth.keySet()) {
+            assertTrue(folderList.contains(folder));
+        }
+        // folderList should not contain any bookmark
+        for (BookmarkId bookmark : bookmarks) {
+            assertFalse(folderList.contains(bookmark));
+        }
+        assertFalse(folderList.contains(mOtherNode));
+        assertFalse(folderList.contains(mMobileNode));
+
+        for (int i = 0; i < folderList.size(); i++) {
+            assertNotNull(folderList.get(i));
+            assertNotNull(depthList.get(i));
+            assertTrue(depthList.get(i) >= 0);
+            assertTrue(idToDepth.containsKey(folderList.get(i)));
+            assertEquals(depthList.get(i), idToDepth.get(folderList.get(i)));
+        }
+    }
+}