Skip to content

Commit

Permalink
For mozilla-mobile#9838: Fix onCreditCardSave tests.
Browse files Browse the repository at this point in the history 
Previous implementation made the tests pass regardless of the results in verify/assert.
  • Loading branch information
@mcarare
mcarare committed May 10, 2022
1 parent bf7ed58 commit 7004226
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 27 deletions.
4 changes: 2 additions & 2 deletions ...java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ 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 A [DefaultCreditCardValidationDelegate] used to check if a card can be saved in the [storage].
*/
class GeckoCreditCardsAddressesStorageDelegate(
private val storage: Lazy<CreditCardsAddressesStorage>,
private val scope: CoroutineScope = CoroutineScope(Dispatchers.IO),
private val validationDelegate: DefaultCreditCardValidationDelegate = DefaultCreditCardValidationDelegate(storage),
private val isCreditCardAutofillEnabled: () -> Boolean = { false }
) : CreditCardsAddressesStorageDelegate {

Expand Down Expand Up @@ -64,8 +66,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 -> {
Expand Down
56 changes: 31 additions & 25 deletions .../mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegateTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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()
Expand All @@ -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
Expand All @@ -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()
}
Expand Down Expand Up @@ -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,
Expand All @@ -144,41 +148,43 @@ 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,
expiryYear = expiryYear,
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
Expand Down

0 comments on commit 7004226

Please sign in to comment.