Testing and stability improvements (#696)

.github/workflows/build.yml

@@ -1,4 +1,5 @@
 # https://help.github.com/en/actions/automating-your-workflow-with-github-actions/workflow-syntax-for-github-actions
+# Renaming ? Change the README badge.
 name: Build
 on:
   push:
@@ -37,28 +38,46 @@ jobs:
     name: Emulator Tests
     runs-on: macOS-latest
     strategy:
+      fail-fast: false
       matrix:
-        # TODO 29 fails due to Mockito issues, probably reproducible locally.
-        # 22, 23, 24, 25, 26, 27, 28 work - some of them, with SdkExclude restrictions.
+        # TODO 29 fails due to Mockito issues, probably reproducible locally
+        # 22-28 work (some of them, with SdkExclude restrictions)
         EMULATOR_API: [22, 23, 24, 25, 26, 27, 28]
-        EMULATOR_ARCH: [x86_64]
+        include:
+          - EMULATOR_API: 28
+            EMULATOR_ARCH: x86_64
+          - EMULATOR_API: 27
+            EMULATOR_ARCH: x86_64
+          - EMULATOR_API: 26
+            EMULATOR_ARCH: x86_64
+          - EMULATOR_API: 25
+            EMULATOR_ARCH: x86
+          - EMULATOR_API: 24
+            EMULATOR_ARCH: x86
+          - EMULATOR_API: 23
+            EMULATOR_ARCH: x86
+          - EMULATOR_API: 22
+            EMULATOR_ARCH: x86
     steps:
       - uses: actions/checkout@v1
       - uses: actions/setup-java@v1
         with:
           java-version: 1.8
       - name: Execute emulator tests
-        uses: reactivecircus/android-emulator-runner@v2
+        timeout-minutes: 20
+        uses: reactivecircus/[email protected]
         with:
           api-level: ${{ matrix.EMULATOR_API }}
           arch: ${{ matrix.EMULATOR_ARCH }}
           disable-animations: true
-          emulator-options: -no-snapshot -no-window -no-boot-anim -camera-back emulated -camera-front emulated -memory 2048
-          script: ./gradlew cameraview:connectedCheck
+          profile: Nexus 5X
+          emulator-options: -no-snapshot -no-window -no-boot-anim -camera-back emulated -camera-front emulated -gpu swiftshader_indirect
+          emulator-build: 6031357
+          script: ./.github/workflows/emulator_script.sh
       - name: Upload emulator tests artifact
         uses: actions/upload-artifact@v1
         with:
-          name: emulator_tests
+          name: emulator_tests_${{ matrix.EMULATOR_API }}
           path: ./cameraview/build/outputs/code_coverage/debugAndroidTest/connected
   CODE_COVERAGE:
     name: Code Coverage Report
@@ -72,13 +91,15 @@ jobs:
       - name: Download unit tests artifact
         uses: actions/download-artifact@v1
         with:
-            name: unit_tests
-            path: ./cameraview/build/jacoco/
+          name: unit_tests
+          path: ./cameraview/build/jacoco/
       - name: Download emulator tests artifact
         uses: actions/download-artifact@v1
         with:
-            name: emulator_tests
-            path: ./cameraview/build/outputs/code_coverage/debugAndroidTest/connected
+          # 27 is the EMULATOR_API with less SdkExclude annotations, and should have
+          # the best possible coverage.
+          name: emulator_tests_27
+          path: ./cameraview/build/outputs/code_coverage/debugAndroidTest/connected
       - name: Create merged coverage report
         run: ./gradlew cameraview:mergeCoverageReports
       - name: Upload merged coverage report (GitHub)

.github/workflows/emulator_script.sh

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+# Core
+ADB_TAGS="CameraView:I CameraCallbacks:I CameraOrchestrator:I CameraEngine:I"
+ADB_TAGS="$ADB_TAGS CameraUtils:I WorkerHandler:I"
+# Recorders
+ADB_TAGS="$ADB_TAGS VideoRecorder:I FullVideoRecorder:I SnapshotVideoRecorder:I"
+ADB_TAGS="$ADB_TAGS FullPictureRecorder:I SnapshotPictureRecorder:I DeviceEncoders:I"
+# Video encoders
+ADB_TAGS="$ADB_TAGS MediaEncoderEngine:I MediaEncoder:I AudioMediaEncoder:I VideoMediaEncoder:I TextureMediaEncoder:I"
+# Debugging
+ADB_TAGS="$ADB_TAGS CameraIntegrationTest:I MessageQueue:W MPEG4Writer:I"
+adb logcat -c
+adb logcat $ADB_TAGS *:E -v color &
+./gradlew cameraview:connectedCheck

cameraview/build.gradle

@@ -17,7 +17,9 @@ android {
         versionCode 1
         versionName project.version
         testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
-        testInstrumentationRunnerArgument "filter", "com.otaliastudios.cameraview.tools.SdkExcludeFilter"
+        testInstrumentationRunnerArgument "filter", "" +
+                "com.otaliastudios.cameraview.tools.SdkExcludeFilter," +
+                "com.otaliastudios.cameraview.tools.SdkIncludeFilter"
     }
 
     buildTypes {
@@ -241,17 +243,16 @@ task mergeCoverageReports(type: JacocoReport) {
     if (isCI) {
         // All these classes are tested by the integration tests that we are not able to
         // run on the CI emulator.
-        classFilter.add('**/com/otaliastudios/cameraview/engine/CameraEngine**.*')
-        classFilter.add('**/com/otaliastudios/cameraview/engine/Camera1Engine**.*')
-        classFilter.add('**/com/otaliastudios/cameraview/engine/Camera2Engine**.*')
-        classFilter.add('**/com/otaliastudios/cameraview/engine/action/**.*')
-        classFilter.add('**/com/otaliastudios/cameraview/engine/lock/**.*')
-        classFilter.add('**/com/otaliastudios/cameraview/engine/meter/**.*')
-        classFilter.add('**/com/otaliastudios/cameraview/picture/**.*')
-        classFilter.add('**/com/otaliastudios/cameraview/video/**.*')
-        // TODO these below could be easily testable ALSO outside of the integration tests
-        classFilter.add('**/com/otaliastudios/cameraview/orchestrator/**.*')
-        classFilter.add('**/com/otaliastudios/cameraview/video/encoding/**.*')
+        // classFilter.add('**/com/otaliastudios/cameraview/engine/CameraEngine**.*')
+        // classFilter.add('**/com/otaliastudios/cameraview/engine/Camera1Engine**.*')
+        // classFilter.add('**/com/otaliastudios/cameraview/engine/Camera2Engine**.*')
+        // classFilter.add('**/com/otaliastudios/cameraview/engine/action/**.*')
+        // classFilter.add('**/com/otaliastudios/cameraview/engine/lock/**.*')
+        // classFilter.add('**/com/otaliastudios/cameraview/engine/meter/**.*')
+        // classFilter.add('**/com/otaliastudios/cameraview/picture/**.*')
+        // classFilter.add('**/com/otaliastudios/cameraview/video/**.*')
+        // classFilter.add('**/com/otaliastudios/cameraview/orchestrator/**.*')
+        // classFilter.add('**/com/otaliastudios/cameraview/video/encoding/**.*')
     }
     // We don't test OpenGL filters.
     classFilter.add('**/com/otaliastudios/cameraview/filters/**.*')

cameraview/src/androidTest/java/com/otaliastudios/cameraview/BaseTest.java

@@ -10,6 +10,7 @@
 
 import androidx.annotation.NonNull;
 import androidx.test.platform.app.InstrumentationRegistry;
+import androidx.test.rule.GrantPermissionRule;
 
 import com.otaliastudios.cameraview.tools.Op;
 
@@ -89,19 +90,6 @@ protected static void waitUiIdle() {
         InstrumentationRegistry.getInstrumentation().waitForIdleSync();
     }
 
-    protected static void grantAllPermissions() {
-        grantPermission("android.permission.CAMERA");
-        grantPermission("android.permission.RECORD_AUDIO");
-        grantPermission("android.permission.WRITE_EXTERNAL_STORAGE");
-    }
-
-    @SuppressWarnings("WeakerAccess")
-    protected static void grantPermission(@NonNull String permission) {
-        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) return;
-        String command = "pm grant " + getContext().getPackageName() + " " + permission;
-        InstrumentationRegistry.getInstrumentation().getUiAutomation().executeShellCommand(command);
-    }
-
     @NonNull
     protected static Stubber doCountDown(@NonNull final CountDownLatch latch) {
         return doAnswer(new Answer<Object>() {

cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewTest.java

@@ -6,6 +6,7 @@
 import android.graphics.PointF;
 import android.location.Location;
 import androidx.annotation.NonNull;
+import androidx.test.annotation.UiThreadTest;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import androidx.test.filters.MediumTest;
 
@@ -135,7 +136,7 @@ public void testClose() {
     public void testDestroy() {
         cameraView.destroy();
         verify(mockPreview, times(1)).onDestroy();
-        verify(mockController, times(1)).destroy();
+        verify(mockController, times(1)).destroy(true);
     }
 
     //region testDefaults

cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/Camera1IntegrationTest.java

@@ -18,8 +18,8 @@
  */
 @RunWith(AndroidJUnit4.class)
 @LargeTest
-@RequiresDevice
-public class Camera1IntegrationTest extends CameraIntegrationTest {
+// @RequiresDevice
+public class Camera1IntegrationTest extends CameraIntegrationTest<Camera1Engine> {
 
     @NonNull
     @Override

cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/Camera2IntegrationTest.java

@@ -1,7 +1,9 @@
 package com.otaliastudios.cameraview.engine;
 
+import android.hardware.camera2.CameraCharacteristics;
 import android.hardware.camera2.CaptureRequest;
 import android.hardware.camera2.TotalCaptureResult;
+import android.os.Handler;
 
 import com.otaliastudios.cameraview.controls.Engine;
 import com.otaliastudios.cameraview.engine.action.ActionHolder;
@@ -24,8 +26,8 @@
  */
 @RunWith(AndroidJUnit4.class)
 @LargeTest
-@RequiresDevice
-public class Camera2IntegrationTest extends CameraIntegrationTest {
+// @RequiresDevice
+public class Camera2IntegrationTest extends CameraIntegrationTest<Camera2Engine> {
 
     @NonNull
     @Override
@@ -39,7 +41,6 @@ protected void onOpenSync() {
         // Extra wait for the first frame to be dispatched.
         // This is because various classes require getLastResult to be non-null
         // and that's typically the case in a real app.
-        Camera2Engine engine = (Camera2Engine) controller;
         final CountDownLatch latch = new CountDownLatch(1);
         new BaseAction() {
             @Override
@@ -50,12 +51,29 @@ public void onCaptureCompleted(@NonNull ActionHolder holder,
                 latch.countDown();
                 setState(STATE_COMPLETED);
             }
-        }.start(engine);
+        }.start(controller);
         try { latch.await(); } catch (InterruptedException ignore) {}
     }
 
     @Override
     protected long getMeteringTimeoutMillis() {
         return Camera2Engine.METER_TIMEOUT;
     }
+
+    /**
+     * setMaxDuration can crash on legacy devices (most emulator are), and I don't see
+     * any way to fix this in code. They shouldn't use Camera2 at all.
+     * @return true if possible.
+     */
+    @Override
+    protected boolean canSetVideoMaxDuration() {
+        if (!super.canSetVideoMaxDuration()) return false;
+        boolean shouldOpen = !camera.isOpened();
+        if (shouldOpen) openSync(true);
+        boolean result = controller.readCharacteristic(
+                CameraCharacteristics.INFO_SUPPORTED_HARDWARE_LEVEL, -1)
+                != CameraCharacteristics.INFO_SUPPORTED_HARDWARE_LEVEL_LEGACY;
+        if (shouldOpen) closeSync(true);
+        return result;
+    }
 }