From 5001d7a9b82136cc6fd3eda401d71084d015e470 Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Thu, 1 Oct 2020 11:25:23 +0200
Subject: [PATCH] Solve unreliable tests (#1266)

* Fix unreliable test.
Random could be randomly 0 which would fail the test.

* Create types for buildflavors to prevent string comparison.
Use these build variants for specific tests.

* Don't assign the times variable to a final value as it breaks tests.
Until refactored, it's okay to recalculate this.

* Remove a few sonar code smells.

* Fix broken instrumentation test for key verification.
Add matching binary, signature and pubkey from pod.
---
 .../util/security/VerificationKeysTest.kt     | 39 +++++++++++++------
 .../coronawarnapp/CoronaWarnApplication.kt    | 16 ++++++--
 .../environment/EnvironmentSetup.kt           |  2 +-
 .../rki/coronawarnapp/risk/TimeVariables.kt   | 22 +++++------
 .../coronawarnapp/ui/calendar/CalendarView.kt | 19 ++++-----
 .../InteroperabilityConfigurationFragment.kt  |  1 +
 ...ssionResultPositiveOtherWarningFragment.kt |  4 +-
 .../de/rki/coronawarnapp/util/CWADebug.kt     | 12 +++++-
 .../environment/EnvironmentSetupTest.kt       |  6 +--
 .../coronawarnapp/risk/TimeVariablesTest.kt   | 15 ++++---
 .../rki/coronawarnapp/util/PaddingToolTest.kt |  6 ++-
 11 files changed, 86 insertions(+), 56 deletions(-)

diff --git a/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/VerificationKeysTest.kt b/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/VerificationKeysTest.kt
index f43f22095..817159d68 100644
--- a/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/VerificationKeysTest.kt
+++ b/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/VerificationKeysTest.kt
@@ -1,17 +1,29 @@
 package de.rki.coronawarnapp.util.security
 
+import de.rki.coronawarnapp.environment.EnvironmentSetup
 import de.rki.coronawarnapp.exception.CwaSecurityException
 import io.kotest.assertions.throwables.shouldThrow
 import io.kotest.matchers.shouldBe
+import io.mockk.MockKAnnotations
+import io.mockk.every
+import io.mockk.impl.annotations.MockK
 import okio.ByteString.Companion.decodeHex
+import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 
 @RunWith(JUnit4::class)
 class VerificationKeysTest {
+    @MockK lateinit var environmentSetup: EnvironmentSetup
 
-    private fun createTool() = VerificationKeys()
+    @Before
+    fun setup() {
+        MockKAnnotations.init(this)
+        every { environmentSetup.appConfigVerificationKey } returns PUB_KEY
+    }
+
+    private fun createTool() = VerificationKeys(environmentSetup)
 
     @Test
     fun goodBinaryAndSignature() {
@@ -54,18 +66,21 @@ class VerificationKeysTest {
     }
 
     companion object {
+        private const val PUB_KEY =
+            "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEc7DEstcUIRcyk35OYDJ95/hTg" +
+                "3UVhsaDXKT0zK7NhHPXoyzipEnOp3GyNXDVpaPi3cAfQmxeuFMZAIX2+6A5Xg=="
         private const val GOOD_BINARY =
-            "080b124d0a230a034c4f57180f221a68747470733a2f2f7777772e636f726f6e617761726e2e6170700a26" +
-                    "0a0448494748100f1848221a68747470733a2f2f7777772e636f726f6e617761726e2e6170701a" +
-                    "640a10080110021803200428053006380740081100000000000049401a0a200128013001380140" +
-                    "012100000000000049402a10080510051805200528053005380540053100000000000034403a0e" +
-                    "1001180120012801300138014001410000000000004940221c0a040837103f1212090000000000" +
-                    "00f03f11000000000000e03f20192a1a0a0a0a041008180212021005120c0a0408011804120408" +
-                    "011804"
+            "080b124e0a230a034c4f57180f221a68747470733a2f2f7777772e636f726f6e617761726e2e6170700a2" +
+                "70a0448494748100f188f4e221a68747470733a2f2f7777772e636f726f6e617761726e2e6170701a" +
+                "600a0c1803200428053006380740081100000000000049401a0a20012801300138014001210000000" +
+                "0000049402a10080510051805200528053005380540053100000000000034403a0e10021802200228" +
+                "02300238024002410000000000004940221c0a040837103f121209000000000000f03f11000000000" +
+                "000e03f20322a1a0a0a0a041008180212021005120c0a040801180412040801180432220a200a1c69" +
+                "73506c61757369626c6544656e696162696c6974794163746976651001"
         private const val GOOD_SIGNATURE =
-            "0a87010a380a1864652e726b692e636f726f6e617761726e6170702d6465761a02763122033236322a1331" +
-                    "2e322e3834302e31303034352e342e332e321001180122473045022100cf32ff24ea18a1ffcc7f" +
-                    "f4c9fe8d1808cecbc5a37e3e1d4c9ce682120450958c022064bf124b6973a9b510a43d479ff93e" +
-                    "0ef97a5b893c7af4abc4a8d399969cd8a0"
+            "0a83010a340a1464652e726b692e636f726f6e617761726e6170701a02763122033236322a13312e322e3" +
+                "834302e31303034352e342e332e32100118012247304502210099836666c962dd7a44292f6211b55e" +
+                "f2364ea3a5995238c862c3b58f774237da02200ae0e793d02f92826e5dea2d7758cbfb564089b1c2a" +
+                "f296d5bb80331bc5e0c5e"
     }
 }
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt
index abeedeea4..ff6e6b984 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt
@@ -94,13 +94,21 @@ class CoronaWarnApplication : Application(), HasAndroidInjector {
             }
         }
 
-        override fun onActivityStarted(activity: Activity) {}
+        override fun onActivityStarted(activity: Activity) {
+            // NOOP
+        }
 
-        override fun onActivityDestroyed(activity: Activity) {}
+        override fun onActivityDestroyed(activity: Activity) {
+            // NOOP
+        }
 
-        override fun onActivitySaveInstanceState(activity: Activity, outState: Bundle) {}
+        override fun onActivitySaveInstanceState(activity: Activity, outState: Bundle) {
+            // NOOP
+        }
 
-        override fun onActivityStopped(activity: Activity) {}
+        override fun onActivityStopped(activity: Activity) {
+            // NOOP
+        }
 
         @SuppressLint("SourceLockedOrientationActivity")
         override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) {
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/environment/EnvironmentSetup.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/environment/EnvironmentSetup.kt
index 1ca6cf28a..1150661ab 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/environment/EnvironmentSetup.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/environment/EnvironmentSetup.kt
@@ -58,7 +58,7 @@ class EnvironmentSetup @Inject constructor(
                 ?.toEnvironmentType() ?: defaultEnvironment
         }
         set(value) {
-            if (CWADebug.isDebugBuildOrMode) {
+            if (CWADebug.buildFlavor == CWADebug.BuildFlavor.DEVICE_FOR_TESTERS) {
                 prefs.edit {
                     putString(PKEY_CURRENT_ENVINROMENT, value.rawKey)
                 }
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/TimeVariables.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/TimeVariables.kt
index a12d781a1..99a10696c 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/TimeVariables.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/TimeVariables.kt
@@ -1,13 +1,13 @@
 package de.rki.coronawarnapp.risk
 
 import com.google.android.gms.common.api.ApiException
-import de.rki.coronawarnapp.BuildConfig
 import de.rki.coronawarnapp.CoronaWarnApplication
 import de.rki.coronawarnapp.exception.ExceptionCategory
 import de.rki.coronawarnapp.exception.reporting.report
 import de.rki.coronawarnapp.nearby.InternalExposureNotificationClient
 import de.rki.coronawarnapp.storage.LocalData
 import de.rki.coronawarnapp.storage.tracing.TracingIntervalRepository
+import de.rki.coronawarnapp.util.CWADebug
 import de.rki.coronawarnapp.util.TimeAndDateExtensions.daysToMilliseconds
 import de.rki.coronawarnapp.util.TimeAndDateExtensions.roundUpMsToDays
 
@@ -99,21 +99,16 @@ object TimeVariables {
      * Delay in milliseconds for manual key retrieval process
      * Value for testing: 1 min =  1000 * 60 * 1
      * Value: 24 hours = 1000 * 60 * 60 * 24 milliseconds
+     *
+     * @return delay of key retrieval in milliseconds
      */
-    private val MANUAL_KEY_RETRIEVAL_DELAY =
-        if (BuildConfig.FLAVOR == "deviceForTesters") {
+    fun getManualKeyRetrievalDelay() =
+        if (CWADebug.buildFlavor == CWADebug.BuildFlavor.DEVICE_FOR_TESTERS) {
             MILISECONDS_IN_A_SECOND * SECONDS_IN_A_MINUTES
         } else {
             MILISECONDS_IN_A_SECOND * SECONDS_IN_A_MINUTES * MINUTES_IN_AN_HOUR * HOURS_IN_AN_DAY
         }
 
-    /**
-     * Getter function for [MANUAL_KEY_RETRIEVAL_DELAY]
-     *
-     * @return delay of key retrieval in milliseconds
-     */
-    fun getManualKeyRetrievalDelay() = MANUAL_KEY_RETRIEVAL_DELAY
-
     /**
      * This is the maximum attenuation duration value for the risk level calculation
      * in minutes
@@ -175,8 +170,8 @@ object TimeVariables {
      * @return in milliseconds
      */
     fun getTimeActiveTracingDuration(): Long = System.currentTimeMillis() -
-            (getInitialExposureTracingActivationTimestamp() ?: 0L) -
-            LocalData.totalNonActiveTracing()
+        (getInitialExposureTracingActivationTimestamp() ?: 0L) -
+        LocalData.totalNonActiveTracing()
 
     suspend fun getActiveTracingDaysInRetentionPeriod(): Long {
         // the active tracing time during the retention period - all non active tracing times
@@ -205,7 +200,8 @@ object TimeVariables {
         // the inactive intervals list to account for the time of inactivity between the last time
         // en was not active and now.
         if (enIsDisabled && lastNonActiveTracingTimestamp != null) {
-            val lastTimeTracingWasNotActivated = LocalData.lastNonActiveTracingTimestamp() ?: current
+            val lastTimeTracingWasNotActivated =
+                LocalData.lastNonActiveTracingTimestamp() ?: current
             inactiveTracingIntervals.add(Pair(lastTimeTracingWasNotActivated, current))
         }
         val inactiveTracingMS = inactiveTracingIntervals
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/calendar/CalendarView.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/calendar/CalendarView.kt
index c5045db27..cb545f2fe 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/calendar/CalendarView.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/calendar/CalendarView.kt
@@ -3,7 +3,6 @@ package de.rki.coronawarnapp.ui.calendar
 import android.content.Context
 import android.util.AttributeSet
 import android.view.LayoutInflater
-import android.view.View
 import android.widget.LinearLayout
 import android.widget.TextView
 import androidx.recyclerview.widget.GridLayoutManager
@@ -133,10 +132,10 @@ class CalendarView @JvmOverloads constructor(
         recyclerView.adapter = adapter
 
         // Setup day legend
-        setUpDayLegend(this)
+        setUpDayLegend()
 
         // Setup month
-        setUpMonthTextView(this)
+        setUpMonthTextView()
     }
 
     /**
@@ -160,10 +159,8 @@ class CalendarView @JvmOverloads constructor(
      * SetUp day legend (week day)
      *
      * NOTE: DaysOfWeek is impossible to use due to API 23
-     *
-     * @param view View - CalendarView
      */
-    private fun setUpDayLegend(view: View) {
+    private fun setUpDayLegend() {
         // Get day legend layout
         val dayLegendLayout = findViewById<LinearLayout>(R.id.calendar_day_legend)
         // Get current week day
@@ -173,8 +170,10 @@ class CalendarView @JvmOverloads constructor(
             val dayOfWeek = CalendarWeekDayView(context)
             val weekDay = date.withDayOfWeek(dayId).dayOfWeek()
             // weekDay.getAsText returns in either "Fri" or "Friday" format, substring first latter
-            dayOfWeek.setUp(weekDay.getAsText(Locale.getDefault()).take(1),
-                weekDay.get() == currentWeekDay)
+            dayOfWeek.setUp(
+                weekDay.getAsText(Locale.getDefault()).take(1),
+                weekDay.get() == currentWeekDay
+            )
             dayLegendLayout.addView(dayOfWeek)
         }
     }
@@ -182,11 +181,9 @@ class CalendarView @JvmOverloads constructor(
     /**
      * SetUp month text view
      *
-     * @param view View - CalendarView
-     *
      * @see CalendarCalculation.getMonthText
      */
-    private fun setUpMonthTextView(view: View) {
+    private fun setUpMonthTextView() {
         // Get month text view
         val monthTextView = findViewById<TextView>(R.id.calendar_month)
 
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/interoperability/InteroperabilityConfigurationFragment.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/interoperability/InteroperabilityConfigurationFragment.kt
index efe05031d..7b9f389a5 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/interoperability/InteroperabilityConfigurationFragment.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/interoperability/InteroperabilityConfigurationFragment.kt
@@ -33,6 +33,7 @@ class InteroperabilityConfigurationFragment :
         }
 
         override fun onNetworkUnavailable() {
+            // NOOP
         }
     }
 
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/fragment/SubmissionResultPositiveOtherWarningFragment.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/fragment/SubmissionResultPositiveOtherWarningFragment.kt
index 5f3595ba3..b864c0bc6 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/fragment/SubmissionResultPositiveOtherWarningFragment.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/fragment/SubmissionResultPositiveOtherWarningFragment.kt
@@ -7,7 +7,6 @@ import android.view.accessibility.AccessibilityEvent
 import androidx.activity.OnBackPressedCallback
 import androidx.fragment.app.Fragment
 import androidx.fragment.app.activityViewModels
-import androidx.lifecycle.Observer
 import androidx.navigation.fragment.findNavController
 import com.google.android.gms.nearby.exposurenotification.TemporaryExposureKey
 import de.rki.coronawarnapp.R
@@ -120,7 +119,7 @@ class SubmissionResultPositiveOtherWarningFragment :
             DialogHelper.showDialog(buildErrorDialog(it))
         }
 
-        submissionViewModel.submissionState.observe(viewLifecycleOwner, Observer {
+        submissionViewModel.submissionState.observe(viewLifecycleOwner, {
             if (it == ApiRequestState.SUCCESS) {
                 navigateToSubmissionDoneFragment()
             }
@@ -205,5 +204,6 @@ class SubmissionResultPositiveOtherWarningFragment :
     }
 
     override fun onFailure(exception: Exception?) {
+        // NOOP
     }
 }
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/CWADebug.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/CWADebug.kt
index 4c0373fa4..66d4d98b7 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/CWADebug.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/CWADebug.kt
@@ -14,11 +14,19 @@ object CWADebug {
         if (BuildConfig.DEBUG) {
             Timber.plant(Timber.DebugTree())
         }
-        if ((BuildConfig.FLAVOR == "deviceForTesters" || BuildConfig.DEBUG)) {
+        if ((buildFlavor == BuildFlavor.DEVICE_FOR_TESTERS || BuildConfig.DEBUG)) {
             fileLogger = FileLogger(application)
         }
     }
 
     val isDebugBuildOrMode: Boolean
-        get() = BuildConfig.DEBUG || BuildConfig.BUILD_VARIANT == "deviceForTesters"
+        get() = BuildConfig.DEBUG || buildFlavor == BuildFlavor.DEVICE_FOR_TESTERS
+
+    val buildFlavor: BuildFlavor
+        get() = BuildFlavor.values().single { it.rawValue == BuildConfig.BUILD_VARIANT }
+
+    enum class BuildFlavor(val rawValue: String) {
+        DEVICE("device"),
+        DEVICE_FOR_TESTERS("deviceForTesters")
+    }
 }
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/environment/EnvironmentSetupTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/environment/EnvironmentSetupTest.kt
index e0282e5f5..3dc9e4407 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/environment/EnvironmentSetupTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/environment/EnvironmentSetupTest.kt
@@ -52,7 +52,7 @@ class EnvironmentSetupTest : BaseTest() {
 
     @Test
     fun `mapping between function and JSON variable names is correct`() {
-        every { CWADebug.isDebugBuildOrMode } returns true
+        every { CWADebug.buildFlavor } returns CWADebug.BuildFlavor.DEVICE_FOR_TESTERS
         val envSetup = createEnvSetup()
 
         EnvironmentSetup.Type.values().forEach { type ->
@@ -70,7 +70,7 @@ class EnvironmentSetupTest : BaseTest() {
 
     @Test
     fun `default environment type is set correctly`() {
-        if (CWADebug.isDebugBuildOrMode) {
+        if (CWADebug.buildFlavor == CWADebug.BuildFlavor.DEVICE_FOR_TESTERS) {
             createEnvSetup().defaultEnvironment shouldBe EnvironmentSetup.Type.DEV
             createEnvSetup().alternativeEnvironment shouldBe EnvironmentSetup.Type.WRU_XA
         } else {
@@ -81,7 +81,7 @@ class EnvironmentSetupTest : BaseTest() {
 
     @Test
     fun `switching the default type is persisted in storage (preferences)`() {
-        if (CWADebug.isDebugBuildOrMode) {
+        if (CWADebug.buildFlavor == CWADebug.BuildFlavor.DEVICE_FOR_TESTERS) {
             createEnvSetup().apply {
                 defaultEnvironment shouldBe EnvironmentSetup.Type.DEV
                 currentEnvironment shouldBe EnvironmentSetup.Type.DEV
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/risk/TimeVariablesTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/risk/TimeVariablesTest.kt
index 457522af9..4055ddf1f 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/risk/TimeVariablesTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/risk/TimeVariablesTest.kt
@@ -1,7 +1,9 @@
 package de.rki.coronawarnapp.risk
 
-import de.rki.coronawarnapp.BuildConfig
+import de.rki.coronawarnapp.util.CWADebug
 import de.rki.coronawarnapp.util.TimeAndDateExtensions.daysToMilliseconds
+import io.mockk.every
+import io.mockk.mockkObject
 import org.junit.Assert
 import org.junit.Test
 
@@ -39,11 +41,12 @@ class TimeVariablesTest {
 
     @Test
     fun getManualKeyRetrievalDelay() {
-        if (BuildConfig.FLAVOR == "deviceForTesters") {
-            Assert.assertEquals(TimeVariables.getManualKeyRetrievalDelay(), 1000 * 60)
-        } else {
-            Assert.assertEquals(TimeVariables.getManualKeyRetrievalDelay(), 1000 * 60 * 60 * 24)
-        }
+        mockkObject(CWADebug)
+        every { CWADebug.buildFlavor } returns CWADebug.BuildFlavor.DEVICE_FOR_TESTERS
+        Assert.assertEquals(TimeVariables.getManualKeyRetrievalDelay(), 1000 * 60)
+
+        every { CWADebug.buildFlavor } returns CWADebug.BuildFlavor.DEVICE
+        Assert.assertEquals(TimeVariables.getManualKeyRetrievalDelay(), 1000 * 60 * 60 * 24)
     }
 
     @Test
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/PaddingToolTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/PaddingToolTest.kt
index e6cef8427..593f9f566 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/PaddingToolTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/PaddingToolTest.kt
@@ -3,6 +3,7 @@ package de.rki.coronawarnapp.util
 import io.kotest.matchers.shouldBe
 import org.junit.jupiter.api.Test
 import testhelpers.BaseTest
+import timber.log.Timber
 import kotlin.math.abs
 import kotlin.random.Random
 
@@ -12,10 +13,11 @@ class PaddingToolTest : BaseTest() {
 
     @Test
     fun `verify padding patterns`() {
-        repeat(100) {
-            val randomLength = abs(Random.nextInt(1024))
+        repeat(1000) {
+            val randomLength = abs(Random.nextInt(1, 1024))
             PaddingTool.requestPadding(randomLength).apply {
                 length shouldBe randomLength
+                Timber.v("RandomLength: %d, Padding: %s", randomLength, this)
                 validPattern.matches(this) shouldBe true
             }
         }
-- 
GitLab