diff --git a/build.gradle b/build.gradle index d32853b7ad46..154555aa1233 100644 --- a/build.gradle +++ b/build.gradle @@ -26,7 +26,7 @@ buildscript { } classpath 'gradle.plugin.com.github.spotbugs:spotbugs-gradle-plugin:1.6.6' classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" - classpath "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.0.0-RC14" + classpath "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.0.1" } } diff --git a/src/main/java/com/nextcloud/client/media/Player.kt b/src/main/java/com/nextcloud/client/media/Player.kt index d3fbe7b87957..c47687fdd075 100644 --- a/src/main/java/com/nextcloud/client/media/Player.kt +++ b/src/main/java/com/nextcloud/client/media/Player.kt @@ -93,9 +93,7 @@ internal class Player( override fun onStartDownloading() { trace("onStartDownloading()") - if (playedFile == null) { - throw IllegalStateException("File not set.") - } + checkNotNull(playedFile) { "File not set." } playedFile?.let { val client = buildClient() val task = LoadUrlTask(client, it.remoteId, this@Player::onDownloaded) diff --git a/src/test/java/com/nextcloud/client/core/ManualAsyncRunnerTest.kt b/src/test/java/com/nextcloud/client/core/ManualAsyncRunnerTest.kt index aff2b7b1a60b..9992d5282d61 100644 --- a/src/test/java/com/nextcloud/client/core/ManualAsyncRunnerTest.kt +++ b/src/test/java/com/nextcloud/client/core/ManualAsyncRunnerTest.kt @@ -34,6 +34,14 @@ import org.mockito.MockitoAnnotations class ManualAsyncRunnerTest { + private companion object { + const val EMPTY = 0 + const val ONE_TASK = 1 + const val TWO_TASKS = 2 + const val THREE_TASKS = 3 + const val TIMEOUT = 10000L + } + private lateinit var runner: ManualAsyncRunner @Mock @@ -45,23 +53,23 @@ class ManualAsyncRunnerTest { @Mock private lateinit var onError: OnErrorCallback - private var taskCalls: Int = 0 + private var taskCalls: Int = EMPTY @Before fun setUp() { MockitoAnnotations.initMocks(this) runner = ManualAsyncRunner() - taskCalls = 0 + taskCalls = EMPTY whenever(task.invoke()).thenAnswer { taskCalls++; taskCalls } } @Test fun `tasks are queued`() { - assertEquals(0, runner.size) + assertEquals(EMPTY, runner.size) runner.post(task, onResult, onError) runner.post(task, onResult, onError) runner.post(task, onResult, onError) - assertEquals("Expected 3 tasks to be enqueued", 3, runner.size) + assertEquals("Expected 3 tasks to be enqueued", THREE_TASKS, runner.size) } @Test @@ -70,12 +78,12 @@ class ManualAsyncRunnerTest { runner.post(task, onResult, onError) runner.post(task, onResult, onError) - assertEquals("Queue should contain all enqueued tasks", 3, runner.size) + assertEquals("Queue should contain all enqueued tasks", THREE_TASKS, runner.size) val run = runner.runOne() assertTrue("Executed task should be acknowledged", run) - assertEquals("One task should be run", 1, taskCalls) - verify(onResult).invoke(eq(1)) - assertEquals("Only 1 task should be consumed", 2, runner.size) + assertEquals("One task should be run", ONE_TASK, taskCalls) + verify(onResult).invoke(eq(ONE_TASK)) + assertEquals("Only 1 task should be consumed", TWO_TASKS, runner.size) } @Test @@ -83,12 +91,12 @@ class ManualAsyncRunnerTest { runner.post(task, onResult, onError) runner.post(task, onResult, onError) - assertEquals("Queue should contain all enqueued tasks", 2, runner.size) + assertEquals("Queue should contain all enqueued tasks", TWO_TASKS, runner.size) val count = runner.runAll() - assertEquals("Executed tasks should be acknowledged", 2, count) - verify(task, times(2)).invoke() - verify(onResult, times(2)).invoke(any()) - assertEquals("Entire queue should be processed", 0, runner.size) + assertEquals("Executed tasks should be acknowledged", TWO_TASKS, count) + verify(task, times(TWO_TASKS)).invoke() + verify(onResult, times(TWO_TASKS)).invoke(any()) + assertEquals("Entire queue should be processed", EMPTY, runner.size) } @Test @@ -98,7 +106,7 @@ class ManualAsyncRunnerTest { @Test fun `run all tasks when queue is empty`() { - assertEquals("No task should be run", 0, runner.runAll()) + assertEquals("No task should be run", EMPTY, runner.runAll()) } @Test @@ -112,7 +120,7 @@ class ManualAsyncRunnerTest { runner.post(task) }) }) - assertEquals(1, runner.size) + assertEquals(ONE_TASK, runner.size) // WHEN // runs all @@ -120,10 +128,10 @@ class ManualAsyncRunnerTest { // THEN // all subsequently scheduled tasks are run too - assertEquals(3, count) + assertEquals(THREE_TASKS, count) } - @Test(expected = IllegalStateException::class, timeout = 10000) + @Test(expected = IllegalStateException::class, timeout = TIMEOUT) fun `runner detects infinite loops caused by scheduling tasks recusively`() { val recursiveTask: () -> String = object : Function0 { override fun invoke(): String { diff --git a/src/test/java/com/nextcloud/client/core/ThreadPoolAsyncRunnerTest.kt b/src/test/java/com/nextcloud/client/core/ThreadPoolAsyncRunnerTest.kt index bd921ea71498..bbda1f57c05a 100644 --- a/src/test/java/com/nextcloud/client/core/ThreadPoolAsyncRunnerTest.kt +++ b/src/test/java/com/nextcloud/client/core/ThreadPoolAsyncRunnerTest.kt @@ -41,6 +41,11 @@ class ThreadPoolAsyncRunnerTest { private lateinit var handler: Handler private lateinit var r: ThreadPoolAsyncRunner + private companion object { + const val INIT_COUNT = 1 + const val THREAD_SLEEP = 500L + } + @Before fun setUp() { handler = spy(Handler()) @@ -83,7 +88,7 @@ class ThreadPoolAsyncRunnerTest { @Test fun `returns error via handler`() { - val afterPostLatch = CountDownLatch(1) + val afterPostLatch = CountDownLatch(INIT_COUNT) doAnswer { (it.arguments[0] as Runnable).run() afterPostLatch.countDown() @@ -101,8 +106,8 @@ class ThreadPoolAsyncRunnerTest { @Test fun `cancelled task does not return result`() { - val taskIsCancelled = CountDownLatch(1) - val taskIsRunning = CountDownLatch(1) + val taskIsCancelled = CountDownLatch(INIT_COUNT) + val taskIsRunning = CountDownLatch(INIT_COUNT) val t = r.post({ taskIsRunning.countDown() taskIsCancelled.await() @@ -111,23 +116,23 @@ class ThreadPoolAsyncRunnerTest { assertAwait(taskIsRunning) t.cancel() taskIsCancelled.countDown() - Thread.sleep(500) // yuck! + Thread.sleep(THREAD_SLEEP) // yuck! verify(handler, never()).post(any()) } @Test fun `cancelled task does not return error`() { - val taskIsCancelled = CountDownLatch(1) - val taskIsRunning = CountDownLatch(1) + val taskIsCancelled = CountDownLatch(INIT_COUNT) + val taskIsRunning = CountDownLatch(INIT_COUNT) val t = r.post({ taskIsRunning.countDown() taskIsCancelled.await() - throw RuntimeException("whatever") + throw IllegalStateException("whatever") }, onResult = {}, onError = {}) assertAwait(taskIsRunning) t.cancel() taskIsCancelled.countDown() - Thread.sleep(500) // yuck! + Thread.sleep(THREAD_SLEEP) // yuck! verify(handler, never()).post(any()) } } diff --git a/src/test/java/com/nextcloud/client/logger/FileLogHandlerTest.kt b/src/test/java/com/nextcloud/client/logger/FileLogHandlerTest.kt index 708fa4f7df78..96814fe52377 100644 --- a/src/test/java/com/nextcloud/client/logger/FileLogHandlerTest.kt +++ b/src/test/java/com/nextcloud/client/logger/FileLogHandlerTest.kt @@ -28,8 +28,17 @@ import java.io.File import java.nio.charset.Charset import java.nio.file.Files +@Suppress("TooManyFunctions") class FileLogHandlerTest { + private companion object { + const val FILE_SIZE = 1024L + const val MAX_FILE_SIZE = 20L + const val THREE_LOG_FILES = 3 + const val EXPECTED_LINE_COUNT_6 = 6 + const val EXPECTED_LINE_COUNT_12 = 12 + } + private lateinit var logDir: File private fun readLogFile(name: String): String { @@ -64,7 +73,7 @@ class FileLogHandlerTest { // WHEN // file is opened - val handler = FileLogHandler(nonexistingLogsDir, "log.txt", 1000) + val handler = FileLogHandler(nonexistingLogsDir, "log.txt", FILE_SIZE) handler.open() // THEN @@ -90,7 +99,7 @@ class FileLogHandlerTest { writeLogFile("log.txt.1", "2") writeLogFile("log.txt.2", "3") - val writer = FileLogHandler(logDir, "log.txt", 1024) + val writer = FileLogHandler(logDir, "log.txt", FILE_SIZE) // WHEN // files are rotated @@ -112,7 +121,7 @@ class FileLogHandlerTest { // log file limit is 20 bytes // log writer is opened writeLogFile("log.txt", "0123456789") - val writer = FileLogHandler(logDir, "log.txt", 20) + val writer = FileLogHandler(logDir, "log.txt", MAX_FILE_SIZE) writer.open() // WHEN @@ -132,7 +141,7 @@ class FileLogHandlerTest { // log file limit is 20 bytes // log writer is opened writeLogFile("log.txt", "0123456789") - val writer = FileLogHandler(logDir, "log.txt", 20) + val writer = FileLogHandler(logDir, "log.txt", MAX_FILE_SIZE) writer.open() // WHEN @@ -162,14 +171,14 @@ class FileLogHandlerTest { // WHEN // log file is read including rotated content - val writer = FileLogHandler(logDir, "log.txt", 1000) - val rawLogs = writer.loadLogFiles(3) + val writer = FileLogHandler(logDir, "log.txt", FILE_SIZE) + val rawLogs = writer.loadLogFiles(THREE_LOG_FILES) // THEN // all files are loaded // lines are loaded in correct order // log files size is correctly reported - assertEquals(12, rawLogs.lines.size) + assertEquals(EXPECTED_LINE_COUNT_12, rawLogs.lines.size) assertEquals( listOf( "line1", "line2", "line3", @@ -193,13 +202,13 @@ class FileLogHandlerTest { // WHEN // log file is read including rotated content - val writer = FileLogHandler(logDir, "log.txt", 1000) - val lines = writer.loadLogFiles(3) + val writer = FileLogHandler(logDir, "log.txt", FILE_SIZE) + val lines = writer.loadLogFiles(THREE_LOG_FILES) // THEN // all files are loaded // log file size is non-zero - assertEquals(6, lines.lines.size) + assertEquals(EXPECTED_LINE_COUNT_6, lines.lines.size) assertTrue(lines.logSize > 0) } @@ -207,7 +216,7 @@ class FileLogHandlerTest { fun `load log lines - negative count is illegal`() { // WHEN // requesting negative number of rotated files - val writer = FileLogHandler(logDir, "log.txt", 1000) + val writer = FileLogHandler(logDir, "log.txt", FILE_SIZE) val lines = writer.loadLogFiles(-1) // THEN @@ -218,7 +227,7 @@ class FileLogHandlerTest { fun `all log files are deleted`() { // GIVEN // log files exist - val handler = FileLogHandler(logDir, "log.txt", 100) + val handler = FileLogHandler(logDir, "log.txt", MAX_FILE_SIZE) for (i in 0 until handler.maxLogFilesCount) { handler.rotateLogs() handler.open() diff --git a/src/test/java/com/nextcloud/client/logger/LogEntryTest.kt b/src/test/java/com/nextcloud/client/logger/LogEntryTest.kt index 8f8760a2a931..0c86befebd23 100644 --- a/src/test/java/com/nextcloud/client/logger/LogEntryTest.kt +++ b/src/test/java/com/nextcloud/client/logger/LogEntryTest.kt @@ -19,9 +19,6 @@ */ package com.nextcloud.client.logger -import java.util.Date -import java.util.SimpleTimeZone -import java.util.concurrent.TimeUnit import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull @@ -29,6 +26,9 @@ import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.Suite +import java.util.Date +import java.util.SimpleTimeZone +import java.util.concurrent.TimeUnit @RunWith(Suite::class) @Suite.SuiteClasses( @@ -36,6 +36,9 @@ import org.junit.runners.Suite LogEntryTest.Parse::class ) class LogEntryTest { + private companion object { + const val SEVEN_HOUR = 7L + } class ToString { @Test @@ -57,7 +60,7 @@ class LogEntryTest { tag = "tag", message = "some message" ) - val sevenHours = TimeUnit.HOURS.toMillis(7).toInt() + val sevenHours = TimeUnit.HOURS.toMillis(SEVEN_HOUR).toInt() val tz = SimpleTimeZone(sevenHours, "+0700") assertEquals("1970-01-01T07:00:00.000+0700;D;tag;some message", entry.toString(tz)) } diff --git a/src/test/java/com/nextcloud/client/logger/LoggerTest.kt b/src/test/java/com/nextcloud/client/logger/LoggerTest.kt index 6ec95f70a9d4..69fee3ff2ff9 100644 --- a/src/test/java/com/nextcloud/client/logger/LoggerTest.kt +++ b/src/test/java/com/nextcloud/client/logger/LoggerTest.kt @@ -49,6 +49,13 @@ class LoggerTest { private companion object { const val QUEUE_CAPACITY = 100 + const val FILE_SIZE = 1024L + const val LATCH_WAIT = 3L + const val LATCH_INIT = 3 + const val EMPTY = 0 + const val EMPTY_LONG = 0L + const val TIMEOUT = 3000L + const val MESSAGE_COUNT = 3 } private lateinit var clock: Clock @@ -61,7 +68,7 @@ class LoggerTest { MockitoAnnotations.initMocks(this) val tempDir = Files.createTempDirectory("log-test").toFile() clock = ClockImpl() - logHandler = spy(FileLogHandler(tempDir, "log.txt", 1024)) + logHandler = spy(FileLogHandler(tempDir, "log.txt", FILE_SIZE)) osHandler = mock() logger = LoggerImpl(clock, logHandler, osHandler, QUEUE_CAPACITY) } @@ -70,7 +77,7 @@ class LoggerTest { fun `write is done on background thread`() { val callerThreadId = Thread.currentThread().id val writerThreadIds = mutableListOf() - val latch = CountDownLatch(3) + val latch = CountDownLatch(LATCH_INIT) doAnswer { writerThreadIds.add(Thread.currentThread().id) @@ -102,7 +109,7 @@ class LoggerTest { // message is processed on bg thread // all handler invocations happen on bg thread // all handler invocations happen on single thread - assertTrue(latch.await(3, TimeUnit.SECONDS)) + assertTrue(latch.await(LATCH_WAIT, TimeUnit.SECONDS)) writerThreadIds.forEach { writerThreadId -> assertNotEquals("All requests must be made on bg thread", callerThreadId, writerThreadId) @@ -117,7 +124,7 @@ class LoggerTest { fun `message is written via log handler`() { val tag = "test tag" val message = "test log message" - val latch = CountDownLatch(3) + val latch = CountDownLatch(LATCH_INIT) doAnswer { it.callRealMethod(); latch.countDown() }.whenever(logHandler).open() doAnswer { it.callRealMethod(); latch.countDown() }.whenever(logHandler).write(any()) doAnswer { it.callRealMethod(); latch.countDown() }.whenever(logHandler).close() @@ -135,7 +142,7 @@ class LoggerTest { // log handler writes entry // log handler closes log file // no lost messages - val called = latch.await(3, TimeUnit.SECONDS) + val called = latch.await(LATCH_WAIT, TimeUnit.SECONDS) assertTrue("Expected open(), write() and close() calls on bg thread", called) val inOrder = inOrder(logHandler) inOrder.verify(logHandler).open() @@ -177,7 +184,7 @@ class LoggerTest { logger.d("tag", "message 2") logger.d("tag", "message 3") logger.load(listener) - val called = latch.await(3, TimeUnit.SECONDS) + val called = latch.await(LATCH_WAIT, TimeUnit.SECONDS) assertTrue("Response not posted", called) // THEN @@ -194,7 +201,7 @@ class LoggerTest { val logsCaptor = ArgumentCaptor.forClass(List::class.java) as ArgumentCaptor> val sizeCaptor = ArgumentCaptor.forClass(Long::class.java) verify(listener).invoke(capture(logsCaptor), capture(sizeCaptor)) - assertEquals(3, logsCaptor.value.size) + assertEquals(MESSAGE_COUNT, logsCaptor.value.size) assertTrue("message 1" in logsCaptor.value[0].message) assertTrue("message 2" in logsCaptor.value[1].message) assertTrue("message 3" in logsCaptor.value[2].message) @@ -239,7 +246,7 @@ class LoggerTest { logger.start() // THEN - // overflow occurence is logged + // overflow occurrence is logged val posted = CountDownLatch(1) whenever(osHandler.post(any())).thenAnswer { (it.arguments[0] as Runnable).run() @@ -279,15 +286,16 @@ class LoggerTest { // THEN // handler writes files // handler deletes all files - assertTrue(latch.await(3, TimeUnit.SECONDS)) - verify(logHandler, times(3)).write(any()) + assertTrue(latch.await(LATCH_WAIT, TimeUnit.SECONDS)) + verify(logHandler, times(MESSAGE_COUNT)).write(any()) verify(logHandler).deleteAll() val loaded = logHandler.loadLogFiles(logHandler.maxLogFilesCount) - assertEquals(0, loaded.lines.size) - assertEquals(0L, loaded.logSize) + assertEquals(EMPTY, loaded.lines.size) + assertEquals(EMPTY_LONG, loaded.logSize) } @Test + @Suppress("TooGenericExceptionCaught") fun `thread interruption is handled while posting log message`() { Thread { val callerThread = Thread.currentThread() @@ -313,7 +321,7 @@ class LoggerTest { assertTrue("Expected current thread to stay interrupted", callerThread.isInterrupted) }.apply { start() - join(3000) + join(TIMEOUT) } } } diff --git a/src/test/java/com/nextcloud/client/logger/ui/AsyncFilterTest.kt b/src/test/java/com/nextcloud/client/logger/ui/AsyncFilterTest.kt index 08532141838f..16f42f5e2e53 100644 --- a/src/test/java/com/nextcloud/client/logger/ui/AsyncFilterTest.kt +++ b/src/test/java/com/nextcloud/client/logger/ui/AsyncFilterTest.kt @@ -30,6 +30,7 @@ import org.junit.Assert.assertNull import org.junit.Before import org.junit.Test +@Suppress("MagicNumber") // numbers are used for testing sort class AsyncFilterTest { class OnResult : (List, Long) -> Unit { diff --git a/src/test/java/com/nextcloud/client/logger/ui/LogsViewModelTest.kt b/src/test/java/com/nextcloud/client/logger/ui/LogsViewModelTest.kt index 99af65fd8235..5d7e8c349fb9 100644 --- a/src/test/java/com/nextcloud/client/logger/ui/LogsViewModelTest.kt +++ b/src/test/java/com/nextcloud/client/logger/ui/LogsViewModelTest.kt @@ -59,6 +59,8 @@ class LogsViewModelTest { ) val TEST_LOG_SIZE_KILOBYTES = 42L val TEST_LOG_SIZE_BYTES = TEST_LOG_SIZE_KILOBYTES * 1024L + const val TOTAL_ENTRY_COUNT = 3 + const val QUERY_TIME = 4 } class TestLogRepository : LogsRepository { @@ -67,7 +69,8 @@ class LogsViewModelTest { override val lostEntries: Boolean = false override fun load(onLoaded: OnLogsLoaded) { this.onLoadedCallback = onLoaded; loadRequestCount++ } - override fun deleteAll() {} + override fun deleteAll() { /* no implementation neeeded */ + } } abstract class Fixture { @@ -233,8 +236,10 @@ class LogsViewModelTest { assertEquals("Status should contain size in kB", TEST_LOG_SIZE_KILOBYTES, statusArgs[1]) assertEquals("Status should show matched entries count", vm.entries.value?.size, statusArgs[2]) - assertEquals("Status should contain total entries count", TEST_LOG_ENTRIES.size, statusArgs[3]) - assertTrue("Status should contain query time in ms", statusArgs[4] is Long) + assertEquals("Status should contain total entries count", + TEST_LOG_ENTRIES.size, + statusArgs[TOTAL_ENTRY_COUNT]) + assertTrue("Status should contain query time in ms", statusArgs[QUERY_TIME] is Long) } } } diff --git a/src/test/java/com/nextcloud/client/media/AudioFocusTest.kt b/src/test/java/com/nextcloud/client/media/AudioFocusTest.kt index cb0080676b7d..75211dbdbaaf 100644 --- a/src/test/java/com/nextcloud/client/media/AudioFocusTest.kt +++ b/src/test/java/com/nextcloud/client/media/AudioFocusTest.kt @@ -25,10 +25,13 @@ import org.junit.Assert.assertNull import org.junit.Test class AudioFocusTest { + private companion object { + const val INVALID_FOCUS = -10000 + } @Test fun `invalid values result in null`() { - val focus = AudioFocus.fromPlatformFocus(-10000) + val focus = AudioFocus.fromPlatformFocus(INVALID_FOCUS) assertNull(focus) } diff --git a/src/test/java/com/owncloud/android/ui/db/OCUploadComparatorTest.kt b/src/test/java/com/owncloud/android/ui/db/OCUploadComparatorTest.kt index e7aee8f4469e..f2b409dbf3a9 100644 --- a/src/test/java/com/owncloud/android/ui/db/OCUploadComparatorTest.kt +++ b/src/test/java/com/owncloud/android/ui/db/OCUploadComparatorTest.kt @@ -48,6 +48,10 @@ class OCUploadComparatorTest { val equalsNotSame = mock(name = "equalsNotSame") val inProgress = mock(name = "InProgress") val inProgressNow = mock(name = "inProgressNow") + private const val FIXED_UPLOAD_END_TIMESTAMP = 42L + private const val FIXED_UPLOAD_END_TIMESTAMP_LATER = 43L + private const val UPLOAD_ID = 40L + private const val UPLOAD_ID2 = 43L fun uploads(): Array { return arrayOf(failed, failedLater, inProgress, inProgressNow, failedSameTimeOtherId) @@ -68,14 +72,14 @@ class OCUploadComparatorTest { whenever(inProgressNow.isFixedUploadingNow).thenReturn(true) whenever(inProgress.isFixedUploadingNow).thenReturn(false) - whenever(failed.fixedUploadEndTimeStamp).thenReturn(42) - whenever(failedLater.fixedUploadEndTimeStamp).thenReturn(43) - whenever(failedSameTimeOtherId.fixedUploadEndTimeStamp).thenReturn(42) - whenever(equalsNotSame.fixedUploadEndTimeStamp).thenReturn(42) + whenever(failed.fixedUploadEndTimeStamp).thenReturn(FIXED_UPLOAD_END_TIMESTAMP) + whenever(failedLater.fixedUploadEndTimeStamp).thenReturn(FIXED_UPLOAD_END_TIMESTAMP_LATER) + whenever(failedSameTimeOtherId.fixedUploadEndTimeStamp).thenReturn(FIXED_UPLOAD_END_TIMESTAMP) + whenever(equalsNotSame.fixedUploadEndTimeStamp).thenReturn(FIXED_UPLOAD_END_TIMESTAMP) - whenever(failedLater.uploadId).thenReturn(43) - whenever(failedSameTimeOtherId.uploadId).thenReturn(40) - whenever(equalsNotSame.uploadId).thenReturn(40) + whenever(failedLater.uploadId).thenReturn(UPLOAD_ID2) + whenever(failedSameTimeOtherId.uploadId).thenReturn(UPLOAD_ID) + whenever(equalsNotSame.uploadId).thenReturn(UPLOAD_ID) } } }