From 913e5da2f51641eff463d0ee01ddefc61a5acf34 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 15 Jul 2022 15:19:11 +0100 Subject: [PATCH] Refactor test expectations, add test for nothing to download. --- .../ContactMailboxDownloadWorkerTest.java | 216 +++++++++--------- 1 file changed, 110 insertions(+), 106 deletions(-) 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 32ac944a0..d4f70e32a 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 @@ -69,20 +69,47 @@ public class ContactMailboxDownloadWorkerTest extends BrambleMockTestCase { @Test public void testChecksConnectivityWhenStartedAndRemovesObserverWhenDestroyed() { // When the worker is started it should start a connectivity check - context.checking(new Expectations() {{ - oneOf(connectivityChecker).checkConnectivity( - with(mailboxProperties), with(worker)); - }}); - + expectStartConnectivityCheck(); worker.start(); // When the worker is destroyed it should remove the connectivity // and reachability observers - context.checking(new Expectations() {{ - oneOf(connectivityChecker).removeObserver(worker); - oneOf(torReachabilityMonitor).removeObserver(worker); - }}); + expectRemoveObservers(); + worker.destroy(); + } + @Test + public void testChecksForFilesWhenConnectivityCheckSucceeds() + throws Exception { + // When the worker is started it should start a connectivity check + expectStartConnectivityCheck(); + worker.start(); + + // When the connectivity check succeeds, a list-inbox task should be + // started for the first download cycle + AtomicReference listTask = new AtomicReference<>(); + expectStartTask(listTask); + worker.onConnectivityCheckSucceeded(); + + // When the list-inbox tasks runs and finds no files to download, + // it should add a Tor reachability observer + expectCheckForFiles(emptyList()); + expectAddReachabilityObserver(); + assertFalse(listTask.get().callApi()); + + // When the reachability observer is called, a list-inbox task should + // be started for the second download cycle + expectStartTask(listTask); + worker.onTorReachable(); + + // When the list-inbox tasks runs and finds no files to download, + // it should finish the second download cycle + expectCheckForFiles(emptyList()); + assertFalse(listTask.get().callApi()); + + // When the worker is destroyed it should remove the connectivity + // and reachability observers + expectRemoveObservers(); worker.destroy(); } @@ -90,150 +117,127 @@ public class ContactMailboxDownloadWorkerTest extends BrambleMockTestCase { public void testDownloadsFilesWhenConnectivityCheckSucceeds() throws Exception { // When the worker is started it should start a connectivity check - context.checking(new Expectations() {{ - oneOf(connectivityChecker).checkConnectivity( - with(mailboxProperties), with(worker)); - }}); - + expectStartConnectivityCheck(); worker.start(); // When the connectivity check succeeds, a list-inbox task should be // started for the first download cycle AtomicReference listTask = new AtomicReference<>(); - context.checking(new Expectations() {{ - oneOf(mailboxApiCaller).retryWithBackoff(with(any(ApiCall.class))); - will(new DoAllAction( - new CaptureArgumentAction<>(listTask, ApiCall.class, 0), - returnValue(apiCall) - )); - }}); - + expectStartTask(listTask); worker.onConnectivityCheckSucceeded(); // 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<>(); - context.checking(new Expectations() {{ - oneOf(mailboxApi).getFiles(mailboxProperties, - requireNonNull(mailboxProperties.getInboxId())); - will(returnValue(files)); - oneOf(mailboxApiCaller).retryWithBackoff(with(any(ApiCall.class))); - will(new DoAllAction( - new CaptureArgumentAction<>(downloadTask, ApiCall.class, 0), - returnValue(apiCall) - )); - }}); - + expectCheckForFiles(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<>(); - context.checking(new Expectations() {{ - oneOf(mailboxFileManager).createTempFileForDownload(); - will(returnValue(tempFile)); - oneOf(mailboxApi).getFile(mailboxProperties, - requireNonNull(mailboxProperties.getInboxId()), - file1.name, tempFile); - oneOf(mailboxFileManager).handleDownloadedFile(tempFile); - oneOf(mailboxApiCaller).retryWithBackoff(with(any(ApiCall.class))); - will(new DoAllAction( - new CaptureArgumentAction<>(deleteTask, ApiCall.class, 0), - returnValue(apiCall) - )); - }}); - + expectDownloadFile(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 - context.checking(new Expectations() {{ - oneOf(mailboxApi).deleteFile(mailboxProperties, - requireNonNull(mailboxProperties.getInboxId()), file1.name); - will(throwException(new TolerableFailureException())); - oneOf(mailboxApiCaller).retryWithBackoff(with(any(ApiCall.class))); - will(new DoAllAction( - new CaptureArgumentAction<>(downloadTask, ApiCall.class, 0), - returnValue(apiCall) - )); - }}); - + expectDeleteFile(file1, true); // Delete fails tolerably + 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 - context.checking(new Expectations() {{ - oneOf(mailboxFileManager).createTempFileForDownload(); - will(returnValue(tempFile)); - oneOf(mailboxApi).getFile(mailboxProperties, - requireNonNull(mailboxProperties.getInboxId()), - file2.name, tempFile); - oneOf(mailboxFileManager).handleDownloadedFile(tempFile); - oneOf(mailboxApiCaller).retryWithBackoff(with(any(ApiCall.class))); - will(new DoAllAction( - new CaptureArgumentAction<>(deleteTask, ApiCall.class, 0), - returnValue(apiCall) - )); - }}); - + expectDownloadFile(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 - context.checking(new Expectations() {{ - oneOf(mailboxApi).deleteFile(mailboxProperties, - requireNonNull(mailboxProperties.getInboxId()), file2.name); - will(throwException(new TolerableFailureException())); - oneOf(mailboxApiCaller).retryWithBackoff(with(any(ApiCall.class))); - will(new DoAllAction( - new CaptureArgumentAction<>(listTask, ApiCall.class, 0), - returnValue(apiCall) - )); - }}); - + expectDeleteFile(file2, false); // Delete succeeds + 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 - context.checking(new Expectations() {{ - oneOf(mailboxApi).getFiles(mailboxProperties, - requireNonNull(mailboxProperties.getInboxId())); - will(returnValue(emptyList())); - oneOf(torReachabilityMonitor).addOneShotObserver(worker); - }}); - + expectCheckForFiles(emptyList()); + expectAddReachabilityObserver(); assertFalse(listTask.get().callApi()); // When the reachability observer is called, a list-inbox task should // be started for the second download cycle - context.checking(new Expectations() {{ - oneOf(mailboxApiCaller).retryWithBackoff(with(any(ApiCall.class))); - will(new DoAllAction( - new CaptureArgumentAction<>(listTask, ApiCall.class, 0), - returnValue(apiCall) - )); - }}); - + expectStartTask(listTask); worker.onTorReachable(); // When the list-inbox tasks runs and finds no more files to download, // it should finish the second download cycle - context.checking(new Expectations() {{ - oneOf(mailboxApi).getFiles(mailboxProperties, - requireNonNull(mailboxProperties.getInboxId())); - will(returnValue(emptyList())); - }}); - + expectCheckForFiles(emptyList()); assertFalse(listTask.get().callApi()); // When the worker is destroyed it should remove the connectivity // and reachability observers + 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); }}); - - worker.destroy(); } }