Skip to content

Extracts setup and teardown to BasePurchasesTest#1011

Merged
vegaro merged 5 commits into
mainfrom
cesar/sdk-3136-create-basepurchasestest-in-android-so
May 20, 2023
Merged

Extracts setup and teardown to BasePurchasesTest#1011
vegaro merged 5 commits into
mainfrom
cesar/sdk-3136-create-basepurchasestest-in-android-so

Conversation

@vegaro

@vegaro vegaro commented May 19, 2023

Copy link
Copy Markdown
Member

Extracted setup and tearDown from PurchasesTest.kt so it's easier not only to split Purchases.kt into different test files, but also it's easier to create new tests that depend on creation of a Purchases object.

@vegaro vegaro added the test label May 19, 2023
protected val randomAppUserId = "\$RCAnonymousID:ff68f26e432648369a713849a9f93b58"
protected val appUserId = "fakeUserID"
protected lateinit var purchases: Purchases
protected val mockInfo = mockk<CustomerInfo>()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept here properties that are used in BasePurchasesTest, whatever is only used in PurchasesTest, meaning it's not needed for setup and teardown is kept in Purchases.kt

Whatever is in this new class has been directly copied from Purchases.kt

@@ -143,43 +116,7 @@ class PurchasesTest {

private val mockLifecycle = mockk<Lifecycle>()
private val mockLifecycleOwner = mockk<LifecycleOwner>()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be some properties that will probably be needed by different test classes and could be moved to BasePurchasesTest, but we can extract them whenever we need to.

queriedINAPP = emptyMap(),
notInCache = listOf(activePurchasedPurchase, activePendingPurchase, activeUnspecifiedPurchase)
)
val productInfo = mockQueryingProductDetails("product", ProductType.SUBS, null)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was unused

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol I broke this test for not thinking haha

@vegaro vegaro requested a review from a team May 19, 2023 03:24

@tonidero tonidero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!! We are chipping away that PurchasesTest class 💪

protected lateinit var purchases: Purchases
protected val mockInfo = mockk<CustomerInfo>()

@After

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Total nitpick but I kinda prefer to have the @After after the @Before 😅

}

protected fun buildPurchases(anonymous: Boolean, autoSync: Boolean = true) {
purchases = Purchases(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only other place we build this in tests currently is in SubscriberAttributesPurchasesTests. Might be good trying to have that use this BasePurchasesTest. But we can do that as a followup.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's do it as a followup, otherwise the pr gets harder to review

@NachoSoto NachoSoto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great refactor 🙏

Comment thread purchases/src/test/java/com/revenuecat/purchases/BasePurchasesTest.kt Outdated
@vegaro

vegaro commented May 19, 2023

Copy link
Copy Markdown
Member Author

oh weird one test is failing 🤔

@codecov

codecov Bot commented May 20, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1011 (e6d5b14) into main (c924d62) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1011   +/-   ##
=======================================
  Coverage   85.35%   85.35%           
=======================================
  Files         168      168           
  Lines        5997     5997           
  Branches      837      837           
=======================================
  Hits         5119     5119           
  Misses        546      546           
  Partials      332      332           

@vegaro vegaro merged commit 3dc486d into main May 20, 2023
@vegaro vegaro deleted the cesar/sdk-3136-create-basepurchasestest-in-android-so branch May 20, 2023 01:49
This was referenced May 24, 2023
tonidero added a commit that referenced this pull request May 25, 2023
**This is an automatic release.**

### New Features
* Support DEFERRED mode (#985) via swehner (@swehner)
* Add completion callback to syncPurchases API (#1002) via Toni Rico
(@tonidero)
### Bugfixes
* Workaround bug in android 4 for JSON objects with List<String> (#942)
via Andy Boedo (@aboedo)
### Dependency Updates
* Bump fastlane-plugin-revenuecat_internal from `fe45299` to `13773d2`
(#1015) via dependabot[bot] (@dependabot[bot])
### Other Changes
* Bump dokka to 1.8.10 to support Gradle 8 (#1009) via Toni Rico
(@tonidero)
* Disable offline entitlements temporarily (#1023) via Toni Rico
(@tonidero)
* Fix integration tests in CI (#1019) via Toni Rico (@tonidero)
* Add offline entitlements integration tests (#1006) via Toni Rico
(@tonidero)
* Disable offline entitlements in observer mode (#1014) via Toni Rico
(@tonidero)
* Extracts setup and teardown to BasePurchasesTest (#1011) via Cesar de
la Vega (@vegaro)
* Support forcing server errors for tests (#1008) via Toni Rico
(@tonidero)

---------

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants