From 0865a06ac812f3a9a6a0ce9afe3f04a8d68794ad Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 15 Jul 2022 16:24:22 +0100 Subject: [PATCH] Refactor duplicated test code into superclass. --- .../ContactMailboxDownloadWorkerTest.java | 133 ++---------------- .../mailbox/MailboxDownloadWorkerTest.java | 130 +++++++++++++++++ .../mailbox/OwnMailboxDownloadWorkerTest.java | 117 +-------------- 3 files changed, 150 insertions(+), 230 deletions(-) create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxDownloadWorkerTest.java diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxDownloadWorkerTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxDownloadWorkerTest.java index d4f70e32a..55cbac373 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxDownloadWorkerTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxDownloadWorkerTest.java @@ -1,71 +1,24 @@ package org.briarproject.bramble.mailbox; -import org.briarproject.bramble.api.Cancellable; -import org.briarproject.bramble.api.mailbox.MailboxFileId; -import org.briarproject.bramble.api.mailbox.MailboxProperties; -import org.briarproject.bramble.mailbox.MailboxApi.MailboxFile; -import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; -import org.briarproject.bramble.test.BrambleMockTestCase; -import org.briarproject.bramble.test.CaptureArgumentAction; -import org.jmock.Expectations; -import org.jmock.lib.action.DoAllAction; -import org.junit.After; -import org.junit.Before; import org.junit.Test; -import java.io.File; -import java.util.List; import java.util.concurrent.atomic.AtomicReference; -import static java.util.Arrays.asList; import static java.util.Collections.emptyList; import static org.briarproject.bramble.api.mailbox.MailboxConstants.CLIENT_SUPPORTS; -import static org.briarproject.bramble.api.nullsafety.NullSafety.requireNonNull; -import static org.briarproject.bramble.test.TestUtils.deleteTestDirectory; import static org.briarproject.bramble.test.TestUtils.getMailboxProperties; -import static org.briarproject.bramble.test.TestUtils.getRandomId; -import static org.briarproject.bramble.test.TestUtils.getTestDirectory; import static org.junit.Assert.assertFalse; -public class ContactMailboxDownloadWorkerTest extends BrambleMockTestCase { +public class ContactMailboxDownloadWorkerTest + extends MailboxDownloadWorkerTest { - private final ConnectivityChecker connectivityChecker = - context.mock(ConnectivityChecker.class); - private final TorReachabilityMonitor torReachabilityMonitor = - context.mock(TorReachabilityMonitor.class); - private final MailboxApiCaller mailboxApiCaller = - context.mock(MailboxApiCaller.class); - private final MailboxApi mailboxApi = context.mock(MailboxApi.class); - private final MailboxFileManager mailboxFileManager = - context.mock(MailboxFileManager.class); - private final Cancellable apiCall = context.mock(Cancellable.class); - - private final MailboxProperties mailboxProperties = - getMailboxProperties(false, CLIENT_SUPPORTS); - private final long now = System.currentTimeMillis(); - private final MailboxFile file1 = - new MailboxFile(new MailboxFileId(getRandomId()), now - 1); - private final MailboxFile file2 = - new MailboxFile(new MailboxFileId(getRandomId()), now); - private final List files = asList(file1, file2); - - private File testDir, tempFile; - private ContactMailboxDownloadWorker worker; - - @Before - public void setUp() { - testDir = getTestDirectory(); - tempFile = new File(testDir, "temp"); + public ContactMailboxDownloadWorkerTest() { + mailboxProperties = getMailboxProperties(false, CLIENT_SUPPORTS); worker = new ContactMailboxDownloadWorker(connectivityChecker, torReachabilityMonitor, mailboxApiCaller, mailboxApi, mailboxFileManager, mailboxProperties); } - @After - public void tearDown() { - deleteTestDirectory(testDir); - } - @Test public void testChecksConnectivityWhenStartedAndRemovesObserverWhenDestroyed() { // When the worker is started it should start a connectivity check @@ -93,7 +46,7 @@ public class ContactMailboxDownloadWorkerTest extends BrambleMockTestCase { // When the list-inbox tasks runs and finds no files to download, // it should add a Tor reachability observer - expectCheckForFiles(emptyList()); + expectCheckForFiles(mailboxProperties.getInboxId(), emptyList()); expectAddReachabilityObserver(); assertFalse(listTask.get().callApi()); @@ -104,7 +57,7 @@ public class ContactMailboxDownloadWorkerTest extends BrambleMockTestCase { // When the list-inbox tasks runs and finds no files to download, // it should finish the second download cycle - expectCheckForFiles(emptyList()); + expectCheckForFiles(mailboxProperties.getInboxId(), emptyList()); assertFalse(listTask.get().callApi()); // When the worker is destroyed it should remove the connectivity @@ -129,39 +82,39 @@ public class ContactMailboxDownloadWorkerTest extends BrambleMockTestCase { // When the list-inbox tasks runs and finds some files to download, // it should start a download task for the first file AtomicReference downloadTask = new AtomicReference<>(); - expectCheckForFiles(files); + expectCheckForFiles(mailboxProperties.getInboxId(), files); expectStartTask(downloadTask); assertFalse(listTask.get().callApi()); // When the first download task runs it should download the file to the // location provided by the file manager and start a delete task AtomicReference deleteTask = new AtomicReference<>(); - expectDownloadFile(file1); + expectDownloadFile(mailboxProperties.getInboxId(), file1); expectStartTask(deleteTask); assertFalse(downloadTask.get().callApi()); // When the first delete task runs it should delete the file, ignore // the tolerable failure, and start a download task for the next file - expectDeleteFile(file1, true); // Delete fails tolerably + expectDeleteFile(mailboxProperties.getInboxId(), file1, true); expectStartTask(downloadTask); assertFalse(deleteTask.get().callApi()); // When the second download task runs it should download the file to // the location provided by the file manager and start a delete task - expectDownloadFile(file2); + expectDownloadFile(mailboxProperties.getInboxId(), file2); expectStartTask(deleteTask); assertFalse(downloadTask.get().callApi()); // When the second delete task runs it should delete the file and // start a list-inbox task to check for files that may have arrived // since the first download cycle started - expectDeleteFile(file2, false); // Delete succeeds + expectDeleteFile(mailboxProperties.getInboxId(), file2, false); expectStartTask(listTask); assertFalse(deleteTask.get().callApi()); // When the list-inbox tasks runs and finds no more files to download, // it should add a Tor reachability observer - expectCheckForFiles(emptyList()); + expectCheckForFiles(mailboxProperties.getInboxId(), emptyList()); expectAddReachabilityObserver(); assertFalse(listTask.get().callApi()); @@ -172,7 +125,7 @@ public class ContactMailboxDownloadWorkerTest extends BrambleMockTestCase { // When the list-inbox tasks runs and finds no more files to download, // it should finish the second download cycle - expectCheckForFiles(emptyList()); + expectCheckForFiles(mailboxProperties.getInboxId(), emptyList()); assertFalse(listTask.get().callApi()); // When the worker is destroyed it should remove the connectivity @@ -180,64 +133,4 @@ public class ContactMailboxDownloadWorkerTest extends BrambleMockTestCase { expectRemoveObservers(); worker.destroy(); } - - private void expectStartConnectivityCheck() { - context.checking(new Expectations() {{ - oneOf(connectivityChecker).checkConnectivity( - with(mailboxProperties), with(worker)); - }}); - } - - private void expectStartTask(AtomicReference task) { - context.checking(new Expectations() {{ - oneOf(mailboxApiCaller).retryWithBackoff(with(any(ApiCall.class))); - will(new DoAllAction( - new CaptureArgumentAction<>(task, ApiCall.class, 0), - returnValue(apiCall) - )); - }}); - } - - private void expectCheckForFiles(List files) throws Exception { - context.checking(new Expectations() {{ - oneOf(mailboxApi).getFiles(mailboxProperties, - requireNonNull(mailboxProperties.getInboxId())); - will(returnValue(files)); - }}); - } - - private void expectDownloadFile(MailboxFile file) throws Exception { - context.checking(new Expectations() {{ - oneOf(mailboxFileManager).createTempFileForDownload(); - will(returnValue(tempFile)); - oneOf(mailboxApi).getFile(mailboxProperties, - requireNonNull(mailboxProperties.getInboxId()), - file.name, tempFile); - oneOf(mailboxFileManager).handleDownloadedFile(tempFile); - }}); - } - - private void expectDeleteFile(MailboxFile file, boolean tolerableFailure) - throws Exception { - context.checking(new Expectations() {{ - oneOf(mailboxApi).deleteFile(mailboxProperties, - requireNonNull(mailboxProperties.getInboxId()), file.name); - if (tolerableFailure) { - will(throwException(new TolerableFailureException())); - } - }}); - } - - private void expectAddReachabilityObserver() { - context.checking(new Expectations() {{ - oneOf(torReachabilityMonitor).addOneShotObserver(worker); - }}); - } - - private void expectRemoveObservers() { - context.checking(new Expectations() {{ - oneOf(connectivityChecker).removeObserver(worker); - oneOf(torReachabilityMonitor).removeObserver(worker); - }}); - } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxDownloadWorkerTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxDownloadWorkerTest.java new file mode 100644 index 000000000..0213e594c --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxDownloadWorkerTest.java @@ -0,0 +1,130 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.Cancellable; +import org.briarproject.bramble.api.mailbox.MailboxFileId; +import org.briarproject.bramble.api.mailbox.MailboxFolderId; +import org.briarproject.bramble.api.mailbox.MailboxProperties; +import org.briarproject.bramble.mailbox.MailboxApi.MailboxFile; +import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; +import org.briarproject.bramble.test.BrambleMockTestCase; +import org.briarproject.bramble.test.CaptureArgumentAction; +import org.jmock.Expectations; +import org.jmock.lib.action.DoAllAction; +import org.junit.After; +import org.junit.Before; + +import java.io.File; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; + +import static java.util.Arrays.asList; +import static org.briarproject.bramble.test.TestUtils.deleteTestDirectory; +import static org.briarproject.bramble.test.TestUtils.getRandomId; +import static org.briarproject.bramble.test.TestUtils.getTestDirectory; + +abstract class MailboxDownloadWorkerTest + extends BrambleMockTestCase { + + final ConnectivityChecker connectivityChecker = + context.mock(ConnectivityChecker.class); + final TorReachabilityMonitor torReachabilityMonitor = + context.mock(TorReachabilityMonitor.class); + final MailboxApiCaller mailboxApiCaller = + context.mock(MailboxApiCaller.class); + final MailboxApi mailboxApi = context.mock(MailboxApi.class); + final MailboxFileManager mailboxFileManager = + context.mock(MailboxFileManager.class); + private final Cancellable apiCall = context.mock(Cancellable.class); + + private final long now = System.currentTimeMillis(); + final MailboxFile file1 = + new MailboxFile(new MailboxFileId(getRandomId()), now - 1); + final MailboxFile file2 = + new MailboxFile(new MailboxFileId(getRandomId()), now); + final List files = asList(file1, file2); + + private File testDir, tempFile; + MailboxProperties mailboxProperties; + W worker; + + @Before + public void setUp() { + testDir = getTestDirectory(); + tempFile = new File(testDir, "temp"); + } + + @After + public void tearDown() { + deleteTestDirectory(testDir); + } + + + void expectStartConnectivityCheck() { + context.checking(new Expectations() {{ + oneOf(connectivityChecker).checkConnectivity( + with(mailboxProperties), with(worker)); + }}); + } + + void expectStartTask(AtomicReference task) { + context.checking(new Expectations() {{ + oneOf(mailboxApiCaller).retryWithBackoff(with(any(ApiCall.class))); + will(new DoAllAction( + new CaptureArgumentAction<>(task, ApiCall.class, 0), + returnValue(apiCall) + )); + }}); + } + + void expectCheckForFoldersWithAvailableFiles( + List folderIds) throws Exception { + context.checking(new Expectations() {{ + oneOf(mailboxApi).getFolders(mailboxProperties); + will(returnValue(folderIds)); + }}); + } + + void expectCheckForFiles(MailboxFolderId folderId, + List files) throws Exception { + context.checking(new Expectations() {{ + oneOf(mailboxApi).getFiles(mailboxProperties, folderId); + will(returnValue(files)); + }}); + } + + void expectDownloadFile(MailboxFolderId folderId, + MailboxFile file) + throws Exception { + context.checking(new Expectations() {{ + oneOf(mailboxFileManager).createTempFileForDownload(); + will(returnValue(tempFile)); + oneOf(mailboxApi).getFile(mailboxProperties, folderId, file.name, + tempFile); + oneOf(mailboxFileManager).handleDownloadedFile(tempFile); + }}); + } + + void expectDeleteFile(MailboxFolderId folderId, MailboxFile file, + boolean tolerableFailure) throws Exception { + context.checking(new Expectations() {{ + oneOf(mailboxApi).deleteFile(mailboxProperties, folderId, + file.name); + if (tolerableFailure) { + will(throwException(new TolerableFailureException())); + } + }}); + } + + void expectAddReachabilityObserver() { + context.checking(new Expectations() {{ + oneOf(torReachabilityMonitor).addOneShotObserver(worker); + }}); + } + + void expectRemoveObservers() { + context.checking(new Expectations() {{ + oneOf(connectivityChecker).removeObserver(worker); + oneOf(torReachabilityMonitor).removeObserver(worker); + }}); + } +} diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/OwnMailboxDownloadWorkerTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/OwnMailboxDownloadWorkerTest.java index 07a669d78..33d83d542 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/OwnMailboxDownloadWorkerTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/OwnMailboxDownloadWorkerTest.java @@ -1,20 +1,10 @@ package org.briarproject.bramble.mailbox; -import org.briarproject.bramble.api.Cancellable; import org.briarproject.bramble.api.mailbox.MailboxFileId; import org.briarproject.bramble.api.mailbox.MailboxFolderId; -import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.mailbox.MailboxApi.MailboxFile; -import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; -import org.briarproject.bramble.test.BrambleMockTestCase; -import org.briarproject.bramble.test.CaptureArgumentAction; -import org.jmock.Expectations; -import org.jmock.lib.action.DoAllAction; -import org.junit.After; -import org.junit.Before; import org.junit.Test; -import java.io.File; import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedList; @@ -28,56 +18,31 @@ import static java.util.Collections.emptyList; import static org.briarproject.bramble.api.mailbox.MailboxConstants.CLIENT_SUPPORTS; import static org.briarproject.bramble.mailbox.MailboxDownloadWorker.FolderFile; import static org.briarproject.bramble.mailbox.OwnMailboxDownloadWorker.MAX_ROUND_ROBIN_FILES; -import static org.briarproject.bramble.test.TestUtils.deleteTestDirectory; import static org.briarproject.bramble.test.TestUtils.getMailboxProperties; import static org.briarproject.bramble.test.TestUtils.getRandomId; -import static org.briarproject.bramble.test.TestUtils.getTestDirectory; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -public class OwnMailboxDownloadWorkerTest extends BrambleMockTestCase { +public class OwnMailboxDownloadWorkerTest + extends MailboxDownloadWorkerTest { - private final ConnectivityChecker connectivityChecker = - context.mock(ConnectivityChecker.class); - private final TorReachabilityMonitor torReachabilityMonitor = - context.mock(TorReachabilityMonitor.class); - private final MailboxApiCaller mailboxApiCaller = - context.mock(MailboxApiCaller.class); - private final MailboxApi mailboxApi = context.mock(MailboxApi.class); - private final MailboxFileManager mailboxFileManager = - context.mock(MailboxFileManager.class); - private final Cancellable apiCall = context.mock(Cancellable.class); - - private final MailboxProperties mailboxProperties = - getMailboxProperties(true, CLIENT_SUPPORTS); - private final long now = System.currentTimeMillis(); private final MailboxFolderId folderId1 = new MailboxFolderId(getRandomId()); private final MailboxFolderId folderId2 = new MailboxFolderId(getRandomId()); private final List folderIds = asList(folderId1, folderId2); - private final MailboxFile file1 = - new MailboxFile(new MailboxFileId(getRandomId()), now - 1); - private final MailboxFile file2 = - new MailboxFile(new MailboxFileId(getRandomId()), now); - private final List files = asList(file1, file2); - private File testDir, tempFile; - private OwnMailboxDownloadWorker worker; - - @Before - public void setUp() { - testDir = getTestDirectory(); - tempFile = new File(testDir, "temp"); + public OwnMailboxDownloadWorkerTest() { + mailboxProperties = getMailboxProperties(true, CLIENT_SUPPORTS); worker = new OwnMailboxDownloadWorker(connectivityChecker, torReachabilityMonitor, mailboxApiCaller, mailboxApi, mailboxFileManager, mailboxProperties); } - @After - public void tearDown() { - deleteTestDirectory(testDir); + @Override + public void setUp() { + super.setUp(); } @Test @@ -234,74 +199,6 @@ public class OwnMailboxDownloadWorkerTest extends BrambleMockTestCase { } } - private void expectStartConnectivityCheck() { - context.checking(new Expectations() {{ - oneOf(connectivityChecker).checkConnectivity( - with(mailboxProperties), with(worker)); - }}); - } - - private void expectStartTask(AtomicReference task) { - context.checking(new Expectations() {{ - oneOf(mailboxApiCaller).retryWithBackoff(with(any(ApiCall.class))); - will(new DoAllAction( - new CaptureArgumentAction<>(task, ApiCall.class, 0), - returnValue(apiCall) - )); - }}); - } - - private void expectCheckForFoldersWithAvailableFiles( - List folderIds) throws Exception { - context.checking(new Expectations() {{ - oneOf(mailboxApi).getFolders(mailboxProperties); - will(returnValue(folderIds)); - }}); - } - - private void expectCheckForFiles(MailboxFolderId folderId, - List files) throws Exception { - context.checking(new Expectations() {{ - oneOf(mailboxApi).getFiles(mailboxProperties, folderId); - will(returnValue(files)); - }}); - } - - private void expectDownloadFile(MailboxFolderId folderId, MailboxFile file) - throws Exception { - context.checking(new Expectations() {{ - oneOf(mailboxFileManager).createTempFileForDownload(); - will(returnValue(tempFile)); - oneOf(mailboxApi).getFile(mailboxProperties, folderId, file.name, - tempFile); - oneOf(mailboxFileManager).handleDownloadedFile(tempFile); - }}); - } - - private void expectDeleteFile(MailboxFolderId folderId, MailboxFile file, - boolean tolerableFailure) throws Exception { - context.checking(new Expectations() {{ - oneOf(mailboxApi).deleteFile(mailboxProperties, folderId, - file.name); - if (tolerableFailure) { - will(throwException(new TolerableFailureException())); - } - }}); - } - - private void expectAddReachabilityObserver() { - context.checking(new Expectations() {{ - oneOf(torReachabilityMonitor).addOneShotObserver(worker); - }}); - } - - private void expectRemoveObservers() { - context.checking(new Expectations() {{ - oneOf(connectivityChecker).removeObserver(worker); - oneOf(torReachabilityMonitor).removeObserver(worker); - }}); - } - private Map> createAvailableFiles( int numFolders, int numFiles) { Map> available = new HashMap<>();