[#560] Detect strict mode violations on CI

.github/workflows/pull-request.yml

@@ -313,7 +313,49 @@ jobs:
           name: Test Android modules with WTF results
           path: ~/artifacts
 
-  release_build:
+
+  # Performs a button mash test on the debug build of the app with strict mode enabled
+  test_robo_debug:
+    if: needs.check_firebase_secrets.outputs.has-secrets == 'true'
+    needs: [check_firebase_secrets]
+    runs-on: ubuntu-latest
+    permissions:
+      packages: read
+      contents: read
+      id-token: write
+    steps:
+      - name: Checkout
+        timeout-minutes: 1
+        uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b
+      - name: Setup
+        id: setup
+        timeout-minutes: 5
+        uses: ./.github/actions/setup
+      - name: Build
+        timeout-minutes: 20
+        env:
+          ORG_GRADLE_PROJECT_IS_CRASH_ON_STRICT_MODE_VIOLATION: true
+        run: |
+          ./gradlew :app:assembleDebug
+      - name: Authenticate to Google Cloud for Firebase Test Lab
+        id: auth_test_lab
+        uses: google-github-actions/auth@ceee102ec2387dd9e844e01b530ccd4ec87ce955
+        with:
+          create_credentials_file: true
+          project_id: ${{ secrets.FIREBASE_TEST_LAB_PROJECT }}
+          service_account: ${{ secrets.FIREBASE_TEST_LAB_SERVICE_ACCOUNT }}
+          workload_identity_provider: ${{ secrets.FIREBASE_TEST_LAB_WORKLOAD_IDENTITY_PROVIDER }}
+          access_token_lifetime: '900s'
+      - name: Robo test
+        timeout-minutes: 20
+        env:
+          # This first environment variable is used by Flank, since the temporary token is missing the project name
+          GOOGLE_CLOUD_PROJECT: ${{ secrets.FIREBASE_TEST_LAB_PROJECT }}
+          ORG_GRADLE_PROJECT_ZCASH_FIREBASE_TEST_LAB_API_KEY_PATH: ${{ steps.auth_test_lab.outputs.credentials_file_path }}
+        run: |
+          ./gradlew :app:runFlankSanityConfigDebug
+
+  build:
     needs: prime_cache
     runs-on: ubuntu-latest
     permissions:
@@ -360,10 +402,9 @@ jobs:
           path: ~/artifacts
 
   # Performs a button mash test on the release build of the app
-  # Note that we might need to help it get past the onboarding test with a script
-  test_robo:
+  test_robo_release:
     if: needs.check_firebase_secrets.outputs.has-secrets == 'true'
-    needs: [release_build, check_firebase_secrets]
+    needs: [build, check_firebase_secrets]
     runs-on: ubuntu-latest
     permissions:
       packages: read
@@ -400,4 +441,4 @@ jobs:
           ORG_GRADLE_PROJECT_ZCASH_FIREBASE_TEST_LAB_API_KEY_PATH: ${{ steps.auth_test_lab.outputs.credentials_file_path }}
         run: |
           unzip ${BINARIES_ZIP_PATH}
-          ./gradlew :app:runFlankSanityConfig
+          ./gradlew :app:runFlankSanityConfigRelease

app/build.gradle.kts

@@ -104,6 +104,13 @@ android {
                 signingConfig = signingConfigs.getByName("release")
             }
         }
+        all {
+            buildConfigField(
+                "boolean",
+                "IS_STRICT_MODE_CRASH_ENABLED",
+                project.property("IS_CRASH_ON_STRICT_MODE_VIOLATION").toString()
+            )
+        }
     }
 
     // Resolve final app name
@@ -112,16 +119,16 @@ android {
         val debugAppNameSuffix = project.property("ZCASH_DEBUG_APP_NAME_SUFFIX").toString()
         when (this.name) {
             "zcashtestnetDebug" -> {
-                resValue( "string", "app_name", "$defaultAppName ($testnetNetworkName)$debugAppNameSuffix")
+                resValue("string", "app_name", "$defaultAppName ($testnetNetworkName)$debugAppNameSuffix")
             }
             "zcashmainnetDebug" -> {
-                resValue( "string", "app_name", "$defaultAppName$debugAppNameSuffix")
+                resValue("string", "app_name", "$defaultAppName$debugAppNameSuffix")
             }
             "zcashtestnetRelease" -> {
-                resValue( "string", "app_name", "$defaultAppName ($testnetNetworkName)")
+                resValue("string", "app_name", "$defaultAppName ($testnetNetworkName)")
             }
             "zcashmainnetRelease" -> {
-                resValue( "string", "app_name", defaultAppName)
+                resValue("string", "app_name", defaultAppName)
             }
         }
     }
