From bf7966aef227b12bb7c7f991d86b0628a266b7ae Mon Sep 17 00:00:00 2001 From: mcarare Date: Tue, 10 May 2022 19:16:41 +0300 Subject: [PATCH] For #9838: Fix onCreditCardSave tests. Previous implementation made the tests pass regardless of the results in verify/assert. --- ...eckoCreditCardsAddressesStorageDelegate.kt | 5 +- ...CreditCardsAddressesStorageDelegateTest.kt | 56 ++++++++++--------- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt index 1c49c81722e..20ce1b3c9b3 100644 --- a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt +++ b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt @@ -26,10 +26,13 @@ import mozilla.components.support.ktx.kotlin.last4Digits * @param storage The [CreditCardsAddressesStorage] used for looking up addresses and credit cards to autofill. * @param scope [CoroutineScope] for long running operations. Defaults to using the [Dispatchers.IO]. * @param isCreditCardAutofillEnabled callback allowing to limit [storage] operations if autofill is disabled. + * @param validationDelegate The [DefaultCreditCardValidationDelegate] used to check if a credit card + * can be saved in [storage] and returns information about why it can or cannot */ class GeckoCreditCardsAddressesStorageDelegate( private val storage: Lazy, private val scope: CoroutineScope = CoroutineScope(Dispatchers.IO), + private val validationDelegate: DefaultCreditCardValidationDelegate = DefaultCreditCardValidationDelegate(storage), private val isCreditCardAutofillEnabled: () -> Boolean = { false } ) : CreditCardsAddressesStorageDelegate { @@ -64,8 +67,6 @@ class GeckoCreditCardsAddressesStorageDelegate( } override suspend fun onCreditCardSave(creditCard: CreditCardEntry) { - val validationDelegate = DefaultCreditCardValidationDelegate(storage) - scope.launch { when (val result = validationDelegate.shouldCreateOrUpdate(creditCard)) { is CreditCardValidationDelegate.Result.CanBeCreated -> { diff --git a/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegateTest.kt b/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegateTest.kt index 514749f0fa5..b16361a0d69 100644 --- a/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegateTest.kt +++ b/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegateTest.kt @@ -6,11 +6,11 @@ package mozilla.components.service.sync.autofill import androidx.test.ext.junit.runners.AndroidJUnit4 import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest import mozilla.components.concept.storage.CreditCard import mozilla.components.concept.storage.CreditCardEntry import mozilla.components.concept.storage.CreditCardNumber +import mozilla.components.concept.storage.CreditCardValidationDelegate import mozilla.components.concept.storage.NewCreditCardFields import mozilla.components.concept.storage.UpdatableCreditCardFields import mozilla.components.lib.dataprotect.SecureAbove22Preferences @@ -26,6 +26,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito.doReturn import org.mockito.Mockito.never +import org.mockito.Mockito.spy import org.mockito.Mockito.times import org.mockito.Mockito.verify @@ -40,6 +41,7 @@ class GeckoCreditCardsAddressesStorageDelegateTest { private lateinit var storage: AutofillCreditCardsAddressesStorage private lateinit var securePrefs: SecureAbove22Preferences private lateinit var delegate: GeckoCreditCardsAddressesStorageDelegate + private val validationDelegate: DefaultCreditCardValidationDelegate = mock() init { testContext.getDatabasePath(AUTOFILL_DB_NAME)!!.parentFile!!.mkdirs() @@ -49,8 +51,8 @@ class GeckoCreditCardsAddressesStorageDelegateTest { fun before() { // forceInsecure is set in the tests because a keystore wouldn't be configured in the test environment. securePrefs = SecureAbove22Preferences(testContext, "autofill", forceInsecure = true) - storage = AutofillCreditCardsAddressesStorage(testContext, lazy { securePrefs }) - delegate = GeckoCreditCardsAddressesStorageDelegate(lazy { storage }, scope) + storage = spy(AutofillCreditCardsAddressesStorage(testContext, lazy { securePrefs })) + delegate = GeckoCreditCardsAddressesStorageDelegate(lazy { storage }, scope, validationDelegate) } @Test @@ -76,7 +78,7 @@ class GeckoCreditCardsAddressesStorageDelegateTest { @Test fun `WHEN onAddressFetch is called THEN the storage is called to gett all addresses`() { - scope.launch { + runTest { delegate.onAddressesFetch() verify(storage, times(1)).getAllAddresses() } @@ -112,23 +114,25 @@ class GeckoCreditCardsAddressesStorageDelegateTest { @Test fun `GIVEN a new credit card WHEN onCreditCardSave is called THEN it adds a new credit card in storage`() { - scope.launch { + runTest { val billingName = "Jon Doe" val cardNumber = "4111111111111111" val expiryMonth = 12L val expiryYear = 2028L val cardType = "amex" - delegate.onCreditCardSave( - CreditCardEntry( - name = billingName, - number = cardNumber, - expiryMonth = expiryMonth.toString(), - expiryYear = expiryYear.toString(), - cardType = cardType - ) + val creditCardEntry = CreditCardEntry( + name = billingName, + number = cardNumber, + expiryMonth = expiryMonth.toString(), + expiryYear = expiryYear.toString(), + cardType = cardType ) + doReturn(CreditCardValidationDelegate.Result.CanBeCreated).`when`(validationDelegate).shouldCreateOrUpdate(creditCardEntry) + + delegate.onCreditCardSave(creditCardEntry) + verify(storage, times(1)).addCreditCard( creditCardFields = NewCreditCardFields( billingName = billingName, @@ -144,16 +148,24 @@ class GeckoCreditCardsAddressesStorageDelegateTest { @Test fun `GIVEN an existing credit card WHEN onCreditCardSave is called THEN it updates the existing credit card in storage`() { - scope.launch { + runTest { val billingName = "Jon Doe" val cardNumber = "4111111111111111" val expiryMonth = 12L val expiryYear = 2028L val cardType = "amex" + val creditCardEntry = CreditCardEntry( + name = billingName, + number = cardNumber, + expiryMonth = expiryMonth.toString(), + expiryYear = expiryYear.toString(), + cardType = cardType + ) + val creditCard = storage.addCreditCard( NewCreditCardFields( - billingName = "Jon Doe", + billingName = billingName, plaintextCardNumber = CreditCardNumber.Plaintext(cardNumber), cardNumberLast4 = "1111", expiryMonth = expiryMonth, @@ -161,24 +173,18 @@ class GeckoCreditCardsAddressesStorageDelegateTest { cardType = cardType ) ) + doReturn(CreditCardValidationDelegate.Result.CanBeUpdated(creditCard)).`when`(validationDelegate).shouldCreateOrUpdate(creditCardEntry) delegate.onCreditCardSave( - CreditCardEntry( - guid = creditCard.guid, - name = billingName, - number = "4111111111111112", - expiryMonth = expiryMonth.toString(), - expiryYear = expiryYear.toString(), - cardType = cardType - ) + creditCardEntry ) verify(storage, times(1)).updateCreditCard( guid = creditCard.guid, creditCardFields = UpdatableCreditCardFields( billingName = billingName, - cardNumber = CreditCardNumber.Plaintext("4111111111111112"), - cardNumberLast4 = "4111111111111112".last4Digits(), + cardNumber = CreditCardNumber.Plaintext("4111111111111111"), + cardNumberLast4 = "4111111111111111".last4Digits(), expiryMonth = expiryMonth, expiryYear = expiryYear, cardType = cardType