Trusted entitlements: Add integration tests by tonidero · Pull Request #1071 · RevenueCat/purchases-android

tonidero · 2023-06-15T09:04:47Z

Description

This adds integration tests to verify the basic behavior of the trusted entitlements feature.

codecov · 2023-06-15T09:15:38Z

Codecov Report

Merging #1071 (dc6a1fa) into main (d669a99) will increase coverage by 0.00%.
The diff coverage is 90.00%.

❗ Current head dc6a1fa differs from pull request most recent head 3d0190f. Consider uploading reports for the commit 3d0190f to get more accurate results

@@           Coverage Diff           @@
##             main    #1071   +/-   ##
=======================================
  Coverage   85.64%   85.64%           
=======================================
  Files         176      176           
  Lines        6247     6256    +9     
  Branches      859      861    +2     
=======================================
+ Hits         5350     5358    +8     
  Misses        563      563           
- Partials      334      335    +1

Impacted Files	Coverage Δ
.../java/com/revenuecat/purchases/common/AppConfig.kt	`81.63% <66.66%> (-0.98%)`	⬇️
...at/purchases/common/verification/SigningManager.kt	`100.00% <100.00%> (ø)`
...otlin/com/revenuecat/purchases/PurchasesFactory.kt	`84.21% <100.00%> (+0.24%)`	⬆️

NachoSoto

Great start 👏🏻 just a question about test setup to be able to test changing conditions (I still have to add tests like that in iOS too).

NachoSoto · 2023-06-15T15:02:56Z

I wish there was a way to ensure this cannot run in production builds

Yeah honestly it's not great... With the server version of this I was thinking of a way by creating a new class that exists in each flavor (the integration test and the defaults flavors). The defaults flavor version wouldn't do anything and the integration test flavor would indicate the failure, however, we would still have to use that code here and it would complicate things more... I would really like to have what we have in iOS where we can make code compile only with certain flags but that's more difficult to architecture in Android and not sure if it's worth it...

Actually having a file that's different on each flavor, wouldn't it be possible to at least detect the flavor at runtime?
So AppConfig for example can't even allow this value being true in the "release" flavor.

Good idea! We actually can get the flavor from BuildConfig.FLAVOR without extra files. We can use that to do what you said yeah. It would still have this code in the final bundle, but it would be another safety measure, I can do it like that 👍

Maybe you're thinking of something better, but we could change AppConfig so that some values can never return anything but the default depending on BuildConfig.FLAVOR.

Done in the base PR: e16c410

NachoSoto · 2023-06-15T15:03:39Z

Maybe to be overly cautious it would help to write a unit test to verify that this is false by default, as a way to ensure production builds never ship with this on?

Done in the base PR in 371d747

NachoSoto · 2023-06-15T15:04:20Z

Maybe TrustedEntitlementsInformationalModeIntegrationTest? Since you might want to test successes too?
Oh I see you have a separate one. I'm usually not against splitting test classes, easier to maintain. But I think it might be weird because it would be really useful to add tests that verify the behavior when the signature goes from correct to invalid and vice-versa. What do you think?

Yeah I added one in the success file, but it's true it doesn't fully fit... I was torn because I didn't want to make a huge test class and it's easier to setup the condition in the @Before of the test, but well I think we don't have that many currently and we can indeed modify these things later in the integration test, so I will change it back to only one class.

Done in dc6a1fa

NachoSoto · 2023-06-16T16:34:29Z

Can't wait to see these with coroutines 😅

Pretty straightforward and neat 💅 #1077

NachoSoto · 2023-06-16T16:35:15Z

This is nice 👍🏻

@tonidero