@@ -265,7 +272,25 @@ fladle {
     }
 
     configs {
-        create("sanityConfig") {
+        create("sanityConfigDebug") {
+            clearPropertiesForSanityRobo()
+
+            debugApk.set(
+                project.provider {
+                    "${buildDir}/outputs/apk/zcashmainnet/debug/app-zcashmainnet-debug.apk"
+                }
+            )
+
+            testTimeout.set("3m")
+
+            devices.addAll(
+                mapOf("model" to "Pixel2", "version" to minSdkVersion),
+                mapOf("model" to "Pixel2", "version" to targetSdkVersion)
+            )
+
+            flankVersion.set(libs.versions.flank.get())
+        }
+        create("sanityConfigRelease") {
             clearPropertiesForSanityRobo()
 
             debugApk.set(

app/src/main/java/co/electriccoin/zcash/app/ZcashApplication.kt

@@ -15,7 +15,7 @@ class ZcashApplication : Application() {
         Twig.info { "Starting application…" }
 
         if (BuildConfig.DEBUG) {
-            StrictModeCompat.enableStrictMode()
+            StrictModeCompat.enableStrictMode(BuildConfig.IS_STRICT_MODE_CRASH_ENABLED)
 
             // This is an internal API to the Zcash SDK to enable logging; it could change in the future
             cash.z.ecc.android.sdk.internal.Twig.enabled(true)

gradle.properties

@@ -26,6 +26,10 @@ IS_ANDROID_INSTRUMENTATION_TEST_COVERAGE_ENABLED=false
 # It is disabled by default, because it causes tests to take about 2x longer to run.
 IS_USE_TEST_ORCHESTRATOR=false
 
+# Optionally enable crashes for strict mode violations in debug builds.
+# It is disabled by default, because it can be annoying when debugging. Gets turned on by CI jobs that need it.
+IS_CRASH_ON_STRICT_MODE_VIOLATION=false
+
 # Either provide a path to a Firebase Test Lab service key (best for CI)
 # OR
 # login with `./gradlew flankAuth` and provide the project name (best for local development)

spackle-android-lib/src/main/kotlin/co/electriccoin/zcash/spackle/StrictModeCompat.kt

@@ -8,26 +8,32 @@ import android.os.StrictMode
 
 object StrictModeCompat {
 
-    fun enableStrictMode() {
-        configureStrictMode()
+    fun enableStrictMode(isCrashOnViolation: Boolean) {
+        configureStrictMode(isCrashOnViolation)
 
         // Workaround for Android bug
         // https://issuetracker.google.com/issues/36951662
         // Not needed if target O_MR1 and running on O_MR1
         // Don't really need to check target, because of Google Play enforcement on targetSdkVersion for app updates
         if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O_MR1) {
-            Handler(Looper.getMainLooper()).postAtFrontOfQueue { configureStrictMode() }
+            Handler(Looper.getMainLooper()).postAtFrontOfQueue {
+                configureStrictMode(isCrashOnViolation)
+            }
         }
     }
 
     @SuppressLint("NewApi")
-    private fun configureStrictMode() {
+    private fun configureStrictMode(isCrashOnViolation: Boolean) {
         StrictMode.enableDefaults()
 
         StrictMode.setThreadPolicy(
             StrictMode.ThreadPolicy.Builder().apply {
                 detectAll()
-                penaltyLog()
+                if (isCrashOnViolation) {
+                    penaltyDeath()
+                } else {
+                    penaltyLog()
+                }
             }.build()
         )
 
@@ -46,13 +52,23 @@ object StrictModeCompat {
                         // Disable because this is mostly flagging Android X and Play Services
                         // builder.detectNonSdkApiUsage();
                     }
+
+                    if (isCrashOnViolation) {
+                        penaltyDeath()
+                    } else {
+                        penaltyLog()
+                    }
                 }.build()
             )
         } else {
             StrictMode.setVmPolicy(
                 StrictMode.VmPolicy.Builder().apply {
                     detectAll()
-                    penaltyLog()
+                    if (isCrashOnViolation) {
+                        penaltyDeath()
+                    } else {
+                        penaltyLog()
+                    }
                 }.build()
             )
         }