Bug 1258127 - Improve existing bookmarks tracker tests. draft
authorMark Hammond <mhammond@skippinet.com.au>
Tue, 10 May 2016 22:47:51 -0700
changeset 365641 cd6856e752d26638f1a6e5161122cba06d01efe4
parent 365640 b5d9a9dfb6f53ca599b83b187c4d83306f6e6f9d
child 365642 2003812dcb0983a55f8b77f2284c4c387b70b29e
child 365644 a0aed2a3a5db68c6a3b85e12fa13703eb9425be9
push id17803
push userkcambridge@mozilla.com
push dateWed, 11 May 2016 06:12:00 +0000
bugs1258127
milestone49.0a1
Bug 1258127 - Improve existing bookmarks tracker tests. The intention of this patch is 2-fold: * Capture more existing semantics of the tracker that aren't currently tested. The intention is that this patch doesn't touch the existing tracker or bookmarks engine implementation at all. * Make structural changes such that later patches that want to ensure the same semantics exist using SQL queries are more obvious and limited only to things directly related to the new tracker - for example, this patch uses tasks/promises even though they aren't necessary here, but will become necessary in later patches. MozReview-Commit-ID: 4ks1YBJZw9Y
services/sync/tests/unit/test_bookmark_tracker.js
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -1,178 +1,483 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://services-sync/bookmark_utils.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/engines/bookmarks.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 
 Service.engineManager.register(BookmarksEngine);
 var engine = Service.engineManager.get("bookmarks");
 var store  = engine._store;
 var tracker = engine._tracker;
 
 store.wipe();
 tracker.persistChangedIDs = false;
 
-function test_tracking() {
+// Test helpers.
+function* verifyTrackerEmpty() {
+  do_check_empty(tracker.changedIDs);
+  do_check_eq(tracker.score, 0);
+}
+
+function* cleanup() {
+  store.wipe();
+  tracker.clearChangedIDs();
+  tracker.resetScore();
+  yield stopTracking();
+}
+
+// startTracking is a signal that the test wants to notice things that happen
+// after this is called (ie, things already tracked should be discarded.)
+function* startTracking() {
+  Svc.Obs.notify("weave:engine:start-tracking");
+}
+
+function* stopTracking() {
+  Svc.Obs.notify("weave:engine:stop-tracking");
+}
+
+function* verifyTrackedItems(tracked) {
+  let trackedIDs = new Set(Object.keys(tracker.changedIDs));
+  for (let guid of tracked) {
+    ok(tracker.changedIDs[guid] > 0, `${guid} should be tracked`);
+    trackedIDs.delete(guid);
+  }
+  equal(trackedIDs.size, 0, `Unhandled tracked IDs: ${
+    JSON.stringify(Array.from(trackedIDs))}`);
+}
+
+function* verifyTrackedCount(expected) {
+  do_check_attribute_count(tracker.changedIDs, expected);
+}
+
+add_task(function* test_tracking() {
   _("Verify we've got an empty tracker to work with.");
-  let tracker = engine._tracker;
-  do_check_empty(tracker.changedIDs);
+  yield verifyTrackerEmpty();
 
   let folder = PlacesUtils.bookmarks.createFolder(
     PlacesUtils.bookmarks.bookmarksMenuFolder,
     "Test Folder", PlacesUtils.bookmarks.DEFAULT_INDEX);
   function createBmk() {
     return PlacesUtils.bookmarks.insertBookmark(
       folder, Utils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
   }
 
   try {
     _("Create bookmark. Won't show because we haven't started tracking yet");
     createBmk();
-    do_check_empty(tracker.changedIDs);
+    yield verifyTrackedCount(0);
     do_check_eq(tracker.score, 0);
 
     _("Tell the tracker to start tracking changes.");
-    Svc.Obs.notify("weave:engine:start-tracking");
+    yield startTracking();
     createBmk();
     // We expect two changed items because the containing folder
     // changed as well (new child).
-    do_check_attribute_count(tracker.changedIDs, 2);
+    yield verifyTrackedCount(2);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 2);
 
     _("Notifying twice won't do any harm.");
-    Svc.Obs.notify("weave:engine:start-tracking");
+    yield startTracking();
     createBmk();
-    do_check_attribute_count(tracker.changedIDs, 3);
+    yield verifyTrackedCount(3);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 4);
 
     _("Let's stop tracking again.");
     tracker.clearChangedIDs();
     tracker.resetScore();
-    Svc.Obs.notify("weave:engine:stop-tracking");
+    yield stopTracking();
     createBmk();
-    do_check_empty(tracker.changedIDs);
+    yield verifyTrackedCount(0);
     do_check_eq(tracker.score, 0);
 
     _("Notifying twice won't do any harm.");
-    Svc.Obs.notify("weave:engine:stop-tracking");
+    yield stopTracking();
     createBmk();
-    do_check_empty(tracker.changedIDs);
+    yield verifyTrackedCount(0);
     do_check_eq(tracker.score, 0);
 
   } finally {
     _("Clean up.");
-    store.wipe();
-    tracker.clearChangedIDs();
-    tracker.resetScore();
-    Svc.Obs.notify("weave:engine:stop-tracking");
+    yield cleanup();
   }
-}
+});
 
