Clean up tests per @asutherland

data/lib/mailapi/jobmixins.js

@@ -439,7 +439,8 @@ exports.do_downloadBodyReps = function(op, callback) {
       if (!body.bodyReps.every(function(rep) { return rep.isDownloaded; })) {
         folderConn.downloadBodyReps(header, onDownloadReps);
       } else {
-        onDownloadReps(null, body, true);
+        // passing flushed = true because we don't need to save anything
+        onDownloadReps(null, body, /* flushed = */ true);
       }
     });
   };

data/lib/mailapi/pop3/account.js

@@ -58,7 +58,7 @@ var properties = {
    * abstracts the `done` callback.)
    * @param {function(err, conn, done)} cb
    */
-  withConnection: function(cb) {
+  withConnection: function(cb, whyLabel) {
     // This implementation serializes withConnection requests so that
     // we don't step on requests' toes. While Pop3Client wouldn't mix
     // up the requests themselves, interleaving different operations
@@ -76,7 +76,7 @@ var properties = {
           }
         }.bind(this);
         if (!this._conn || this._conn.state === 'disconnected') {
-          this._makeConnection(next);
+          this._makeConnection(next, whyLabel);
         } else {
           next();
         }
@@ -225,7 +225,7 @@ var properties = {
     }
     this.withConnection(function(err) {
       callback(err);
-    });
+    }, 'checkAccount');
   },
 
   /**

data/lib/mailapi/pop3/sync.js

@@ -37,8 +37,9 @@ exports.Pop3FolderSyncer = Pop3FolderSyncer;
  *
  * @param {boolean} getNew If a fresh connection should always be made.
  * @param {int} cbIndex Index of the parent function's callback in args
+ * @param {string} whyLabel Description for why we need the connection
  */
-function lazyWithConnection(getNew, cbIndex, fn) {
+function lazyWithConnection(getNew, cbIndex, whyLabel, fn) {
   return function pop3LazyWithConnection() {
     var args = Array.slice(arguments);
     require([], function () {
@@ -61,7 +62,7 @@ function lazyWithConnection(getNew, cbIndex, fn) {
             };
             fn.apply(this, [conn].concat(args));
           }
-        }.bind(this));
+        }.bind(this), whyLabel);
       }.bind(this);
 
       // if we require a fresh connection, close out the old one first.
