From ccf15f79f08f2c96cffdc46c4f357d3a545df08e Mon Sep 17 00:00:00 2001 From: Michael Groover Date: Thu, 9 Aug 2018 16:12:42 -0700 Subject: [PATCH] Resolve NPE and unexpected error code in AccountManagerServiceTest ag/3915086 modified AccountManagerService to use the PackageManagerInternal returned from LocalServices. However when AccountManagerServiceTest runs tests against AccountManagerService it is using a local instance of the object which does not have access to system's local PackageManagerInternal. To resolve this a mock PackageManagerInternal is added to the LocalServices with a mocked hasSignatureCapability that returns an appropriate response based on the test. This fix also exposed an issue that appears to be introduced by ag/2959363; with the change in checkKeyIntent to no longer throw a SecurityException the expected return code for error paths is now AccountManager.ERROR_CODE_REMOTE_EXCEPTION. A small change was also added to AccountsDbTest to skip testCeNotAvailableInitially on eng builds as the Log.wtf would crash the test process when invoked on eng builds. Test: adb shell am instrument -w -e package com.android.server.accounts \ com.android.frameworks.servicestests/\ android.support.test.runner.AndroidJUnitRunner Bug: 80380639 Change-Id: I9438550d1c19cc86ec56991da766d71e068784ab Merged-In: I9438550d1c19cc86ec56991da766d71e068784ab --- .../tests/servicestests/AndroidManifest.xml | 1 + .../accounts/AccountManagerServiceTest.java | 40 +++++++++++++++---- .../server/accounts/AccountsDbTest.java | 7 ++++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/services/tests/servicestests/AndroidManifest.xml b/services/tests/servicestests/AndroidManifest.xml index a8efe819e5183..d46b99170ded3 100644 --- a/services/tests/servicestests/AndroidManifest.xml +++ b/services/tests/servicestests/AndroidManifest.xml @@ -65,6 +65,7 @@ + mIntentCaptor; @Captor private ArgumentCaptor mBundleCaptor; @@ -158,6 +160,9 @@ public class AccountManagerServiceTest extends AndroidTestCase { when(mMockContext.getSystemService(Context.DEVICE_POLICY_SERVICE)).thenReturn( mMockDevicePolicyManager); when(mMockAccountManagerResponse.asBinder()).thenReturn(mMockBinder); + when(mMockPackageManagerInternal.hasSignatureCapability(anyInt(), anyInt(), anyInt())) + .thenReturn(true); + LocalServices.addService(PackageManagerInternal.class, mMockPackageManagerInternal); Context realTestContext = getContext(); MyMockContext mockContext = new MyMockContext(realTestContext, mMockContext); @@ -177,6 +182,7 @@ public class AccountManagerServiceTest extends AndroidTestCase { cdl.countDown(); }); cdl.await(1, TimeUnit.SECONDS); + LocalServices.removeServiceForTest(PackageManagerInternal.class); super.tearDown(); } @@ -610,6 +616,8 @@ public class AccountManagerServiceTest extends AndroidTestCase { any(Intent.class), anyInt(), anyInt())).thenReturn(resolveInfo); when(mMockPackageManager.checkSignatures( anyInt(), anyInt())).thenReturn(PackageManager.SIGNATURE_NO_MATCH); + when(mMockPackageManagerInternal.hasSignatureCapability(anyInt(), anyInt(), anyInt())) + .thenReturn(false); final CountDownLatch latch = new CountDownLatch(1); Response response = new Response(latch, mMockAccountManagerResponse); @@ -626,7 +634,7 @@ public class AccountManagerServiceTest extends AndroidTestCase { waitForLatch(latch); verify(mMockAccountManagerResponse, never()).onResult(any(Bundle.class)); verify(mMockAccountManagerResponse).onError( - eq(AccountManager.ERROR_CODE_REMOTE_EXCEPTION), anyString()); + eq(AccountManager.ERROR_CODE_INVALID_RESPONSE), anyString()); } @SmallTest @@ -792,6 +800,8 @@ public class AccountManagerServiceTest extends AndroidTestCase { any(Intent.class), anyInt(), anyInt())).thenReturn(resolveInfo); when(mMockPackageManager.checkSignatures( anyInt(), anyInt())).thenReturn(PackageManager.SIGNATURE_NO_MATCH); + when(mMockPackageManagerInternal.hasSignatureCapability(anyInt(), anyInt(), anyInt())) + .thenReturn(false); final CountDownLatch latch = new CountDownLatch(1); Response response = new Response(latch, mMockAccountManagerResponse); @@ -808,7 +818,7 @@ public class AccountManagerServiceTest extends AndroidTestCase { waitForLatch(latch); verify(mMockAccountManagerResponse, never()).onResult(any(Bundle.class)); verify(mMockAccountManagerResponse).onError( - eq(AccountManager.ERROR_CODE_REMOTE_EXCEPTION), anyString()); + eq(AccountManager.ERROR_CODE_INVALID_RESPONSE), anyString()); } @SmallTest @@ -1092,6 +1102,8 @@ public class AccountManagerServiceTest extends AndroidTestCase { any(Intent.class), anyInt(), anyInt())).thenReturn(resolveInfo); when(mMockPackageManager.checkSignatures( anyInt(), anyInt())).thenReturn(PackageManager.SIGNATURE_NO_MATCH); + when(mMockPackageManagerInternal.hasSignatureCapability(anyInt(), anyInt(), anyInt())) + .thenReturn(false); final CountDownLatch latch = new CountDownLatch(1); Response response = new Response(latch, mMockAccountManagerResponse); @@ -1106,7 +1118,7 @@ public class AccountManagerServiceTest extends AndroidTestCase { waitForLatch(latch); verify(mMockAccountManagerResponse, never()).onResult(any(Bundle.class)); verify(mMockAccountManagerResponse).onError( - eq(AccountManager.ERROR_CODE_REMOTE_EXCEPTION), anyString()); + eq(AccountManager.ERROR_CODE_INVALID_RESPONSE), anyString()); } @SmallTest @@ -1352,6 +1364,8 @@ public class AccountManagerServiceTest extends AndroidTestCase { unlockSystemUser(); when(mMockPackageManager.checkSignatures(anyInt(), anyInt())) .thenReturn(PackageManager.SIGNATURE_NO_MATCH); + when(mMockPackageManagerInternal.hasSignatureCapability(anyInt(), anyInt(), anyInt())) + .thenReturn(false); try { mAms.removeAccountAsUser( mMockAccountManagerResponse, // response @@ -1688,6 +1702,8 @@ public class AccountManagerServiceTest extends AndroidTestCase { any(Intent.class), anyInt(), anyInt())).thenReturn(resolveInfo); when(mMockPackageManager.checkSignatures( anyInt(), anyInt())).thenReturn(PackageManager.SIGNATURE_NO_MATCH); + when(mMockPackageManagerInternal.hasSignatureCapability(anyInt(), anyInt(), anyInt())) + .thenReturn(false); final CountDownLatch latch = new CountDownLatch(1); Response response = new Response(latch, mMockAccountManagerResponse); @@ -1701,7 +1717,7 @@ public class AccountManagerServiceTest extends AndroidTestCase { waitForLatch(latch); verify(mMockAccountManagerResponse, never()).onResult(any(Bundle.class)); verify(mMockAccountManagerResponse).onError( - eq(AccountManager.ERROR_CODE_REMOTE_EXCEPTION), anyString()); + eq(AccountManager.ERROR_CODE_INVALID_RESPONSE), anyString()); } @SmallTest @@ -1959,6 +1975,8 @@ public class AccountManagerServiceTest extends AndroidTestCase { any(Intent.class), anyInt(), anyInt())).thenReturn(resolveInfo); when(mMockPackageManager.checkSignatures( anyInt(), anyInt())).thenReturn(PackageManager.SIGNATURE_NO_MATCH); + when(mMockPackageManagerInternal.hasSignatureCapability(anyInt(), anyInt(), anyInt())) + .thenReturn(false); final CountDownLatch latch = new CountDownLatch(1); Response response = new Response(latch, mMockAccountManagerResponse); @@ -1974,7 +1992,7 @@ public class AccountManagerServiceTest extends AndroidTestCase { waitForLatch(latch); verify(mMockAccountManagerResponse, never()).onResult(any(Bundle.class)); verify(mMockAccountManagerResponse).onError( - eq(AccountManager.ERROR_CODE_REMOTE_EXCEPTION), anyString()); + eq(AccountManager.ERROR_CODE_INVALID_RESPONSE), anyString()); } @SmallTest @@ -2097,6 +2115,8 @@ public class AccountManagerServiceTest extends AndroidTestCase { any(Intent.class), anyInt(), anyInt())).thenReturn(resolveInfo); when(mMockPackageManager.checkSignatures( anyInt(), anyInt())).thenReturn(PackageManager.SIGNATURE_NO_MATCH); + when(mMockPackageManagerInternal.hasSignatureCapability(anyInt(), anyInt(), anyInt())) + .thenReturn(false); final CountDownLatch latch = new CountDownLatch(1); Response response = new Response(latch, mMockAccountManagerResponse); @@ -2110,7 +2130,7 @@ public class AccountManagerServiceTest extends AndroidTestCase { verify(mMockAccountManagerResponse, never()).onResult(any(Bundle.class)); verify(mMockAccountManagerResponse).onError( - eq(AccountManager.ERROR_CODE_REMOTE_EXCEPTION), anyString()); + eq(AccountManager.ERROR_CODE_INVALID_RESPONSE), anyString()); } @SmallTest @@ -2230,6 +2250,8 @@ public class AccountManagerServiceTest extends AndroidTestCase { any(Intent.class), anyInt(), anyInt())).thenReturn(resolveInfo); when(mMockPackageManager.checkSignatures( anyInt(), anyInt())).thenReturn(PackageManager.SIGNATURE_NO_MATCH); + when(mMockPackageManagerInternal.hasSignatureCapability(anyInt(), anyInt(), anyInt())) + .thenReturn(false); final CountDownLatch latch = new CountDownLatch(1); Response response = new Response(latch, mMockAccountManagerResponse); @@ -2245,7 +2267,7 @@ public class AccountManagerServiceTest extends AndroidTestCase { verify(mMockAccountManagerResponse, never()).onResult(any(Bundle.class)); verify(mMockAccountManagerResponse).onError( - eq(AccountManager.ERROR_CODE_REMOTE_EXCEPTION), anyString()); + eq(AccountManager.ERROR_CODE_INVALID_RESPONSE), anyString()); } @SmallTest @@ -2332,6 +2354,8 @@ public class AccountManagerServiceTest extends AndroidTestCase { unlockSystemUser(); when(mMockPackageManager.checkSignatures(anyInt(), anyInt())) .thenReturn(PackageManager.SIGNATURE_NO_MATCH); + when(mMockPackageManagerInternal.hasSignatureCapability(anyInt(), anyInt(), anyInt())) + .thenReturn(false); try { mAms.editProperties( mMockAccountManagerResponse, // response @@ -2621,6 +2645,8 @@ public class AccountManagerServiceTest extends AndroidTestCase { PackageManager.PERMISSION_DENIED); when(mMockPackageManager.checkSignatures(anyInt(), anyInt())) .thenReturn(PackageManager.SIGNATURE_NO_MATCH); + when(mMockPackageManagerInternal.hasSignatureCapability(anyInt(), anyInt(), anyInt())) + .thenReturn(false); final CountDownLatch latch = new CountDownLatch(1); Response response = new Response(latch, mMockAccountManagerResponse); diff --git a/services/tests/servicestests/src/com/android/server/accounts/AccountsDbTest.java b/services/tests/servicestests/src/com/android/server/accounts/AccountsDbTest.java index 5d0c23f817379..ce9b552e418ae 100644 --- a/services/tests/servicestests/src/com/android/server/accounts/AccountsDbTest.java +++ b/services/tests/servicestests/src/com/android/server/accounts/AccountsDbTest.java @@ -22,6 +22,7 @@ import android.content.Context; import android.database.Cursor; import android.support.test.InstrumentationRegistry; import android.support.test.runner.AndroidJUnit4; +import android.os.Build; import android.test.suitebuilder.annotation.SmallTest; import android.util.Pair; @@ -85,6 +86,12 @@ public class AccountsDbTest { @Test public void testCeNotAvailableInitially() { + // If the CE database is not attached to the DE database then any calls that modify the CE + // database will result in a Log.wtf call that will crash this process on eng builds. To + // allow the test to run through to completion skip this test on eng builds. + if (Build.IS_ENG) { + return; + } Account account = new Account("name", "example.com"); long id = mAccountsDb.insertCeAccount(account, ""); assertEquals("Insert into CE should fail until CE database is attached", -1, id);