From c13ba9d27f3bb79869b964541dbeb40d5f44388e Mon Sep 17 00:00:00 2001
From: Rambo Wang <rambowang@google.com>
Date: Thu, 20 Feb 2020 16:42:01 -0800
Subject: [PATCH] Polish developer option enable verbose vendor logging

Robotests cases in EnableVerboseVendorLoggingPreferenceControllerTest
show all pass but actually they are all bypassed due to the fact that
the test class can not grant sepolicy permission as what Settings
app does.

This version introduces mock DumpstateDevice HAL to really
perform the Robotests cases.

This CL also clear up code format issues.

Bug: 149886908
Test: make && make RunSettingsRoboTests
Change-Id: I64025ebea60e18d17338604519a6cae98ca7928b
---
 res/xml/development_settings.xml              |  3 +-
 ...boseVendorLoggingPreferenceController.java |  3 +-
 ...VendorLoggingPreferenceControllerTest.java | 40 ++++++++++++++-----
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/res/xml/development_settings.xml b/res/xml/development_settings.xml
index c6f06318362..c5920f00961 100644
--- a/res/xml/development_settings.xml
+++ b/res/xml/development_settings.xml
@@ -154,8 +154,7 @@
         <SwitchPreference
             android:key="enable_verbose_vendor_logging"
             android:title="@string/enable_verbose_vendor_logging"
-            android:summary="@string/enable_verbose_vendor_logging_summary"
-            />
+            android:summary="@string/enable_verbose_vendor_logging_summary" />
 
         <SwitchPreference
             android:key="automatic_system_server_heap_dumps"
diff --git a/src/com/android/settings/development/EnableVerboseVendorLoggingPreferenceController.java b/src/com/android/settings/development/EnableVerboseVendorLoggingPreferenceController.java
index 17e091e3768..e1db74d54c7 100644
--- a/src/com/android/settings/development/EnableVerboseVendorLoggingPreferenceController.java
+++ b/src/com/android/settings/development/EnableVerboseVendorLoggingPreferenceController.java
@@ -133,7 +133,8 @@ public class EnableVerboseVendorLoggingPreferenceController
     }
 
     /** Return a {@IDumpstateDevice} instance or null if service is not available. */
-    private @Nullable IDumpstateDevice getDumpstateDeviceService() {
+    @VisibleForTesting
+    @Nullable IDumpstateDevice getDumpstateDeviceService() {
         IDumpstateDevice service = null;
         try {
             service = android.hardware.dumpstate.V1_1.IDumpstateDevice
diff --git a/tests/robotests/src/com/android/settings/development/EnableVerboseVendorLoggingPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/development/EnableVerboseVendorLoggingPreferenceControllerTest.java
index 96f06fe3773..6145939dc40 100644
--- a/tests/robotests/src/com/android/settings/development/EnableVerboseVendorLoggingPreferenceControllerTest.java
+++ b/tests/robotests/src/com/android/settings/development/EnableVerboseVendorLoggingPreferenceControllerTest.java
@@ -18,10 +18,13 @@ package com.android.settings.development;
 import static junit.framework.Assert.assertFalse;
 import static junit.framework.Assert.assertTrue;
 
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import android.content.Context;
+import android.hardware.dumpstate.V1_1.IDumpstateDevice;
 
 import androidx.preference.PreferenceScreen;
 import androidx.preference.SwitchPreference;
@@ -34,24 +37,32 @@ import org.mockito.MockitoAnnotations;
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.RuntimeEnvironment;
 
+import java.lang.reflect.Field;
+
 @RunWith(RobolectricTestRunner.class)
 public final class EnableVerboseVendorLoggingPreferenceControllerTest {
     @Mock
     private SwitchPreference mPreference;
     @Mock
     private PreferenceScreen mPreferenceScreen;
+    @Mock
+    IDumpstateDevice mIDumpstateDevice;
 
     private Context mContext;
     private EnableVerboseVendorLoggingPreferenceController mController;
 
     @Before
-    public void setUp() {
+    public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
         mContext = RuntimeEnvironment.application;
-        mController = new EnableVerboseVendorLoggingPreferenceController(mContext);
+        mController = spy(new EnableVerboseVendorLoggingPreferenceController(mContext));
+        doReturn(mIDumpstateDevice).when(mController).getDumpstateDeviceService();
 
-        // bypass if IDumpstateDevice service not avalaible at all
-        org.junit.Assume.assumeTrue(mController.isIDumpstateDeviceV1_1ServiceAvailable());
+        // mock with Dumpstate HAL v1.1
+        Field f = EnableVerboseVendorLoggingPreferenceController.class
+                .getDeclaredField("mDumpstateHalVersion");
+        f.setAccessible(true);
+        f.setInt(mController, 1 /* DUMPSTATE_HAL_VERSION_1_1 */);
 
         when(mPreferenceScreen.findPreference(mController.getPreferenceKey()))
                 .thenReturn(mPreference);
@@ -59,7 +70,9 @@ public final class EnableVerboseVendorLoggingPreferenceControllerTest {
     }
 
     @Test
-    public void onPreferenceChange_settingEnable_enableVendorLoggingShouldBeOn() {
+    public void onPreferenceChange_settingEnable_enableVendorLoggingShouldBeOn() throws Exception {
+        doReturn(true).when(mIDumpstateDevice).getVerboseLoggingEnabled();
+
         mController.onPreferenceChange(mPreference, true /* new value */);
 
         final boolean enabled = mController.getVerboseLoggingEnabled();
@@ -67,7 +80,10 @@ public final class EnableVerboseVendorLoggingPreferenceControllerTest {
     }
 
     @Test
-    public void onPreferenceChange_settingDisable_enableVendorLoggingShouldBeOff() {
+    public void onPreferenceChange_settingDisable_enableVendorLoggingShouldBeOff()
+            throws Exception {
+        doReturn(false).when(mIDumpstateDevice).getVerboseLoggingEnabled();
+
         mController.onPreferenceChange(mPreference,  false /* new value */);
 
         final boolean enabled = mController.getVerboseLoggingEnabled();
@@ -75,7 +91,9 @@ public final class EnableVerboseVendorLoggingPreferenceControllerTest {
     }
 
     @Test
-    public void updateState_settingDisabled_preferenceShouldNotBeChecked() {
+    public void updateState_settingDisabled_preferenceShouldNotBeChecked() throws Exception {
+        doReturn(false).when(mIDumpstateDevice).getVerboseLoggingEnabled();
+
         mController.setVerboseLoggingEnabled(false);
         mController.updateState(mPreference);
 
@@ -83,7 +101,9 @@ public final class EnableVerboseVendorLoggingPreferenceControllerTest {
     }
 
     @Test
-    public void updateState_settingEnabled_preferenceShouldBeChecked() {
+    public void updateState_settingEnabled_preferenceShouldBeChecked() throws Exception {
+        doReturn(true).when(mIDumpstateDevice).getVerboseLoggingEnabled();
+
         mController.setVerboseLoggingEnabled(true);
         mController.updateState(mPreference);
 
@@ -91,7 +111,9 @@ public final class EnableVerboseVendorLoggingPreferenceControllerTest {
     }
 
     @Test
-    public void onDeveloperOptionDisabled_shouldDisablePreference() {
+    public void onDeveloperOptionDisabled_shouldDisablePreference() throws Exception {
+        doReturn(false).when(mIDumpstateDevice).getVerboseLoggingEnabled();
+
         mController.onDeveloperOptionsSwitchDisabled();
 
         final boolean enabled = mController.getVerboseLoggingEnabled();