Refactor duplicated test code into superclass.

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<ContactMailboxDownloadWorker> {
 
-	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<MailboxFile> 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<ApiCall> 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<ApiCall> 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<ApiCall> 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<MailboxFile> 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);
-		}});
-	}
 }

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<W extends MailboxDownloadWorker>
+		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<MailboxFile> 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<ApiCall> 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<MailboxFolderId> folderIds) throws Exception {
+		context.checking(new Expectations() {{
+			oneOf(mailboxApi).getFolders(mailboxProperties);
+			will(returnValue(folderIds));
+		}});
+	}
+
+	void expectCheckForFiles(MailboxFolderId folderId,
+			List<MailboxFile> 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);
+		}});
+	}
+}

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<OwnMailboxDownloadWorker> {
 
-	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<MailboxFolderId> 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<MailboxFile> 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<ApiCall> 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<MailboxFolderId> folderIds) throws Exception {
-		context.checking(new Expectations() {{
-			oneOf(mailboxApi).getFolders(mailboxProperties);
-			will(returnValue(folderIds));
-		}});
-	}
-
-	private void expectCheckForFiles(MailboxFolderId folderId,
-			List<MailboxFile> 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<MailboxFolderId, Queue<MailboxFile>> createAvailableFiles(
 			int numFolders, int numFiles) {
 		Map<MailboxFolderId, Queue<MailboxFile>> available = new HashMap<>();