From 4be3bc1ec8a52308ee1998f0a31d7e491e06b3c1 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Fri, 13 Aug 2010 15:28:23 -0700 Subject: [PATCH] Properly unregister mock controllers Some tests create mock controllers. They register themselves to MessagingController when instantiated, but never unregister. Added a cleanup method, and call it for each instance. (I was hoping it would spped up unit tests, but it didn't. Still it's a nice thing to do.) Change-Id: Ia90f0380aef388d22f7cfcf6e9203e05444b3285 --- src/com/android/email/Controller.java | 10 ++ .../email/ControllerProviderOpsTests.java | 124 ++++++++---------- .../email/activity/MailboxFinderTest.java | 1 + .../email/service/MailServiceTests.java | 52 ++++---- 4 files changed, 93 insertions(+), 94 deletions(-) diff --git a/src/com/android/email/Controller.java b/src/com/android/email/Controller.java index 1ee5bd283..9bdb333fc 100644 --- a/src/com/android/email/Controller.java +++ b/src/com/android/email/Controller.java @@ -96,6 +96,16 @@ public class Controller { mLegacyController.addListener(mLegacyListener); } + /** + * Cleanup for test. Mustn't be called for the regular {@link Controller}, as it's a + * singleton and lives till the process finishes. + * + *

However, this method MUST be called for mock instances. + */ + public void cleanupForTest() { + mLegacyController.removeListener(mLegacyListener); + } + /** * Gets or creates the singleton instance of Controller. */ diff --git a/tests/src/com/android/email/ControllerProviderOpsTests.java b/tests/src/com/android/email/ControllerProviderOpsTests.java index d60a8d4d7..7b0994bdb 100644 --- a/tests/src/com/android/email/ControllerProviderOpsTests.java +++ b/tests/src/com/android/email/ControllerProviderOpsTests.java @@ -16,24 +16,19 @@ package com.android.email; -import com.android.email.mail.MessagingException; -import com.android.email.mail.transport.Rfc822Output; import com.android.email.provider.EmailContent; -import com.android.email.provider.EmailProvider; -import com.android.email.provider.ProviderTestUtils; import com.android.email.provider.EmailContent.Account; import com.android.email.provider.EmailContent.Body; import com.android.email.provider.EmailContent.HostAuth; import com.android.email.provider.EmailContent.Mailbox; import com.android.email.provider.EmailContent.Message; +import com.android.email.provider.EmailProvider; +import com.android.email.provider.ProviderTestUtils; import android.content.Context; import android.net.Uri; import android.test.ProviderTestCase2; -import java.io.File; -import java.io.FileOutputStream; -import java.io.IOException; import java.util.Locale; /** @@ -48,9 +43,10 @@ import java.util.Locale; */ public class ControllerProviderOpsTests extends ProviderTestCase2 { - EmailProvider mProvider; - Context mProviderContext; - Context mContext; + private Context mProviderContext; + private Context mContext; + private TestController mTestController; + public ControllerProviderOpsTests() { super(EmailProvider.class, EmailProvider.EMAIL_AUTHORITY); @@ -61,11 +57,13 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 super.setUp(); mProviderContext = getMockContext(); mContext = getContext(); + mTestController = new TestController(mProviderContext, mContext); } @Override public void tearDown() throws Exception { super.tearDown(); + mTestController.cleanupForTest(); } /** @@ -83,24 +81,22 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 * These are strings that should not change per locale. */ public void testGetMailboxServerName() { - Controller ct = new TestController(mProviderContext, mContext); + assertEquals("", mTestController.getMailboxServerName(-1)); - assertEquals("", ct.getMailboxServerName(-1)); - - assertEquals("Inbox", ct.getMailboxServerName(Mailbox.TYPE_INBOX)); - assertEquals("Outbox", ct.getMailboxServerName(Mailbox.TYPE_OUTBOX)); - assertEquals("Trash", ct.getMailboxServerName(Mailbox.TYPE_TRASH)); - assertEquals("Sent", ct.getMailboxServerName(Mailbox.TYPE_SENT)); - assertEquals("Junk", ct.getMailboxServerName(Mailbox.TYPE_JUNK)); + assertEquals("Inbox", mTestController.getMailboxServerName(Mailbox.TYPE_INBOX)); + assertEquals("Outbox", mTestController.getMailboxServerName(Mailbox.TYPE_OUTBOX)); + assertEquals("Trash", mTestController.getMailboxServerName(Mailbox.TYPE_TRASH)); + assertEquals("Sent", mTestController.getMailboxServerName(Mailbox.TYPE_SENT)); + assertEquals("Junk", mTestController.getMailboxServerName(Mailbox.TYPE_JUNK)); // Now try again with translation Locale savedLocale = Locale.getDefault(); Locale.setDefault(Locale.FRANCE); - assertEquals("Inbox", ct.getMailboxServerName(Mailbox.TYPE_INBOX)); - assertEquals("Outbox", ct.getMailboxServerName(Mailbox.TYPE_OUTBOX)); - assertEquals("Trash", ct.getMailboxServerName(Mailbox.TYPE_TRASH)); - assertEquals("Sent", ct.getMailboxServerName(Mailbox.TYPE_SENT)); - assertEquals("Junk", ct.getMailboxServerName(Mailbox.TYPE_JUNK)); + assertEquals("Inbox", mTestController.getMailboxServerName(Mailbox.TYPE_INBOX)); + assertEquals("Outbox", mTestController.getMailboxServerName(Mailbox.TYPE_OUTBOX)); + assertEquals("Trash", mTestController.getMailboxServerName(Mailbox.TYPE_TRASH)); + assertEquals("Sent", mTestController.getMailboxServerName(Mailbox.TYPE_SENT)); + assertEquals("Junk", mTestController.getMailboxServerName(Mailbox.TYPE_JUNK)); Locale.setDefault(savedLocale); } @@ -116,8 +112,7 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 long oldBoxId = Mailbox.findMailboxOfType(mProviderContext, accountId, Mailbox.TYPE_DRAFTS); assertEquals(Mailbox.NO_MAILBOX, oldBoxId); - Controller ct = new TestController(mProviderContext, mContext); - ct.createMailbox(accountId, Mailbox.TYPE_DRAFTS); + mTestController.createMailbox(accountId, Mailbox.TYPE_DRAFTS); long boxId = Mailbox.findMailboxOfType(mProviderContext, accountId, Mailbox.TYPE_DRAFTS); // check that the drafts mailbox exists @@ -141,23 +136,23 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 box.save(mProviderContext); long boxId = box.mId; - Controller ct = new TestController(mProviderContext, mContext); - long testBoxId = ct.findOrCreateMailboxOfType(accountId, boxType); + long testBoxId = mTestController.findOrCreateMailboxOfType(accountId, boxType); // check it found the right mailbox id assertEquals(boxId, testBoxId); - long boxId2 = ct.findOrCreateMailboxOfType(accountId, Mailbox.TYPE_DRAFTS); + long boxId2 = mTestController.findOrCreateMailboxOfType(accountId, Mailbox.TYPE_DRAFTS); assertTrue("mailbox created", boxId2 != Mailbox.NO_MAILBOX); assertTrue("with different id", testBoxId != boxId2); // check it doesn't create twice when existing - long boxId3 = ct.findOrCreateMailboxOfType(accountId, Mailbox.TYPE_DRAFTS); + long boxId3 = mTestController.findOrCreateMailboxOfType(accountId, Mailbox.TYPE_DRAFTS); assertEquals("don't create if exists", boxId3, boxId2); // check invalid aruments - assertEquals(Mailbox.NO_MAILBOX, ct.findOrCreateMailboxOfType(-1, Mailbox.TYPE_DRAFTS)); - assertEquals(Mailbox.NO_MAILBOX, ct.findOrCreateMailboxOfType(accountId, -1)); + assertEquals(Mailbox.NO_MAILBOX, + mTestController.findOrCreateMailboxOfType(-1, Mailbox.TYPE_DRAFTS)); + assertEquals(Mailbox.NO_MAILBOX, mTestController.findOrCreateMailboxOfType(accountId, -1)); } /** @@ -179,9 +174,7 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 true, mProviderContext); long message1Id = message1.mId; - Controller ct = new TestController(mProviderContext, mContext); - - ct.deleteMessage(message1Id, account1Id); + mTestController.deleteMessage(message1Id, account1Id); // now read back a fresh copy and confirm it's in the trash Message message1get = EmailContent.Message.restoreMessageWithId(mProviderContext, @@ -193,7 +186,7 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 true, mProviderContext); long message2Id = message2.mId; - ct.deleteMessage(message2Id, -1); + mTestController.deleteMessage(message2Id, -1); // now read back a fresh copy and confirm it's in the trash Message message2get = EmailContent.Message.restoreMessageWithId(mProviderContext, @@ -216,9 +209,7 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 mProviderContext); long message1Id = message1.mId; - Controller ct = new TestController(mProviderContext, mContext); - - ct.deleteMessage(message1Id, account1Id); + mTestController.deleteMessage(message1Id, account1Id); // now read back a fresh copy and confirm it's in the trash Message message1get = @@ -248,15 +239,13 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 mProviderContext); long message1Id = message1.mId; - Controller ct = new TestController(mProviderContext, mContext); - // test setting to "read" - ct.setMessageRead(message1Id, true); + mTestController.setMessageRead(message1Id, true); Message message1get = Message.restoreMessageWithId(mProviderContext, message1Id); assertTrue(message1get.mFlagRead); // test setting to "unread" - ct.setMessageRead(message1Id, false); + mTestController.setMessageRead(message1Id, false); message1get = Message.restoreMessageWithId(mProviderContext, message1Id); assertFalse(message1get.mFlagRead); } @@ -277,24 +266,21 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 mProviderContext); long message1Id = message1.mId; - Controller ct = new TestController(mProviderContext, mContext); - // test setting to "favorite" - ct.setMessageFavorite(message1Id, true); + mTestController.setMessageFavorite(message1Id, true); Message message1get = Message.restoreMessageWithId(mProviderContext, message1Id); assertTrue(message1get.mFlagFavorite); // test setting to "not favorite" - ct.setMessageFavorite(message1Id, false); + mTestController.setMessageFavorite(message1Id, false); message1get = Message.restoreMessageWithId(mProviderContext, message1Id); assertFalse(message1get.mFlagFavorite); } public void testGetAndDeleteAttachmentMailbox() { - Controller ct = new TestController(mProviderContext, mContext); - Mailbox box = ct.getAttachmentMailbox(); + Mailbox box = mTestController.getAttachmentMailbox(); assertNotNull(box); - Mailbox anotherBox = ct.getAttachmentMailbox(); + Mailbox anotherBox = mTestController.getAttachmentMailbox(); assertNotNull(anotherBox); // We should always get back the same Mailbox row assertEquals(box.mId, anotherBox.mId); @@ -307,7 +293,7 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 assertEquals(2, EmailContent.count(mProviderContext, Message.CONTENT_URI, Message.MAILBOX_KEY + "=?", new String[] {Long.toString(box.mId)})); // Delete them - ct.deleteAttachmentMessages(); + mTestController.deleteAttachmentMessages(); // Make sure they're gone assertEquals(0, EmailContent.count(mProviderContext, Message.CONTENT_URI, Message.MAILBOX_KEY + "=?", new String[] {Long.toString(box.mId)})); @@ -327,13 +313,12 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 mContext.getFilesDir()); // Load the message via Controller and a Uri - Controller ct = new TestController(mProviderContext, mContext); - Message loadedMsg = ct.loadMessageFromUri(fileUri); + Message loadedMsg = mTestController.loadMessageFromUri(fileUri); // Check server id, mailbox key, account key, and from assertNotNull(loadedMsg); assertTrue(loadedMsg.mServerId.startsWith(Controller.ATTACHMENT_MESSAGE_UID_PREFIX)); - Mailbox box = ct.getAttachmentMailbox(); + Mailbox box = mTestController.getAttachmentMailbox(); assertNotNull(box); assertEquals(box.mId, loadedMsg.mMailboxKey); assertEquals(0, loadedMsg.mAccountKey); @@ -353,42 +338,41 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 } public void testIsMessagingController() { - Controller ct = new TestController(mProviderContext, mContext); Account account1 = ProviderTestUtils.setupAccount("account1", false, mProviderContext); account1.mHostAuthRecv = setupSimpleHostAuth("eas"); account1.save(mProviderContext); - assertFalse(ct.isMessagingController(account1)); + assertFalse(mTestController.isMessagingController(account1)); Account account2 = ProviderTestUtils.setupAccount("account2", false, mProviderContext); account2.mHostAuthRecv = setupSimpleHostAuth("imap"); account2.save(mProviderContext); - assertTrue(ct.isMessagingController(account2)); + assertTrue(mTestController.isMessagingController(account2)); Account account3 = ProviderTestUtils.setupAccount("account3", false, mProviderContext); account3.mHostAuthRecv = setupSimpleHostAuth("pop3"); account3.save(mProviderContext); - assertTrue(ct.isMessagingController(account3)); + assertTrue(mTestController.isMessagingController(account3)); Account account4 = ProviderTestUtils.setupAccount("account4", false, mProviderContext); account4.mHostAuthRecv = setupSimpleHostAuth("smtp"); account4.save(mProviderContext); - assertFalse(ct.isMessagingController(account4)); + assertFalse(mTestController.isMessagingController(account4)); // There should be values for all of these accounts in the legacy map - assertNotNull(ct.mLegacyControllerMap.get(account1.mId)); - assertNotNull(ct.mLegacyControllerMap.get(account2.mId)); - assertNotNull(ct.mLegacyControllerMap.get(account3.mId)); - assertNotNull(ct.mLegacyControllerMap.get(account4.mId)); + assertNotNull(mTestController.mLegacyControllerMap.get(account1.mId)); + assertNotNull(mTestController.mLegacyControllerMap.get(account2.mId)); + assertNotNull(mTestController.mLegacyControllerMap.get(account3.mId)); + assertNotNull(mTestController.mLegacyControllerMap.get(account4.mId)); // The map should have the expected values - assertFalse(ct.mLegacyControllerMap.get(account1.mId)); - assertTrue(ct.mLegacyControllerMap.get(account2.mId)); - assertTrue(ct.mLegacyControllerMap.get(account3.mId)); - assertFalse(ct.mLegacyControllerMap.get(account4.mId)); + assertFalse(mTestController.mLegacyControllerMap.get(account1.mId)); + assertTrue(mTestController.mLegacyControllerMap.get(account2.mId)); + assertTrue(mTestController.mLegacyControllerMap.get(account3.mId)); + assertFalse(mTestController.mLegacyControllerMap.get(account4.mId)); // This second pass should pull values from the cache - assertFalse(ct.isMessagingController(account1)); - assertTrue(ct.isMessagingController(account2)); - assertTrue(ct.isMessagingController(account3)); - assertFalse(ct.isMessagingController(account4)); + assertFalse(mTestController.isMessagingController(account1)); + assertTrue(mTestController.isMessagingController(account2)); + assertTrue(mTestController.isMessagingController(account3)); + assertFalse(mTestController.isMessagingController(account4)); } /** diff --git a/tests/src/com/android/email/activity/MailboxFinderTest.java b/tests/src/com/android/email/activity/MailboxFinderTest.java index 5866a1fbd..fb0acfe4c 100644 --- a/tests/src/com/android/email/activity/MailboxFinderTest.java +++ b/tests/src/com/android/email/activity/MailboxFinderTest.java @@ -72,6 +72,7 @@ public class MailboxFinderTest extends InstrumentationTestCase { @Override protected void tearDown() throws Exception { super.tearDown(); + mMockController.cleanupForTest(); Controller.injectMockControllerForTest(null); } diff --git a/tests/src/com/android/email/service/MailServiceTests.java b/tests/src/com/android/email/service/MailServiceTests.java index 80ed13035..713e3bdb1 100644 --- a/tests/src/com/android/email/service/MailServiceTests.java +++ b/tests/src/com/android/email/service/MailServiceTests.java @@ -199,31 +199,35 @@ public class MailServiceTests extends AccountTestCase { // Setup the SyncReport's for these Accounts MailService mailService = new MailService(); mailService.mController = new TestController(mMockContext, getContext()); - mailService.setupSyncReportsLocked(-1, mMockContext.getContentResolver()); + try { + mailService.setupSyncReportsLocked(-1, mMockContext.getContentResolver()); - // Get back the map created by MailService - HashMap syncReportMap = MailService.mSyncReports; - // Check the SyncReport's for correctness of sync interval - AccountSyncReport syncReport = syncReportMap.get(easAccount.mId); - assertNotNull(syncReport); - // EAS sync interval should have been changed to "never" - assertEquals(Account.CHECK_INTERVAL_NEVER, syncReport.syncInterval); - syncReport = syncReportMap.get(imapAccount.mId); - assertNotNull(syncReport); - assertEquals(60, syncReport.syncInterval); - syncReport = syncReportMap.get(pop3Account.mId); - assertNotNull(syncReport); - assertEquals(90, syncReport.syncInterval); + // Get back the map created by MailService + HashMap syncReportMap = MailService.mSyncReports; + // Check the SyncReport's for correctness of sync interval + AccountSyncReport syncReport = syncReportMap.get(easAccount.mId); + assertNotNull(syncReport); + // EAS sync interval should have been changed to "never" + assertEquals(Account.CHECK_INTERVAL_NEVER, syncReport.syncInterval); + syncReport = syncReportMap.get(imapAccount.mId); + assertNotNull(syncReport); + assertEquals(60, syncReport.syncInterval); + syncReport = syncReportMap.get(pop3Account.mId); + assertNotNull(syncReport); + assertEquals(90, syncReport.syncInterval); - // Change the EAS account to push - ContentValues cv = new ContentValues(); - cv.put(Account.SYNC_INTERVAL, Account.CHECK_INTERVAL_PUSH); - easAccount.update(mMockContext, cv); - syncReportMap.clear(); - mailService.setupSyncReportsLocked(easAccount.mId, mMockContext.getContentResolver()); - syncReport = syncReportMap.get(easAccount.mId); - assertNotNull(syncReport); - // EAS sync interval should be "never" in this case as well - assertEquals(Account.CHECK_INTERVAL_NEVER, syncReport.syncInterval); + // Change the EAS account to push + ContentValues cv = new ContentValues(); + cv.put(Account.SYNC_INTERVAL, Account.CHECK_INTERVAL_PUSH); + easAccount.update(mMockContext, cv); + syncReportMap.clear(); + mailService.setupSyncReportsLocked(easAccount.mId, mMockContext.getContentResolver()); + syncReport = syncReportMap.get(easAccount.mId); + assertNotNull(syncReport); + // EAS sync interval should be "never" in this case as well + assertEquals(Account.CHECK_INTERVAL_NEVER, syncReport.syncInterval); + } finally { + mailService.mController.cleanupForTest(); + } } }