-function test_onItemChanged() {
-  // Anno that's in ANNOS_TO_TRACK.
-  const DESCRIPTION_ANNO = "bookmarkProperties/description";
+add_task(function* test_onItemChanged_exclude_anno() {
+  _("Adds an item with an annotation to exclude from backups");
 
   _("Verify we've got an empty tracker to work with.");
-  let tracker = engine._tracker;
-  do_check_empty(tracker.changedIDs);
+  yield verifyTrackerEmpty();
+  try {
+    yield stopTracking();
+
+    _("Create a bookmark");
+    let b = PlacesUtils.bookmarks.insertBookmark(
+      PlacesUtils.bookmarks.bookmarksMenuFolder,
+      Utils.makeURI("http://getfirefox.com"),
+      PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
+    let bGUID = engine._store.GUIDForId(b);
+
+    yield startTracking();
+
+    _("Exclude the bookmark from backups");
+    PlacesUtils.annotations.setItemAnnotation(
+      b, BookmarkAnnos.EXCLUDEBACKUP_ANNO, "Don't back this up", 0,
+      PlacesUtils.annotations.EXPIRE_NEVER);
+
+    _("Modify the bookmark");
+    PlacesUtils.bookmarks.setItemTitle(b, "Download Firefox");
+
+    _("Excluded items should be ignored");
+    yield verifyTrackedItems([]);
+    do_check_eq(track.score, 0);
+  } finally {
+    _("Clean up.");
+    yield cleanup();
+  }
+});
+
+add_task(function* test_onItemChanged() {
+  _("Verify we've got an empty tracker to work with.");
+  yield verifyTrackerEmpty();
   do_check_eq(tracker.score, 0);
-
   try {
-    Svc.Obs.notify("weave:engine:stop-tracking");
+    yield stopTracking();
     let folder = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
+    let folderGUID = engine._store.GUIDForId(folder);
     _("Track changes to annos.");
     let b = PlacesUtils.bookmarks.insertBookmark(
       folder, Utils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
     let bGUID = engine._store.GUIDForId(b);
     _("New item is " + b);
     _("GUID: " + bGUID);
 
-    Svc.Obs.notify("weave:engine:start-tracking");
+    yield startTracking();
     PlacesUtils.annotations.setItemAnnotation(
-      b, DESCRIPTION_ANNO, "A test description", 0,
+      b, BookmarkAnnos.DESCRIPTION_ANNO, "A test description", 0,
       PlacesUtils.annotations.EXPIRE_NEVER);
-    do_check_true(tracker.changedIDs[bGUID] > 0);
+    // bookmark should be tracked, folder should not.
+    yield verifyTrackedItems([bGUID]);
+    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
+
+  } finally {
+    _("Clean up.");
+    yield cleanup();
+  }
+});
+
+add_task(function* test_onItemTagChanged() {
+  _("Verify we've got an empty tracker to work with.");
+  yield verifyTrackerEmpty();
+  try {
+    _("Clearing engine");
+    store.wipe();
+    tracker.clearChangedIDs();
+    yield resetChangedBookmarks();
+    tracker.resetScore();
+    yield stopTracking();
+
+    _("Create a folder");
+    let folder = PlacesUtils.bookmarks.createFolder(
+      PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent",
+      PlacesUtils.bookmarks.DEFAULT_INDEX);
+    let folderGUID = engine._store.GUIDForId(folder);
+    _("Folder ID: " + folder);
+    _("Folder GUID: " + folderGUID);
+
+    _("Track changes to tags");
+    let uri = Utils.makeURI("http://getfirefox.com");
+    let b = PlacesUtils.bookmarks.insertBookmark(
+      folder, uri,
+      PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
+    let bGUID = engine._store.GUIDForId(b);
+    _("New item is " + b);
+    _("GUID: " + bGUID);
+
+    yield startTracking();
+
+    _("Tag the item");
+    PlacesUtils.tagging.tagURI(uri, ["foo"]);
+
+    // bookmark should be tracked, folder should not be.
+    yield verifyTrackedItems([bGUID]);
+    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
+
+  } finally {
+    _("Clean up.");
+    yield cleanup();
+  }
+});
+
+add_task(function* test_onItemKeywordChanged() {
+  _("Verify we've got an empty tracker to work with.");
+  yield verifyTrackerEmpty();
+  try {
+    yield stopTracking();
+    let folder = PlacesUtils.bookmarks.createFolder(
+      PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent",
+      PlacesUtils.bookmarks.DEFAULT_INDEX);
+    let folderGUID = engine._store.GUIDForId(folder);
+    _("Track changes to keywords");
+    let uri = Utils.makeURI("http://getfirefox.com");
+    let b = PlacesUtils.bookmarks.insertBookmark(
+      folder, uri,
+      PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
+    let bGUID = engine._store.GUIDForId(b);
+    _("New item is " + b);
+    _("GUID: " + bGUID);
+
+    yield startTracking();
+
+    _("Give the item a keyword");
+    PlacesUtils.bookmarks.setKeywordForBookmark(b, "the_keyword");
+
+    // bookmark should be tracked, folder should not be.
+    yield verifyTrackedItems([bGUID]);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
 
   } finally {
     _("Clean up.");
-    store.wipe();
+    yield cleanup();
+  }
+});
+
+add_task(function* test_onItemMoved() {
+  _("Verify we've got an empty tracker to work with.");
+  yield verifyTrackerEmpty();
+  try {
+    let fx_id = PlacesUtils.bookmarks.insertBookmark(
+      PlacesUtils.bookmarks.bookmarksMenuFolder,
+      Utils.makeURI("http://getfirefox.com"),
+      PlacesUtils.bookmarks.DEFAULT_INDEX,
+      "Get Firefox!");
+    let fx_guid = engine._store.GUIDForId(fx_id);
+    _("Firefox GUID: " + fx_guid)
+    let tb_id = PlacesUtils.bookmarks.insertBookmark(
+      PlacesUtils.bookmarks.bookmarksMenuFolder,
+      Utils.makeURI("http://getthunderbird.com"),
+      PlacesUtils.bookmarks.DEFAULT_INDEX,
+      "Get Thunderbird!");
+    let tb_guid = engine._store.GUIDForId(tb_id);
+    _("Thunderbird GUID: " + tb_guid);
+
+    yield startTracking();
+
+    // Moving within the folder will just track the folder.
+    PlacesUtils.bookmarks.moveItem(
+      tb_id, PlacesUtils.bookmarks.bookmarksMenuFolder, 0);
+    yield verifyTrackedItems(['menu']);
+    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
     tracker.clearChangedIDs();
     tracker.resetScore();
-    Svc.Obs.notify("weave:engine:stop-tracking");
+
+    // Moving a bookmark to a different folder will track the old
+    // folder, the new folder and the bookmark.
+    PlacesUtils.bookmarks.moveItem(tb_id, PlacesUtils.bookmarks.toolbarFolder,
+                                   PlacesUtils.bookmarks.DEFAULT_INDEX);
+    yield verifyTrackedItems(['menu', 'toolbar', fx_guid]);
+    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 3);
+
+  } finally {
+    _("Clean up.");
+    yield cleanup();
   }
-}
+});
+
+add_task(function* test_treeMoved() {
+  _("Verify that moving an entire tree of bookmarks tracks correctly.");
+  yield verifyTrackerEmpty();
+  try {
+    // Create a couple of parent folders.
+    let folder1_id = PlacesUtils.bookmarks.createFolder(
+      PlacesUtils.bookmarks.bookmarksMenuFolder,
+      "First test folder",
+      PlacesUtils.bookmarks.DEFAULT_INDEX);
+    let folder1_guid = engine._store.GUIDForId(folder1_id);
+
+    // A second folder in the first.
+    let folder2_id = PlacesUtils.bookmarks.createFolder(
+      folder1_id,
+      "Second test folder",
+      PlacesUtils.bookmarks.DEFAULT_INDEX);
+    let folder2_guid = engine._store.GUIDForId(folder2_id);
 
-function test_onItemMoved() {
+    // Create a couple of bookmarks in the second folder.
+    let fx_id = PlacesUtils.bookmarks.insertBookmark(
+      folder2_id,
+      Utils.makeURI("http://getfirefox.com"),
+      PlacesUtils.bookmarks.DEFAULT_INDEX,
+      "Get Firefox!");
+    let fx_guid = engine._store.GUIDForId(fx_id);
+    let tb_id = PlacesUtils.bookmarks.insertBookmark(
+      folder2_id,
+      Utils.makeURI("http://getthunderbird.com"),
+      PlacesUtils.bookmarks.DEFAULT_INDEX,
+      "Get Thunderbird!");
+    let tb_guid = engine._store.GUIDForId(tb_id);
+
+    yield startTracking();
+
+    // Move folder 2 to be a sibling of folder1.
+    PlacesUtils.bookmarks.moveItem(
+      folder2_id, PlacesUtils.bookmarks.bookmarksMenuFolder, 0);
+    // the menu and both folders should be tracked, the children should not be.
+    yield verifyTrackedItems(['menu', folder1_guid, folder2_guid]);
+    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 3);
+  } finally {
+    _("Clean up.");
+    yield cleanup();
+  }
+});
+
+add_task(function* test_onItemDeleted() {
   _("Verify we've got an empty tracker to work with.");
-  let tracker = engine._tracker;
-  do_check_empty(tracker.changedIDs);
-  do_check_eq(tracker.score, 0);
-
+  yield verifyTrackerEmpty();
   try {
     let fx_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       Utils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
     let fx_guid = engine._store.GUIDForId(fx_id);
     let tb_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       Utils.makeURI("http://getthunderbird.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Thunderbird!");
     let tb_guid = engine._store.GUIDForId(tb_id);
 
-    Svc.Obs.notify("weave:engine:start-tracking");
+    yield startTracking();
+
+    // Delete the last item - the item and parent should be tracked.
+    PlacesUtils.bookmarks.removeItem(tb_id);
+
+    yield verifyTrackedItems(['menu', tb_guid]);
+    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 2);
+  } finally {
+    _("Clean up.");
+    yield cleanup();
+  }
+});
+
+add_task(function* test_onItemDeleted_removeFolderChildren() {
+  _("Verify we've got an empty tracker to work with.");
+  yield verifyTrackerEmpty();
 
-    // Moving within the folder will just track the folder.
-    PlacesUtils.bookmarks.moveItem(
-      tb_id, PlacesUtils.bookmarks.bookmarksMenuFolder, 0);
-    do_check_true(tracker.changedIDs['menu'] > 0);
-    do_check_eq(tracker.changedIDs['toolbar'], undefined);
-    do_check_eq(tracker.changedIDs[fx_guid], undefined);
-    do_check_eq(tracker.changedIDs[tb_guid], undefined);
-    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
-    tracker.clearChangedIDs();
-    tracker.resetScore();
+  try {
+    let mobileRoot = BookmarkSpecialIds.findMobileRoot(true);
+    let mobileGUID = engine._store.GUIDForId(mobileRoot);
+    let fx_id = PlacesUtils.bookmarks.insertBookmark(
+      mobileRoot,
+      Utils.makeURI("http://getfirefox.com"),
+      PlacesUtils.bookmarks.DEFAULT_INDEX,
+      "Get Firefox!");
+    let fx_guid = engine._store.GUIDForId(fx_id);
+    _(`Firefox GUID: ${fx_guid}`);
 
-    // Moving a bookmark to a different folder will track the old
-    // folder, the new folder and the bookmark.
-    PlacesUtils.bookmarks.moveItem(tb_id, PlacesUtils.bookmarks.toolbarFolder,
-                                   PlacesUtils.bookmarks.DEFAULT_INDEX);
-    do_check_true(tracker.changedIDs['menu'] > 0);
-    do_check_true(tracker.changedIDs['toolbar'] > 0);
-    do_check_eq(tracker.changedIDs[fx_guid], undefined);
-    do_check_true(tracker.changedIDs[tb_guid] > 0);
-    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 3);
+    let tb_id = PlacesUtils.bookmarks.insertBookmark(
+      mobileRoot,
+      Utils.makeURI("http://getthunderbird.com"),
+      PlacesUtils.bookmarks.DEFAULT_INDEX,
+      "Get Thunderbird!");
+    let tb_guid = engine._store.GUIDForId(tb_id);
+    _(`Thunderbird GUID: ${tb_guid}`);
 
+    let moz_id = PlacesUtils.bookmarks.insertBookmark(
+      PlacesUtils.bookmarks.bookmarksMenuFolder,
+      Utils.makeURI("https://mozilla.org"),
+      PlacesUtils.bookmarks.DEFAULT_INDEX,
+      "Mozilla"
+    );
+    let moz_guid = engine._store.GUIDForId(moz_id);
+    _(`Mozilla GUID: ${moz_guid}`);
+
+    yield startTracking();
+
+    _(`Mobile root ID: ${mobileRoot}`);
+    PlacesUtils.bookmarks.removeFolderChildren(mobileRoot);
+
+    yield verifyTrackedItems([mobileGUID, fx_guid, tb_guid]);
+    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 4);
   } finally {
     _("Clean up.");
-    store.wipe();
-    tracker.clearChangedIDs();
-    tracker.resetScore();
-    Svc.Obs.notify("weave:engine:stop-tracking");
+    yield cleanup();
   }
+});
 