@@ -90,6 +91,7 @@ Pop3FolderSyncer.prototype = {
    * body part/message downloading. XXX rename this family of methods.
    */
   downloadBodies: lazyWithConnection(/* getNew = */ false, /* cbIndex = */ 2,
+    /* whyLabel = */ 'downloadBodies',
   function(conn, headers, options, callback) {
     var latch = allback.latch();
     var storage = this.storage;
@@ -117,6 +119,7 @@ Pop3FolderSyncer.prototype = {
    * all in one go.
    */
   downloadBodyReps: lazyWithConnection(/* getNew = */ false, /* cbIndex = */ 2,
+    /* whyLabel = */ 'downloadBodyReps',
   function(conn, header, options, callback) {
     if (options instanceof Function) {
       callback = options;
@@ -445,6 +448,7 @@ Pop3FolderSyncer.prototype = {
    * `this.storeMessageUidlForMessageId`).
    */
   sync: lazyWithConnection(/* getNew = */ true, /* cbIndex = */ 2,
+  /* whyLabel = */ 'sync',
   function(conn, syncType, slice, doneCallback, progressCallback) {
     // if we could not establish a connection, abort the sync.
     var self = this;

test/unit/test_downloadbodyreps_idempotency.js

@@ -14,7 +14,10 @@ TD.commonCase('fetch only snippets', function(T, RT) {
   // Create a folder to test on
   var eLazy = T.lazyLogger('misc');
   var folderName = 'test_downloadbodyreps_idempotency';
-  var messageCount = 3;
+  // We want just one message in the inbox; IMAP already adds one for
+  // tests so that we can detect timezones, so only add one for other
+  // account types.
+  var messageCount = (testAccount.type === 'imap' ? 0 : 1);
 
   // Set POP3 to not retrieve any of the message when fetching
   // headers. Otherwise it might have already finished downloading
@@ -26,7 +29,9 @@ TD.commonCase('fetch only snippets', function(T, RT) {
 
   // Use the inbox, so that POP3 will actually run its sync logic.
   var testFolder = testAccount.do_useExistingFolderWithType('inbox', '');
-  testAccount.do_addMessagesToFolder(testFolder, { count: messageCount });
+  if (messageCount > 0) {
+    testAccount.do_addMessagesToFolder(testFolder, { count: messageCount });
+  }
   var testView = testAccount.do_openFolderView(
     'syncs', testFolder, null, null, { syncedToDawnOfTime: 'ignore' });
 
@@ -36,44 +41,64 @@ TD.commonCase('fetch only snippets', function(T, RT) {
   // guarantee that the frontend doesn't have to worry about
   // spurious/redundant body "onchange" notifications.
   T.action('request full body after snippets', eLazy, function() {
-    eLazy.expectUseSetMatching();
+    // only the first job will actually download the bodies, the other
+    // jobs will still happen but will turn into no-ops
+    // this might need conn: true/etc.
 
-    testView.slice.items.forEach(function(header, idx) {
-      var whichCall = 0;
+    // We need three of these: Two for "downloadBodyReps" calls, and
+    // one for the "withBodyReps" call. Only the first will actually
+    // cause us to download and save the bodyReps.
+    testAccount.expect_runOp('downloadBodyReps', {
+      local: false, server: true, save: 'server' });
+    testAccount.expect_runOp('downloadBodyReps', {
+      local: false, server: true, save: false });
+    testAccount.expect_runOp('downloadBodyReps', {
+      local: false, server: true, save: false });
 
-      // The first call should receive a modified onchange event.
-      eLazy.expect_value('modified-' + idx);
-      // Then we called getBody twice, so we should see two more
-      // "done" events _without_ seeing more change events.
-      eLazy.expect_value('done-' + idx);
-      eLazy.expect_value('done-' + idx);
+    // TODO: POP3's logging (or th_main helper utilities) should be
+    // cleaned up to handle connection expectations in a way that lets
+    // us use the same expectations for IMAP as POP3. In this case,
+    // POP3 sets "USES_CONN = false", which renders the expect_runOp's
+    // "conn" helper useless. For IMAP, we already have a connection
+    // from the do_openFolderView, so we don't need conn: true here.
+    // But for POP3, the following expectations are needed to perform
+    // the same function as what "conn: true" woul dhave done in
+    // expect_runOp above, if POP3 supported that properly.
+    if (testAccount.type === 'pop3') {
+      RT.reportActiveActorThisStep(testAccount.eFolderAccount);
+      testAccount.eFolderAccount.ignore_createConnection();
+      testAccount.eFolderAccount.ignore_saveAccountState();
+    }
 
-      function gotBody(body) {
-        whichCall++;
-        if (whichCall === 1) {
-          // Attach the handler for this body here; it should only be
-          // called once even though we're calling getBody multiple
-          // times.
-          body.onchange = function() {
-            eLazy.value('modified-' + idx);
-          }
-        } else {
-          header.getBody({ withBodyReps: true }, function() {
-            eLazy.value('done-' + idx);
-          });
-        }
+    // there's only one message in the inbox
+    var header = testView.slice.items[0];
+
+    // The first call should receive a modified onchange event.
+    eLazy.expect_value('modified');
+    // Then we called getBody twice, so we should see two more
+    // "done" events _without_ seeing more change events.
+    eLazy.expect_value('done');
+
+    // Fetch the body thrice; the first will generate onchange;
+    // the other two should just indicate that we've finished.
+    header.getBody({ downloadBodyReps: true }, function (body) {
+      // Attach the handler for this body here; it should only be
+      // called once even though we're calling getBody multiple
+      // times.
+      body.onchange = function() {
+        eLazy.value('modified');
       }
+    });
 
-      // Fetch the body thrice; the first will generate onchange;
-      // the other two should just indicate that we've finished.
-      header.getBody({ downloadBodyReps: true }, gotBody);
-      header.getBody({ downloadBodyReps: true }, gotBody);
-      header.getBody({ downloadBodyReps: true }, gotBody);
+    header.getBody({ downloadBodyReps: true }, function(body) {
+      // Use { withBodyReps: true } so that the 'done' event
+      // happens after we see onchange.
+      header.getBody({ withBodyReps: true }, function() {
+        eLazy.value('done');
+      });
     });
   });
 
-//  testAccount.do_closeFolderView(testView);
-
 });
 
 }); // end define