Cleanup and fix CI tests (#1464)

10 files changed, 162 insertions, 53 deletions

diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml
index b94c34f3..994a5ca0 100644
--- a/.github/workflows/pull_request.yml
+++ b/.github/workflows/pull_request.yml
@@ -54,7 +54,7 @@ jobs:
       if: ${{ steps.service-changed.outputs.result == 'true' }}
       uses: burrunan/gradle-cache-action@03c71a8ba93d670980695505f48f49daf43704a6
       with:
-        arguments: apiCheck testFreeDebug lintFreeDebug spotlessCheck
+        arguments: apiCheck test lintFreeDebug spotlessCheck -PslimTests
 
     - name: (Fail-only) upload test report
       if: failure()
diff --git a/app/build.gradle.kts b/app/build.gradle.kts
index 887d26aa..adb2fcbc 100644
--- a/app/build.gradle.kts
+++ b/app/build.gradle.kts
@@ -60,17 +60,10 @@ android {
     disable("CoroutineCreationDuringComposition")
   }
 
-  flavorDimensions("free")
-  productFlavors {
-    create("free") {}
-    create("nonFree") {}
-  }
-  testOptions { unitTests.isReturnDefaultValues = true }
-
-  composeOptions {
-    kotlinCompilerVersion = libs.versions.kotlin.get()
-    kotlinCompilerExtensionVersion = libs.versions.compose.get()
-  }
+  //  composeOptions {
+  //    kotlinCompilerVersion = libs.versions.kotlin.get()
+  //    kotlinCompilerExtensionVersion = libs.versions.compose.get()
+  //  }
 }
 
 dependencies {
diff --git a/app/src/main/java/dev/msfjarvis/aps/util/totp/UriTotpFinder.kt b/app/src/main/java/dev/msfjarvis/aps/util/totp/UriTotpFinder.kt
index a420fe5d..ecf5f1ca 100644
--- a/app/src/main/java/dev/msfjarvis/aps/util/totp/UriTotpFinder.kt
+++ b/app/src/main/java/dev/msfjarvis/aps/util/totp/UriTotpFinder.kt
@@ -13,10 +13,10 @@ class UriTotpFinder @Inject constructor() : TotpFinder {
 
   override fun findSecret(content: String): String? {
     content.split("\n".toRegex()).forEach { line ->
-      if (line.startsWith(TOTP_FIELDS[0])) {
+      if (line.startsWith(TotpFinder.TOTP_FIELDS[0])) {
         return Uri.parse(line).getQueryParameter("secret")
       }
-      if (line.startsWith(TOTP_FIELDS[1], ignoreCase = true)) {
+      if (line.startsWith(TotpFinder.TOTP_FIELDS[1], ignoreCase = true)) {
         return line.split(": *".toRegex(), 2).toTypedArray()[1]
       }
     }
@@ -42,15 +42,11 @@ class UriTotpFinder @Inject constructor() : TotpFinder {
   private fun getQueryParameter(content: String, parameterName: String): String? {
     content.split("\n".toRegex()).forEach { line ->
       val uri = Uri.parse(line)
-      if (line.startsWith(TOTP_FIELDS[0]) && uri.getQueryParameter(parameterName) != null) {
+      if (line.startsWith(TotpFinder.TOTP_FIELDS[0]) && uri.getQueryParameter(parameterName) != null
+      ) {
         return uri.getQueryParameter(parameterName)
       }
     }
     return null
   }
-
-  companion object {
-
-    val TOTP_FIELDS = arrayOf("otpauth://totp", "totp:")
-  }
 }
diff --git a/buildSrc/src/main/java/BaseProjectConfig.kt b/buildSrc/src/main/java/BaseProjectConfig.kt
index a829d4f7..e4dfa7e4 100644
--- a/buildSrc/src/main/java/BaseProjectConfig.kt
+++ b/buildSrc/src/main/java/BaseProjectConfig.kt
@@ -13,6 +13,7 @@ import org.gradle.api.tasks.wrapper.Wrapper
 import org.gradle.kotlin.dsl.maven
 import org.gradle.kotlin.dsl.repositories
 import org.gradle.kotlin.dsl.withType
+import org.gradle.language.nativeplatform.internal.BuildType
 import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
 
 /**
@@ -50,9 +51,10 @@ internal fun Project.configureForAllProjects() {
       languageVersion = "1.5"
     }
   }
-  tasks.withType<Test> {
+  tasks.withType<Test>().configureEach {
     maxParallelForks = Runtime.getRuntime().availableProcessors() * 2
     testLogging { events(TestLogEvent.PASSED, TestLogEvent.SKIPPED, TestLogEvent.FAILED) }
+    outputs.upToDateWhen { false }
   }
 }
 
@@ -79,13 +81,19 @@ internal fun BaseAppModuleExtension.configureAndroidApplicationOptions(project:
     buildConfig = true
   }
 
+  flavorDimensions(FlavorDimensions.FREE)
+  productFlavors {
+    create(ProductFlavors.FREE) {}
+    create(ProductFlavors.NON_FREE) {}
+  }
+
   buildTypes {
-    named("release") {
+    named(BuildType.RELEASE.name) {
       isMinifyEnabled = !minifySwitch.isPresent
       setProguardFiles(listOf("proguard-android-optimize.txt", "proguard-rules.pro"))
       buildConfigField("boolean", "ENABLE_DEBUG_FEATURES", "${project.isSnapshot()}")
     }
-    named("debug") {
+    named(BuildType.DEBUG.name) {
       applicationIdSuffix = ".debug"
       versionNameSuffix = "-debug"
       isMinifyEnabled = false
@@ -121,5 +129,8 @@ internal fun TestedExtension.configureCommonAndroidOptions() {
     targetCompatibility = JavaVersion.VERSION_1_8
   }
 
-  testOptions.animationsDisabled = true
+  testOptions {
+    animationsDisabled = true
+    unitTests.isReturnDefaultValues = true
+  }
 }
diff --git a/buildSrc/src/main/java/PasswordStorePlugin.kt b/buildSrc/src/main/java/PasswordStorePlugin.kt
index 0705de82..df491e0d 100644
--- a/buildSrc/src/main/java/PasswordStorePlugin.kt
+++ b/buildSrc/src/main/java/PasswordStorePlugin.kt
@@ -43,6 +43,7 @@ class PasswordStorePlugin : Plugin<Project> {
         is LibraryPlugin -> {
           project.extensions.getByType<TestedExtension>().configureCommonAndroidOptions()
           project.configureExplicitApi()
+          project.configureSlimTests()
         }
         is AppPlugin -> {
           project
@@ -51,6 +52,7 @@ class PasswordStorePlugin : Plugin<Project> {
             .configureAndroidApplicationOptions(project)
           project.extensions.getByType<BaseAppModuleExtension>().configureBuildSigning(project)
           project.extensions.getByType<TestedExtension>().configureCommonAndroidOptions()
+          project.configureSlimTests()
         }
         is SigningPlugin -> {
           project.extensions.getByType<SigningExtension>().configureBuildSigning()
diff --git a/buildSrc/src/main/java/ProductFlavors.kt b/buildSrc/src/main/java/ProductFlavors.kt
new file mode 100644
index 00000000..5b722a37
--- /dev/null
+++ b/buildSrc/src/main/java/ProductFlavors.kt
@@ -0,0 +1,13 @@
+/*
+ * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved.
+ * SPDX-License-Identifier: GPL-3.0-only
+ */
+
+object FlavorDimensions {
+  const val FREE = "free"
+}
+
+object ProductFlavors {
+  const val FREE = "free"
+  const val NON_FREE = "nonFree"
+}
diff --git a/buildSrc/src/main/java/SlimTests.kt b/buildSrc/src/main/java/SlimTests.kt
new file mode 100644
index 00000000..65e9c618
--- /dev/null
+++ b/buildSrc/src/main/java/SlimTests.kt
@@ -0,0 +1,38 @@
+/*
+ * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved.
+ * SPDX-License-Identifier: GPL-3.0-only
+ */
+
+import com.android.build.api.extension.ApplicationAndroidComponentsExtension
+import com.android.build.api.extension.LibraryAndroidComponentsExtension
+import org.gradle.api.Project
+import org.gradle.kotlin.dsl.findByType
+import org.gradle.language.nativeplatform.internal.BuildType
+
+/**
+ * When the "slimTests" project property is provided, disable the unit test tasks on `release` build
+ * type and `nonFree` product flavor to avoid running the same tests repeatedly in different build
+ * variants.
+ *
+ * Examples: `./gradlew test -PslimTests` will run unit tests for `nonFreeDebug` and `debug` build
+ * variants in Android App and Library projects, and all tests in JVM projects.
+ */
+internal fun Project.configureSlimTests() {
+  if (providers.gradleProperty(SLIM_TESTS_PROPERTY).forUseAtConfigurationTime().isPresent) {
+    // disable unit test tasks on the release build type for Android Library projects
+    extensions.findByType<LibraryAndroidComponentsExtension>()?.run {
+      beforeUnitTests(selector().withBuildType(BuildType.RELEASE.name)) { it.enabled = false }
+    }
+
+    // disable unit test tasks on the release build type and free flavor for Android Application
+    // projects.
+    extensions.findByType<ApplicationAndroidComponentsExtension>()?.run {
+      beforeUnitTests(selector().withBuildType(BuildType.RELEASE.name)) { it.enabled = false }
+      beforeUnitTests(selector().withFlavor(FlavorDimensions.FREE to ProductFlavors.NON_FREE)) {
+        it.enabled = false
+      }
+    }
+  }
+}
+
+private const val SLIM_TESTS_PROPERTY = "slimTests"
diff --git a/format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntry.kt b/format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntry.kt
index 0e8f6d2e..5a2b59ad 100644
--- a/format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntry.kt
+++ b/format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntry.kt
@@ -122,8 +122,7 @@ constructor(
             foundUsername = true
             false
           }
-          line.startsWith("otpauth://", ignoreCase = true) ||
-            line.startsWith("totp:", ignoreCase = true) -> {
+          TotpFinder.TOTP_FIELDS.any { prefix -> line.startsWith(prefix, ignoreCase = true) } -> {
             false
           }
           else -> {
diff --git a/format-common/src/test/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntryTest.kt b/format-common/src/test/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntryTest.kt
index edf4dc10..df4dc42e 100644
--- a/format-common/src/test/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntryTest.kt
+++ b/format-common/src/test/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntryTest.kt
@@ -186,6 +186,9 @@ internal class PasswordEntryTest {
         override fun findAlgorithm(content: String): String {
           return "SHA1"
         }
+        override fun findIssuer(content: String): String {
+          return "ACME Co"
+        }
       }
   }
 }
diff --git a/format-common/src/test/kotlin/dev/msfjarvis/aps/util/totp/OtpTest.kt b/format-common/src/test/kotlin/dev/msfjarvis/aps/util/totp/OtpTest.kt
index 4c01ac74..a2de94d8 100644
--- a/format-common/src/test/kotlin/dev/msfjarvis/aps/util/totp/OtpTest.kt
+++ b/format-common/src/test/kotlin/dev/msfjarvis/aps/util/totp/OtpTest.kt
@@ -13,19 +13,35 @@ import org.junit.Test
 
 internal class OtpTest {
 
+  private fun generateOtp(
+    counter: Long,
+    secret: String = "JBSWY3DPEHPK3PXP",
+    algorithm: String = "SHA1",
+    digits: String = "6",
+    issuer: String? = null,
+  ): String? {
+    return Otp.calculateCode(secret, counter, algorithm, digits, issuer).get()
+  }
+
   @Test
   fun testOtpGeneration6Digits() {
     assertEquals(
       "953550",
-      Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333298159 / (1000 * 30), "SHA1", "6").get()
+      generateOtp(
+        counter = 1593333298159 / (1000 * 30),
+      )
     )
     assertEquals(
       "275379",
-      Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333571918 / (1000 * 30), "SHA1", "6").get()
+      generateOtp(
+        counter = 1593333571918 / (1000 * 30),
+      )
     )
     assertEquals(
       "867507",
-      Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333600517 / (1000 * 57), "SHA1", "6").get()
+      generateOtp(
+        counter = 1593333600517 / (1000 * 57),
+      )
     )
   }
 
@@ -33,36 +49,79 @@ internal class OtpTest {
   fun testOtpGeneration10Digits() {
     assertEquals(
       "0740900914",
-      Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333655044 / (1000 * 30), "SHA1", "10").get()
+      generateOtp(
+        counter = 1593333655044 / (1000 * 30),
+        digits = "10",
+      )
     )
     assertEquals(
       "0070632029",
-      Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333691405 / (1000 * 30), "SHA1", "10").get()
+      generateOtp(
+        counter = 1593333691405 / (1000 * 30),
+        digits = "10",
+      )
     )
     assertEquals(
       "1017265882",
-      Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333728893 / (1000 * 83), "SHA1", "10").get()
+      generateOtp(
+        counter = 1593333728893 / (1000 * 83),
+        digits = "10",
+      )
     )
   }
 
   @Test
   fun testOtpGenerationIllegalInput() {
-    assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA0", "10").get())
-    assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA1", "a").get())
-    assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA1", "5").get())
-    assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA1", "11").get())
-    assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXPAAAAB", 10000, "SHA1", "6").get())
+    assertNull(
+      generateOtp(
+        counter = 10000,
+        algorithm = "SHA0",
+        digits = "10",
+      )
+    )
+    assertNull(
+      generateOtp(
+        counter = 10000,
+        digits = "a",
+      )
+    )
+    assertNull(
+      generateOtp(
+        counter = 10000,
+        algorithm = "SHA1",
+        digits = "5",
+      )
+    )
+    assertNull(
+      generateOtp(
+        counter = 10000,
+        digits = "11",
+      )
+    )
+    assertNull(
+      generateOtp(
+        counter = 10000,
+        secret = "JBSWY3DPEHPK3PXPAAAAB",
+        digits = "6",
+      )
+    )
   }
 
   @Test
   fun testOtpGenerationUnusualSecrets() {
     assertEquals(
       "127764",
-      Otp.calculateCode("JBSWY3DPEHPK3PXPAAAAAAAA", 1593367111963 / (1000 * 30), "SHA1", "6").get()
+      generateOtp(
+        counter = 1593367111963 / (1000 * 30),
+        secret = "JBSWY3DPEHPK3PXPAAAAAAAA",
+      )
     )
     assertEquals(
       "047515",
-      Otp.calculateCode("JBSWY3DPEHPK3PXPAAAAA", 1593367171420 / (1000 * 30), "SHA1", "6").get()
+      generateOtp(
+        counter = 1593367171420 / (1000 * 30),
+        secret = "JBSWY3DPEHPK3PXPAAAAA",
+      )
     )
   }
 
@@ -72,21 +131,16 @@ internal class OtpTest {
     // We don't care for the resultant OTP's actual value, we just want both the padded and
     // unpadded variant to generate the same one.
     val unpaddedOtp =
-      Otp.calculateCode(
-          "ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA",
-          1593367171420 / (1000 * 30),
-          "SHA1",
-          "6"
-        )
-        .get()
+      generateOtp(
+        counter = 1593367171420 / (1000 * 30),
+        secret = "ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA",
+      )
     val paddedOtp =
-      Otp.calculateCode(
-          "ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA====",
-          1593367171420 / (1000 * 30),
-          "SHA1",
-          "6"
-        )
-        .get()
+      generateOtp(
+        1593367171420 / (1000 * 30),
+        secret = "ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA====",
+      )
+
     assertNotNull(unpaddedOtp)
     assertNotNull(paddedOtp)
     assertEquals(unpaddedOtp, paddedOtp)