-}
+add_task(function* test_onItemDeleted_tree() {
+  _("Verify we've got an empty tracker to work with.");
+  yield verifyTrackerEmpty();
+
+  try {
+    // Create a couple of parent folders.
+    let folder1_id = PlacesUtils.bookmarks.createFolder(
+      PlacesUtils.bookmarks.bookmarksMenuFolder,
+      "First test folder",
+      PlacesUtils.bookmarks.DEFAULT_INDEX);
+    let folder1_guid = engine._store.GUIDForId(folder1_id);
+
+    // A second folder in the first.
+    let folder2_id = PlacesUtils.bookmarks.createFolder(
+      folder1_id,
+      "Second test folder",
+      PlacesUtils.bookmarks.DEFAULT_INDEX);
+    let folder2_guid = engine._store.GUIDForId(folder2_id);
+
+    // Create a couple of bookmarks in the second folder.
+    let fx_id = PlacesUtils.bookmarks.insertBookmark(
+      folder2_id,
+      Utils.makeURI("http://getfirefox.com"),
+      PlacesUtils.bookmarks.DEFAULT_INDEX,
+      "Get Firefox!");
+    let fx_guid = engine._store.GUIDForId(fx_id);
+    let tb_id = PlacesUtils.bookmarks.insertBookmark(
+      folder2_id,
+      Utils.makeURI("http://getthunderbird.com"),
+      PlacesUtils.bookmarks.DEFAULT_INDEX,
+      "Get Thunderbird!");
+    let tb_guid = engine._store.GUIDForId(tb_id);
+
+    yield startTracking();
+
+    // Delete folder2 - everything we created should be tracked.
+    PlacesUtils.bookmarks.removeItem(folder2_id);
+
+    yield verifyTrackedItems([fx_guid, tb_guid, folder1_guid, folder2_guid]);
+    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 6);
+  } finally {
+    _("Clean up.");
+    yield cleanup();
+  }
+});
+
+// TODO(kitcambridge): Add tests for setItemDateAdded, setItemLastModified,
+// setItemIndex, changeBookmarkURI, insertSeparator, getRemoveFolderTransaction.
+
+// Add tests for Bookmarks.jsm queries.
+// Add a test for a page annotation (`OnPageChanged` can update favicons); make
+// sure these are ignored.
+
+// Test session annotations via TOPIC_SIMULATE_PLACES_SHUTDOWN. Make sure we
+// track changes on shutdown. This is new behavior; I don't think the current
+// tracker handles this.
+
+// Add tests for SYNC_STATUS_UNKNOWN and bookmarks restore.
 
 function run_test() {
   initTestLogging("Trace");
 
   Log.repository.getLogger("Sync.Engine.Bookmarks").level = Log.Level.Trace;
   Log.repository.getLogger("Sync.Store.Bookmarks").level = Log.Level.Trace;
   Log.repository.getLogger("Sync.Tracker.Bookmarks").level = Log.Level.Trace;
 
-  test_tracking();
-  test_onItemChanged();
-  test_onItemMoved();
+  run_next_test();
 }