Bug 987275 - Make test_deleted_items more resistant against timing issues. r=redDragon

calendar/base/src/calDeletedItems.js

@@ -15,6 +15,12 @@ Components.utils.import("resource:///modules/FileUtils.jsm");
  */
 function calDeletedItems() {
     this.wrappedJSObject = this;
+
+    this.completedNotifier = {
+      handleResult: function() {},
+      handleError: function() {},
+      handleCompletion: function() {},
+    };
 }
 
 const calDeletedItemsClassID = Components.ID("{8e6799af-e7e9-4e6c-9a82-a2413e86d8c3}");
@@ -38,10 +44,15 @@ calDeletedItems.prototype = {
     DB_SCHEMA_VERSION: 1,
     STALE_TIME: 30 * 24 * 60 * 60 / 1000, /* 30 days */
 
+    // To make the tests more failsafe, we have an internal notifier function.
+    // As the deleted items store is just meant to be a hint, this should not
+    // be used in real code.
+    completedNotifier: null,
+
     flush: function flush() {
         this.ensureStatements();
         this.stmtFlush.params.stale_time = cal.now().nativeTime - this.STALE_TIME;
-        this.stmtFlush.executeAsync();
+        this.stmtFlush.executeAsync(this.completedNotifier);
     },
 
     getDeletedDate: function calDeletedItems_getDeleted(aId, aCalId) {
@@ -75,13 +86,13 @@ calDeletedItems.prototype = {
         this.stmtMarkDelete.params.id = aItem.id;
         this.stmtMarkDelete.params.time = cal.now().nativeTime;
         this.stmtMarkDelete.params.rid = (aItem.recurrenceId && aItem.recurrenceId.nativeTime) || "";
-        this.stmtMarkDelete.executeAsync();
+        this.stmtMarkDelete.executeAsync(this.completedNotifier);
     },
 
     unmarkDeleted: function calDeletedItems_unmarkDeleted(aItem) {
         this.ensureStatements();
         this.stmtUnmarkDelete.params.id = aItem.id;
-        this.stmtUnmarkDelete.executeAsync();
+        this.stmtUnmarkDelete.executeAsync(this.completedNotifier);
     },
 
     initDB: function initDB() {

calendar/test/unit/test_deleted_items.js

@@ -2,28 +2,47 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+Components.utils.import("resource://gre/modules/Promise.jsm");
+
 function run_test() {
     do_get_profile();
     let delmgr = Components.classes["@mozilla.org/calendar/deleted-items-manager;1"]
                            .getService(Components.interfaces.calIDeletedItems);
     delmgr.observe(null, "profile-after-change", null);
 
-    add_test(test_deleted_items);
     cal.getCalendarManager().startup({ onResult: function() {
         run_next_test();
     }});
 }
 
-function test_deleted_items() {
+function check_delmgr_call(aFunc) {
+    const mISSC = Components.interfaces.mozIStorageStatementCallback;
+    let delmgr = Components.classes["@mozilla.org/calendar/deleted-items-manager;1"]
+                           .getService(Components.interfaces.calIDeletedItems);
+    let deferred = Promise.defer();
+    delmgr.wrappedJSObject.completedNotifier.handleCompletion = (aReason) => {
+        if (aReason == mISSC.REASON_FINISHED) {
+          deferred.resolve();
+        } else {
+          deferred.reject(aReason);
+        }
+    };
+
+    aFunc();
+    return deferred;
+}
+
+add_task(function test_deleted_items() {
     let calmgr = cal.getCalendarManager();
     let delmgr = Components.classes["@mozilla.org/calendar/deleted-items-manager;1"]
                            .getService(Components.interfaces.calIDeletedItems);
-    // No items have been deleted, retrieving one should return null
+    // No items have been deleted, retrieving one should return null.
     do_check_null(delmgr.getDeletedDate("random"));
     do_check_null(delmgr.getDeletedDate("random", "random"));
 
-    // This shouldn't throw anything
-    delmgr.flush();
+    // Make sure the cache is initially flushed and that this doesn't throw an
+    // error.
+    yield check_delmgr_call(function() delmgr.flush());
 
     let memory = calmgr.createCalendar("memory", Services.io.newURI("moz-storage-calendar://", null, null));
     calmgr.registerCalendar(memory);
@@ -33,11 +52,12 @@ function test_deleted_items() {
     item.startDate = cal.now();
     item.endDate = cal.now();
 
-    memory.addItem(item, null);
+    // Add the item, it still shouldn't be in the deleted database.
+    yield check_delmgr_call(function() memory.addItem(item, null));
     do_check_null(delmgr.getDeletedDate(item.id));
     do_check_null(delmgr.getDeletedDate(item.id, memory.id));
 
-    // We need to stop time so we have something to compare with
+    // We need to stop time so we have something to compare with.
     let referenceDate = cal.createDateTime("20120726T112045"); referenceDate.timezone = cal.calendarDefaultTimezone();
     let futureDate = cal.createDateTime("20380101T000000");  futureDate.timezone = cal.calendarDefaultTimezone();
     let useFutureDate = false;
@@ -46,55 +66,39 @@ function test_deleted_items() {
         return (useFutureDate ? futureDate : referenceDate).clone();
     }
 
-    // Deleting an item should trigger it being marked for deletion
-    memory.deleteItem(item, null);
+    // Deleting an item should trigger it being marked for deletion.
+    yield check_delmgr_call(function() memory.deleteItem(item, null));
 
     // Now check if it was deleted at our reference date.
     let deltime = delmgr.getDeletedDate(item.id);
     do_check_neq(deltime, null);
     do_check_eq(deltime.compare(referenceDate), 0);
 
-    // The same with the calendar
+    // The same with the calendar.
     deltime = delmgr.getDeletedDate(item.id, memory.id);
     do_check_neq(deltime, null);
     do_check_eq(deltime.compare(referenceDate), 0);
 
-    // Item should not be found in other calendars
+    // Item should not be found in other calendars.
     do_check_null(delmgr.getDeletedDate(item.id, "random"));
 
-    // Check if flushing works, we need to travel time for that
+    // Check if flushing works, we need to travel time for that.
     useFutureDate = true;
-    delmgr.flush();
+    yield check_delmgr_call(function() delmgr.flush());
     do_check_null(delmgr.getDeletedDate(item.id));
     do_check_null(delmgr.getDeletedDate(item.id, memory.id));
 
-    // Deleting an item and adding it again should consider it not deleted. The
-    // deleted items manager runs the statements asynchronously. Timeouts are
-    // usually a bad way to test, but since the deleted items manager is only
-    // used as a hint, the sql calls in the observers are async.
-    // *** If this test is failing, the timeouts might not be enough ***
+    // Start over with our past time.
     useFutureDate = false;
 
-    function doFirstAdd() {
-        memory.addItem(item, null);
-        do_timeout(100, doRemove);
-    }
-    function doRemove() {
-        memory.deleteItem(item, null);
-        do_timeout(100, doSecondAdd);
-    }
-    function doSecondAdd() {
-        memory.addItem(item, null);
-        do_timeout(100, doCleanup);
-    }
-    function doCleanup() {
-        do_check_null(delmgr.getDeletedDate(item.id));
-        // Revert now function, in case more tests are written
-        cal.now = oldNowFunction;
-
-        run_next_test();
-    }
-
-    doFirstAdd();
-}
+    // Add, delete, add. Item should no longer be deleted.
+    yield check_delmgr_call(function() memory.addItem(item, null));
+    do_check_null(delmgr.getDeletedDate(item.id));
+    yield check_delmgr_call(function() memory.deleteItem(item, null));
+    do_check_eq(delmgr.getDeletedDate(item.id).compare(referenceDate), 0);
+    yield check_delmgr_call(function() memory.addItem(item, null));
+    do_check_null(delmgr.getDeletedDate(item.id));
 
+    // Revert now function, in case more tests are written.
+    cal.now = oldNowFunction;
+});