**This is an automatic release.** ### Bugfixes * Default customer info schema version to latest known by SDK (#1080) via Toni Rico (@tonidero) * Handle other diagnostics-related exceptions (#1076) via Toni Rico (@tonidero) * Return error in queryPurchases if error connecting to billing client (#1072) via Toni Rico (@tonidero) ### Other Changes * Fix offline entitlements integration tests (#1085) via Toni Rico (@tonidero) * Add defaultsRelease variant tests run configuration (#1074) via Toni Rico (@tonidero) * Compose sample app: move to gradle catalog (#1081) via Toni Rico (@tonidero) * Compose sample app: automate builds (#1082) via Toni Rico (@tonidero) * Compose sample app (#1056) via Toni Rico (@tonidero) * Migrate to Gradle version catalog (#1059) via Cesar de la Vega (@vegaro) * Trusted entitlements: Add logs with verification mode (#1067) via Toni Rico (@tonidero) * Sync pending purchases before getting customer info (#1073) via Toni Rico (@tonidero) * Refactor syncing pending transactions logic out of `Purchases` (#1058) via Toni Rico (@tonidero) * Refactor CustomerInfo listener and cache logic into CustomerInfoUpdater (#1052) via Toni Rico (@tonidero) * Trusted entitlements: Add integration tests (#1071) via Toni Rico (@tonidero) * Trusted entitlements: Add internal mechanism to force signing errors for tests (#1070) via Toni Rico (@tonidero) Co-authored-by: revenuecat-ops <ops@revenuecat.com>

@tonidero

…tlementsInformationalModeIntegrationTest` (#1077)  ### Checklist - [ ] If applicable, unit tests - [ ] If applicable, create follow-up issues for `purchases-ios` and hybrids ### Motivation **Why is this change required? What problem does it solve?** Follow up from #1071 (comment)  ### Description **Describe your changes in detail** - Adds `awaitCustomerInfo` / coroutines tests to `TrustedEntitlementsInformationalModeIntegrationTest` **Please describe in detail how you tested your changes** - Run integration tests locally ✅ cc @tonidero @vegaro @NachoSoto

@pablo-guardiola

…tlementsInformationalModeIntegrationTest` (#1077) via @pablo-guardiola (#1107)  ### Checklist - [ ] If applicable, unit tests - [ ] If applicable, create follow-up issues for `purchases-ios` and hybrids ### Motivation **Why is this change required? What problem does it solve?** Follow up from #1071 (comment)  ### Description **Describe your changes in detail** - Adds `awaitCustomerInfo` / coroutines tests to `TrustedEntitlementsInformationalModeIntegrationTest` **Please describe in detail how you tested your changes** - Run integration tests locally ✅ Contributed by @pablo-guardiola Co-authored-by: pablo-guardiola <131195486+pablo-guardiola@users.noreply.github.com>

tonidero added the test label Jun 15, 2023

tonidero marked this pull request as ready for review June 15, 2023 09:04

tonidero requested a review from a team June 15, 2023 09:04

NachoSoto reviewed Jun 15, 2023

View reviewed changes

tonidero changed the base branch from main to toniricodiez/sdk-3182-add-forceverificationerrors-dangerous-setting-for June 16, 2023 07:05

tonidero requested a review from NachoSoto June 16, 2023 07:11

tonidero mentioned this pull request Jun 16, 2023

Trusted entitlements: Add internal mechanism to force signing errors for tests #1070

Merged

NachoSoto approved these changes Jun 16, 2023

View reviewed changes

Base automatically changed from toniricodiez/sdk-3182-add-forceverificationerrors-dangerous-setting-for to main June 19, 2023 06:39

tonidero added 2 commits June 19, 2023 08:41

Add trusted entitlements integration tests

e4d96f9

Join tests into one file

3d0190f

tonidero force-pushed the add-trusted-entitlements-integration-tests branch from fdbc826 to 3d0190f Compare June 19, 2023 06:41

tonidero enabled auto-merge (squash) June 19, 2023 06:41

tonidero merged commit 8ae7c65 into main Jun 19, 2023

tonidero deleted the add-trusted-entitlements-integration-tests branch June 19, 2023 06:51

pablo-guardiola mentioned this pull request Jun 20, 2023

[EXTERNAL] Add awaitCustomerInfo / coroutines tests to TrustedEntitlementsInformationalModeIntegrationTest #1077

Merged

2 tasks

This was referenced Jun 21, 2023

[AUTOMATIC] Release/6.5.1 #1079

Closed

[AUTOMATIC] Release/6.5.1 #1084

Closed

[AUTOMATIC] Release/6.5.1 #1087

Merged

tonidero mentioned this pull request Jun 28, 2023

[EXTERNAL] Add awaitCustomerInfo / coroutines tests to TrustedEntitlementsInformationalModeIntegrationTest (#1077) via @pablo-guardiola #1107

Merged

2 tasks

Uh oh!

Conversation

tonidero commented Jun 15, 2023

Description

Uh oh!

codecov Bot commented Jun 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

NachoSoto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jun 15, 2023 •

edited

Loading