From 8b4fc30f4a660d1e99c964735f6f045d6cecbd07 Mon Sep 17 00:00:00 2001 From: Philip Whitehouse Date: Fri, 30 Sep 2016 22:51:36 +0200 Subject: [PATCH 1/6] IMAP: Clarify location of unimplemented functionality --- .../src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.java b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.java index 716b788ea..407d00c6a 100644 --- a/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.java +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.java @@ -496,7 +496,7 @@ class ImapFolder extends Folder { @Override public void delete(boolean recurse) throws MessagingException { - throw new Error("ImapStore.delete() not yet implemented"); + throw new Error("ImapFolder.delete() not yet implemented"); } @Override From 9a6009d8d483e206b5151b8830348b1b206f1ae8 Mon Sep 17 00:00:00 2001 From: Philip Whitehouse Date: Fri, 30 Sep 2016 22:52:04 +0200 Subject: [PATCH 2/6] IMAP: Test coverage improvements --- .../k9/mail/store/imap/ImapFolderTest.java | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java index d435c3243..f19d68a2b 100644 --- a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java +++ b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java @@ -42,6 +42,7 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -451,6 +452,24 @@ public class ImapFolderTest { } } + @Test + public void getUnreadMessageCount_connectionThrowsIOException_shouldThrowMessagingException() + throws Exception { + ImapFolder folder = createFolder("Folder"); + prepareImapFolderForOpen(OPEN_MODE_RW); + List imapResponses = singletonList(createImapResponse("* SEARCH 1 2 3")); + when(imapConnection.executeSimpleCommand("SEARCH 1:* UNSEEN NOT DELETED")) + .thenThrow(new IOException()); + folder.open(OPEN_MODE_RW); + + try { + folder.getUnreadMessageCount(); + fail("Expected exception"); + } catch (MessagingException e) { + assertEquals("IO Error", e.getMessage()); + } + } + @Test public void getUnreadMessageCount() throws Exception { ImapFolder folder = createFolder("Folder"); @@ -506,6 +525,36 @@ public class ImapFolderTest { assertEquals(42L, result); } + @Test + public void getHighestUid_imapConnectionThrowsNegativesResponse_shouldReturnMinus1() throws Exception { + ImapFolder folder = createFolder("Folder"); + prepareImapFolderForOpen(OPEN_MODE_RW); + List imapResponses = singletonList(createImapResponse("* SEARCH 42")); + when(imapConnection.executeSimpleCommand("UID SEARCH *:*")) + .thenThrow(NegativeImapResponseException.class); + folder.open(OPEN_MODE_RW); + + long result = folder.getHighestUid(); + + assertEquals(-1L, result); + } + + @Test + public void getHighestUid_imapConnectionThrowsIOException_shouldThrowMessagingException() throws Exception { + ImapFolder folder = createFolder("Folder"); + prepareImapFolderForOpen(OPEN_MODE_RW); + List imapResponses = singletonList(createImapResponse("* SEARCH 42")); + when(imapConnection.executeSimpleCommand("UID SEARCH *:*")) + .thenThrow(IOException.class); + folder.open(OPEN_MODE_RW); + + try { + folder.getHighestUid(); + } catch (MessagingException e) { + assertEquals("IO Error", e.getMessage()); + } + } + @Test public void getMessages_withoutDateConstraint() throws Exception { ImapFolder folder = createFolder("Folder"); @@ -724,6 +773,18 @@ public class ImapFolderTest { assertFalse(result); } + public void areMoreMessagesAvailable_withIndexOf1_shouldReturnFalseWithoutPerformingSearch() throws Exception { + ImapFolder folder = createFolder("Folder"); + prepareImapFolderForOpen(OPEN_MODE_RW); + folder.open(OPEN_MODE_RW); + + boolean result = folder.areMoreMessagesAvailable(1, null); + + verify(imapConnection, never()).executeSimpleCommand(anyString()); + + assertFalse(result); + } + @Test public void areMoreMessagesAvailable_withoutAdditionalMessages_shouldIssueSearchCommandsUntilAllMessagesSearched() throws Exception { @@ -1022,6 +1083,25 @@ public class ImapFolderTest { } } + @Test + public void delete_notImplemented() throws MessagingException { + ImapFolder folder = createFolder("Folder"); + try { + folder.delete(false); + fail("Error expected"); + } catch (Error e) { + } + } + + @Test + public void getMessageByUid_returnsNewImapMessage() throws MessagingException { + ImapFolder folder = createFolder("Folder"); + ImapMessage message = folder.getMessage("uid"); + assertNotNull(message); + assertEquals("uid", message.getUid()); + assertEquals(folder, message.getFolder()); + } + private Set extractMessageUids(List messages) { Set result = new HashSet<>(); for (Message message : messages) { From 0a7de4b645dec9f5f7ec20fb0d70c4d1f30c0b2c Mon Sep 17 00:00:00 2001 From: Philip Whitehouse Date: Sat, 1 Oct 2016 00:37:32 +0200 Subject: [PATCH 3/6] IMAP: Add proper test for fetchPart() --- .../k9/mail/store/imap/ImapFolderTest.java | 66 ++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java index f19d68a2b..d13fd956c 100644 --- a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java +++ b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java @@ -1,6 +1,9 @@ package com.fsck.k9.mail.store.imap; +import android.content.Context; + +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -11,6 +14,7 @@ import java.util.Map; import java.util.Set; import java.util.TimeZone; +import com.fsck.k9.mail.Body; import com.fsck.k9.mail.FetchProfile; import com.fsck.k9.mail.FetchProfile.Item; import com.fsck.k9.mail.Flag; @@ -20,11 +24,19 @@ import com.fsck.k9.mail.Message; import com.fsck.k9.mail.MessageRetrievalListener; import com.fsck.k9.mail.MessagingException; import com.fsck.k9.mail.Part; +import com.fsck.k9.mail.internet.BinaryTempFileBody; +import com.fsck.k9.mail.internet.MimeHeader; import com.fsck.k9.mail.store.StoreConfig; + +import org.apache.james.mime4j.util.MimeUtil; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; import static com.fsck.k9.mail.Folder.OPEN_MODE_RO; @@ -57,10 +69,12 @@ public class ImapFolderTest { private ImapStore imapStore; private ImapConnection imapConnection; private StoreConfig storeConfig; - + private Context context; @Before public void setUp() throws Exception { + context = RuntimeEnvironment.application; + BinaryTempFileBody.setTempDirectory(context.getCacheDir()); imapStore = mock(ImapStore.class); storeConfig = mock(StoreConfig.class); when(storeConfig.getInboxFolderName()).thenReturn("INBOX"); @@ -952,6 +966,56 @@ public class ImapFolderTest { verify(imapConnection).sendCommand("UID FETCH 1 (UID BODY.PEEK[1.1])", false); } + @Test + public void fetchPart_withTextSection_shouldProcessImapResponses() throws Exception { + ImapFolder folder = createFolder("Folder"); + prepareImapFolderForOpen(OPEN_MODE_RO); + folder.open(OPEN_MODE_RO); + ImapMessage message = createImapMessage("1"); + Part part = createPart("1.1"); + when(part.getHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING)).thenReturn( + new String[]{MimeUtil.ENC_7BIT} + ); + when(part.getHeader(MimeHeader.HEADER_CONTENT_TYPE)).thenReturn( + new String[]{"text/plain"} + ); + + when(imapConnection.readResponse(any(ImapResponseCallback.class))) + .thenAnswer(new Answer() { + @Override + public ImapResponse answer(InvocationOnMock invocation) throws Throwable { + ImapResponse response = ImapResponse.newContinuationRequest( + (ImapResponseCallback) invocation.getArguments()[0]); + response.add("1"); + response.add("FETCH"); + ImapList fetchList = new ImapList(); + fetchList.add("UID"); + fetchList.add("1"); + fetchList.add("BODY"); + fetchList.add("1.1"); + fetchList.add("text"); + response.add(fetchList); + return response; + } + }) + .thenAnswer(new Answer() { + @Override + public ImapResponse answer(InvocationOnMock invocation) throws Throwable { + ImapResponse response = ImapResponse.newTaggedResponse( + (ImapResponseCallback) invocation.getArguments()[0], "TAG"); + return response; + } + }); + + folder.fetchPart(message, part, null); + ArgumentCaptor bodyArgumentCaptor = ArgumentCaptor.forClass(Body.class); + verify(part).setBody(bodyArgumentCaptor.capture()); + ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); + bodyArgumentCaptor.getValue().writeTo(byteArrayOutputStream); + + assertEquals("text", new String(byteArrayOutputStream.toByteArray())); + } + @Test public void appendMessages_shouldIssueRespectiveCommand() throws Exception { ImapFolder folder = createFolder("Folder"); From dfc3212ca5e7abb6e46800ffc9f15aa0d01c2da8 Mon Sep 17 00:00:00 2001 From: Philip Whitehouse Date: Sat, 1 Oct 2016 12:32:38 +0200 Subject: [PATCH 4/6] IMAP: Finish test when index 1 passed to areMoreMessagesAvailable --- .../test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java index d13fd956c..45b075ab4 100644 --- a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java +++ b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java @@ -787,6 +787,7 @@ public class ImapFolderTest { assertFalse(result); } + @Test public void areMoreMessagesAvailable_withIndexOf1_shouldReturnFalseWithoutPerformingSearch() throws Exception { ImapFolder folder = createFolder("Folder"); prepareImapFolderForOpen(OPEN_MODE_RW); @@ -794,7 +795,8 @@ public class ImapFolderTest { boolean result = folder.areMoreMessagesAvailable(1, null); - verify(imapConnection, never()).executeSimpleCommand(anyString()); + //SELECT during OPEN + verify(imapConnection, times(1)).executeSimpleCommand(anyString()); assertFalse(result); } From 10b46e2f49066d28a105e36dfeae56f7fb4dfd02 Mon Sep 17 00:00:00 2001 From: Philip Whitehouse Date: Thu, 13 Oct 2016 23:14:40 +0100 Subject: [PATCH 5/6] Reformat to match coding standards --- .../k9/mail/store/imap/ImapFolderTest.java | 89 +++++++++++-------- 1 file changed, 52 insertions(+), 37 deletions(-) diff --git a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java index 45b075ab4..531b488a5 100644 --- a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java +++ b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java @@ -61,8 +61,6 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import static org.mockito.internal.util.collections.Sets.newSet; - -//TODO: Increase test coverage e.g. for fetch() and fetchPart() @RunWith(RobolectricTestRunner.class) @Config(manifest = Config.NONE, sdk = 21) public class ImapFolderTest { @@ -974,40 +972,10 @@ public class ImapFolderTest { prepareImapFolderForOpen(OPEN_MODE_RO); folder.open(OPEN_MODE_RO); ImapMessage message = createImapMessage("1"); - Part part = createPart("1.1"); - when(part.getHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING)).thenReturn( - new String[]{MimeUtil.ENC_7BIT} - ); - when(part.getHeader(MimeHeader.HEADER_CONTENT_TYPE)).thenReturn( - new String[]{"text/plain"} - ); - when(imapConnection.readResponse(any(ImapResponseCallback.class))) - .thenAnswer(new Answer() { - @Override - public ImapResponse answer(InvocationOnMock invocation) throws Throwable { - ImapResponse response = ImapResponse.newContinuationRequest( - (ImapResponseCallback) invocation.getArguments()[0]); - response.add("1"); - response.add("FETCH"); - ImapList fetchList = new ImapList(); - fetchList.add("UID"); - fetchList.add("1"); - fetchList.add("BODY"); - fetchList.add("1.1"); - fetchList.add("text"); - response.add(fetchList); - return response; - } - }) - .thenAnswer(new Answer() { - @Override - public ImapResponse answer(InvocationOnMock invocation) throws Throwable { - ImapResponse response = ImapResponse.newTaggedResponse( - (ImapResponseCallback) invocation.getArguments()[0], "TAG"); - return response; - } - }); + Part part = createPlainTextPart("1.1"); + + setupSingleFetchResponseToCallback(); folder.fetchPart(message, part, null); ArgumentCaptor bodyArgumentCaptor = ArgumentCaptor.forClass(Body.class); @@ -1152,22 +1120,69 @@ public class ImapFolderTest { @Test public void delete_notImplemented() throws MessagingException { ImapFolder folder = createFolder("Folder"); + try { folder.delete(false); + fail("Error expected"); } catch (Error e) { } } @Test - public void getMessageByUid_returnsNewImapMessage() throws MessagingException { + public void getMessageByUid_returnsNewImapMessageWithUidInFolder() throws MessagingException { ImapFolder folder = createFolder("Folder"); + ImapMessage message = folder.getMessage("uid"); - assertNotNull(message); + assertEquals("uid", message.getUid()); assertEquals(folder, message.getFolder()); } + private Part createPlainTextPart(String serverExtra) { + Part part = createPart(serverExtra); + when(part.getHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING)).thenReturn( + new String[]{MimeUtil.ENC_7BIT} + ); + when(part.getHeader(MimeHeader.HEADER_CONTENT_TYPE)).thenReturn( + new String[]{"text/plain"} + ); + return part; + } + + private void setupSingleFetchResponseToCallback() throws IOException { + when(imapConnection.readResponse(any(ImapResponseCallback.class))) + .thenAnswer(new Answer() { + @Override + public ImapResponse answer(InvocationOnMock invocation) throws Throwable { + return buildImapFetchResponse( + (ImapResponseCallback) invocation.getArguments()[0]); + } + }) + .thenAnswer(new Answer() { + @Override + public ImapResponse answer(InvocationOnMock invocation) throws Throwable { + ImapResponse response = ImapResponse.newTaggedResponse( + (ImapResponseCallback) invocation.getArguments()[0], "TAG"); + return response; + } + }); + } + + private ImapResponse buildImapFetchResponse(ImapResponseCallback callback) { + ImapResponse response = ImapResponse.newContinuationRequest(callback); + response.add("1"); + response.add("FETCH"); + ImapList fetchList = new ImapList(); + fetchList.add("UID"); + fetchList.add("1"); + fetchList.add("BODY"); + fetchList.add("1.1"); + fetchList.add("text"); + response.add(fetchList); + return response; + } + private Set extractMessageUids(List messages) { Set result = new HashSet<>(); for (Message message : messages) { From 2feb367de1a5ad2950a13542f1d86977f2ae903c Mon Sep 17 00:00:00 2001 From: Philip Whitehouse Date: Wed, 19 Oct 2016 01:34:05 +0100 Subject: [PATCH 6/6] Changes from code review by jberkel --- .../k9/mail/store/imap/ImapFolderTest.java | 29 +++++-------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java index 531b488a5..fbcc57049 100644 --- a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java +++ b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java @@ -67,12 +67,10 @@ public class ImapFolderTest { private ImapStore imapStore; private ImapConnection imapConnection; private StoreConfig storeConfig; - private Context context; @Before public void setUp() throws Exception { - context = RuntimeEnvironment.application; - BinaryTempFileBody.setTempDirectory(context.getCacheDir()); + BinaryTempFileBody.setTempDirectory(RuntimeEnvironment.application.getCacheDir()); imapStore = mock(ImapStore.class); storeConfig = mock(StoreConfig.class); when(storeConfig.getInboxFolderName()).thenReturn("INBOX"); @@ -541,27 +539,24 @@ public class ImapFolderTest { public void getHighestUid_imapConnectionThrowsNegativesResponse_shouldReturnMinus1() throws Exception { ImapFolder folder = createFolder("Folder"); prepareImapFolderForOpen(OPEN_MODE_RW); - List imapResponses = singletonList(createImapResponse("* SEARCH 42")); when(imapConnection.executeSimpleCommand("UID SEARCH *:*")) .thenThrow(NegativeImapResponseException.class); folder.open(OPEN_MODE_RW); - long result = folder.getHighestUid(); - - assertEquals(-1L, result); + assertEquals(-1L, folder.getHighestUid()); } @Test public void getHighestUid_imapConnectionThrowsIOException_shouldThrowMessagingException() throws Exception { ImapFolder folder = createFolder("Folder"); prepareImapFolderForOpen(OPEN_MODE_RW); - List imapResponses = singletonList(createImapResponse("* SEARCH 42")); when(imapConnection.executeSimpleCommand("UID SEARCH *:*")) .thenThrow(IOException.class); folder.open(OPEN_MODE_RW); try { folder.getHighestUid(); + fail("Expected MessagingException"); } catch (MessagingException e) { assertEquals("IO Error", e.getMessage()); } @@ -791,12 +786,10 @@ public class ImapFolderTest { prepareImapFolderForOpen(OPEN_MODE_RW); folder.open(OPEN_MODE_RW); - boolean result = folder.areMoreMessagesAvailable(1, null); + assertFalse(folder.areMoreMessagesAvailable(1, null)); - //SELECT during OPEN + //SELECT during OPEN and no more verify(imapConnection, times(1)).executeSimpleCommand(anyString()); - - assertFalse(result); } @Test @@ -1117,16 +1110,11 @@ public class ImapFolderTest { } } - @Test + @Test(expected = Error.class) public void delete_notImplemented() throws MessagingException { ImapFolder folder = createFolder("Folder"); - try { - folder.delete(false); - - fail("Error expected"); - } catch (Error e) { - } + folder.delete(false); } @Test @@ -1162,9 +1150,8 @@ public class ImapFolderTest { .thenAnswer(new Answer() { @Override public ImapResponse answer(InvocationOnMock invocation) throws Throwable { - ImapResponse response = ImapResponse.newTaggedResponse( + return ImapResponse.newTaggedResponse( (ImapResponseCallback) invocation.getArguments()[0], "TAG"); - return response; } }); }